这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@liam923
Copy link
Contributor

@liam923 liam923 commented Nov 29, 2024

Imports compiler changes for version 5.2.0minus-4. The first three commits are the result of running the import script (and committing conflicts), so they should be skipped when reviewing.

Beyond general review, there are a few things for which I would like special attention from certain people:

  • @goldfirere - It looks to me that the compiler no longer prints user-written jkind annotations. I made Merlin reflect this change. Please looks at the jkind printing changes in Printtyp.tree_of_type_decl. This led to test changes in (only) tests/test-dirs/type-enclosing/jane-street.t/run.t.
  • @goldfirere - Please look at Ptyp_of_type.constructor_argument. I assumed that ca_jkind is the kind for ca_type and can therefore be ignored. (For context, this function is converting from a typed tree to a parse tree for Merlin's construct command.)
  • @riaqn - I left comments at relevant locations

Copy link
Contributor

@goldfirere goldfirere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reviewed the last 6 commits.

at_modalities_expr:
| AT modalities {$2}
| AT error { expecting $loc($2) "modality expression" }
// | AT error { expecting $loc($2) "modality expression" }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why comment this out?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merlin's error recovery when parsing works differently than in the compiler. You'll notice there's other similar things commented out.

@goldfirere
Copy link
Contributor

I've read the questions in the top post and am OK with these changes.

Specifically about jkind annotations: the change was to drop automatically printing user-written jkind annotations on [@@unboxed] types. This is reflected in the new output, and I like the change. You can get the annotation with a higher verbosity. Good!

@liam923 liam923 merged commit 79107c6 into main Dec 2, 2024
1 of 2 checks passed
@liam923 liam923 deleted the merge-5.2.0minus-4 branch December 2, 2024 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants