-
Notifications
You must be signed in to change notification settings - Fork 471
Parser leading attr strictness #7787
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
c91ae80
to
01158eb
Compare
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/win32-x64
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great.
Just some comments on code duplication.
compiler/syntax/src/res_core.ml
Outdated
| Lbrace -> | ||
(* consume the attributes and any doc comments before the record *) | ||
ignore (parse_attributes p); | ||
let rec skip_docs () = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same fun defined a few lines above.
compiler/syntax/src/res_core.ml
Outdated
| DotDot -> | ||
(* attributes before an open variant marker; consume attrs/docs then handle `..` *) | ||
ignore (parse_attributes p); | ||
let rec skip_docs () = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here
compiler/syntax/src/res_core.ml
Outdated
Parser.lookahead p (fun state -> | ||
ignore (parse_attributes state); | ||
(* optionally skip a run of doc comments before deciding *) | ||
let rec skip_docs () = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again
y: int } | ||
type nonrec t = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The printer will also need attention. Perhaps for later and one can just add the issue for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean?
01158eb
to
4329600
Compare
Follow up for #7782.