Remove local rewrite flag and enable it by default #44
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
df7ba4d to
0184a59
Compare
goldfirere
requested changes
Nov 8, 2023
Author
|
Removed the logic for erasing legacy local annotation syntax in |
21fd82a to
4bb9282
Compare
eceb638 to
9f85b91
Compare
goldfirere
reviewed
Nov 21, 2023
goldfirere
approved these changes
Nov 27, 2023
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>
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>
ffab059 to
f458255
Compare
Signed-off-by: alanechang <alanechang@janestreet.com>
Signed-off-by: alanechang <alanechang@janestreet.com>
Author
|
@goldfirere Added one more commit to handle the pattern case differently. This fixes an issue caught by the CI tests. PR should be ready to merge if that commit looks good. |
|
Took a look at that last commit. Looks fine. Let's merge. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes the
--rewrite-old-style-jane-street-local-annotationsflag and enables rewriting with the new syntax by default.A few consequences of this change are:
--erase-jane-syntaxnow erases the old style local annotation too. See local-erased.ml.ref for differencesIn order for the AST equality checks to pass when we are modifying the tree deliberately, a newThe standard AST normalization is changed to convert old local syntax into the new syntax in order for the equality check to pass.ignore_local_annot_differencesoption is added to ignore changes around local annotations in the standard AST. This option is set automatically when the legacy syntax is encountered. The alternative here is to convert old local syntax to the new or vice versa during normalization, which is non-trivial and will involve duplicating logic from jane syntax.