Conversation
goldfirere
approved these changes
Aug 2, 2024
goldfirere
approved these changes
Aug 2, 2024
goldfirere
reviewed
Aug 5, 2024
This reverts commit fc1857f.
c69f6d2 to
4ccb2ae
Compare
ITO444
pushed a commit
to ITO444/flambda-backend
that referenced
this pull request
Aug 14, 2024
* Modify cms format * Convert cms_uid_to_loc to be an array * Add crs * Add comment to need_to_clear_env * Remove debugging type annotation * Convert directly to array * Remove loadpath from cms * Guard initial_env behind -bin-annot-occurrences * Revert "Convert directly to array" This reverts commit fc1857f. * Directly use array * Revert "Directly use array" This reverts commit 4ccb2ae. * Revert "Convert cms_uid_to_loc to be an array" This reverts commit 808b35b.
lukemaurer
pushed a commit
to lukemaurer/flambda-backend
that referenced
this pull request
Oct 23, 2024
* Modify cms format * Convert cms_uid_to_loc to be an array * Add crs * Add comment to need_to_clear_env * Remove debugging type annotation * Convert directly to array * Remove loadpath from cms * Guard initial_env behind -bin-annot-occurrences * Revert "Convert directly to array" This reverts commit fc1857f. * Directly use array * Revert "Directly use array" This reverts commit 4ccb2ae. * Revert "Convert cms_uid_to_loc to be an array" This reverts commit 808b35b.
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
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.
To support project wide occurrences, in the short term, cms files need to contain the loadpath and initial environment, along with the text of an identifier. This is information that can likely be removed in the future, but this will get us to a working initial version faster.
Also, this PR updates the
cms_uid_to_locfield to carry a location that is more desirable. Merlin uses this field to tie a uid to a definition location: https://github.com/janestreet/merlin-jst/blob/main/src/analysis/locate.ml#L224. When usingcmts, Merlin returns the location of the name in the definition, via a utility functionloc_of_decl. When using cms files, it uses the location stored incms_uid_to_loc. But currently, the location stored incms_uid_to_locis the location of the entire definition. This PR copies Merlin'sloc_of_decl(with some slight modifications) and uses the location returned by it forcms_uid_to_loc. This will make Merlin's behavior consistent betweencmsandcmtfiles.