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

Conversation

@alanechang
Copy link

@alanechang alanechang commented Feb 5, 2024

This PR adds support for erasing layout annotations when the --erase-jane-syntax flag is provided.

This includes parser standard changes:

  • Parser standard is changed to parse layout annotation on type declarations as attribute names prefixed with jane.erasable.layouts to have them dropped during ast normalization. Implementation here differs from the jane syntax encoding used by the flambda-backend parser. This is meant as a temporary solution before we can do straightforward parser imports to make them the same again. The alternative of porting this PR doesn't seem to be worth the effort.
  • Standard ast normalization is changed to allow float# --> float rewrites.

And parser extended changes:

  • Parser extended is changed to drop all layout annotations during parsing (same as what we do for locals).
  • Parser extended removes type alias nodes with no names completely (since (... as _) is invalid syntax)

This PR notably doesn't make any changes to [@@immediate] and [@@immediate64]. Those attributes remain the same through this process. See update

This PR should be merged after #60. Already merged


Update regarding [@@immediate] and [@@immediate64]

Last three commits added to this PR changes the treatment of these attributes.

Now ocamlformat rewrites both of these attributes into equivalent layout annotations by default. And when running with --erase-jane-syntax, the program does the opposite and rewrites immediate/immediate64 layout annotation on type declarations back into attributes.

Make sure this flambda-backend PR gets merged before this change to not break existing code. merged

Might be easier to review all the changes before this and then the immediate rewrite commits separately.

@alanechang alanechang changed the base branch from aec/backport-gadt-var-fix to jane February 14, 2024 15:11
Signed-off-by: alanechang <alanechang@janestreet.com>
Signed-off-by: alanechang <alanechang@janestreet.com>
Signed-off-by: alanechang <alanechang@janestreet.com>
Signed-off-by: alanechang <alanechang@janestreet.com>
Signed-off-by: alanechang <alanechang@janestreet.com>
Signed-off-by: alanechang <alanechang@janestreet.com>
@alanechang alanechang force-pushed the aec/erase-layout-annot branch from 0ba87c2 to f98013a Compare February 14, 2024 15:12
Copy link

@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.

This implementation looks sensible, but see a comment in the test -- I think we disagree about what the spec here is.

Signed-off-by: alanechang <alanechang@janestreet.com>
Copy link

@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.

Ah, yes, I forgot to review the second half!

Signed-off-by: alanechang <alanechang@janestreet.com>
Signed-off-by: alanechang <alanechang@janestreet.com>
Signed-off-by: alanechang <alanechang@janestreet.com>
@goldfirere goldfirere merged commit d87754a into jane Feb 20, 2024
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.

3 participants