Merge in upstream main for 5.2 merge #82
Merged
Conversation
This file contains 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
Merlin has no use for the `-cmi-file` flag but should not reject it.
(35fdd0226e2e05a1a8244ecfec780b563b23b59c)
rev: 02b39701d81ef4d4f5824a2d018e6387b1eeb5a7
- enable or disable aliases traversal - mark approximated results as such - improve constr / labels cases + traverse aliases - refactor locate and env_lookup
FIXME Merlin recovery and other feature related to functions are probably broken
and expected locations improvements
|
Thanks for the review. It should be back to you @liam923 |
liam923
approved these changes
Aug 23, 2024
ncik-roberts
force-pushed
the
merge-with-upstream-merlin-5.2
branch
from
0352b4f
to
6796198
Compare
|
I'll leave instructions for how to merge this. |
…m-merlin-round-2-of-conflict-fixing
* Make shape reduction use shapes from read files * Incorporate pr feedback
* Add UNIT_NAME_FOR directive * Promote tests * Capitalize unitname to match UNIT_NAME behavior * Fix stray grep * Add Misc.unitname for wrapping prefix case * Fix WRAPPING_PREFIX removals * Rename tag_error * Use rev_split_words * Add error message to merlin_dot_protocol parser
* Import ocaml sources for ocaml-flambda/flambda-backend@af06aef5b8 * Import OCaml for minus-23 * Update magic numbers * Fix compilation errors
* Import ocaml sources for ocaml-flambda/flambda-backend@a54ec041f * Automatic merges * Bump magic numbers * Fix build error in profile.ml * Promote tests * Add ignored flags
…of github.com:janestreet/merlin-jst into merge-with-upstream-merlin-round-2-of-conflict-fixing
…nd-2-of-conflict-fixing 5.2 merge: round 2 of conflict fixing
ncik-roberts
merged commit ece8c65
into
merge-with-flambda-backend-5.2-merge
1 of 2 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Merges the HEAD of the parent PR #81 with a reasonably recent
upstream/main. Fix all tests.Caveat: I probably should have done the merge with
upstream/v5.1-502, which is the tagged version for 5.2. I skimmed through the commits tomainsince then (there are about 20) and almost all of them are bugfixes. There is a new feature: ocaml/merlin#1745, but I'm not worried about it as it doesn't touch existing code paths.Note for next version bump: be sure to merge with the tagged version! I don't think we hit any difficulty in this merge, but that's half by luck.
This merge was difficult. The last merge was done with https://github.com/ocaml/merlin/tree/v4.14-501, which does not share history with upstream
main(orv5.1-502). This means thatgit mergecreates a lot of spurious conflicts.The way I created this PR was as follows:
Bwhere I checked out tov4.14-501, and committed a change bringing the files to the same state asupstream/main.Bwith the HEAD of 5.2 Merge #81.upstream/mainto make the branches share history (6b3f6af).Bafter step 2 (240f8aa), and (ii) a series of commits fixing up the conflicts and fixing the tests.I expect that future merges will be less painful — I think we'll just be able to run
git mergewith the tagged version.The reason I went to all this effort is because it halved the number of conflicts. There are about 400 conflicts in
B, and about 800 conflicts that I would have had to fix if I had just merged withupstream/mainfrom the start.There might have been a better way to do this. Let me know if you think of one.
Instructions for reviewer
Review this diff: https://github.com/janestreet/merlin-jst/pull/82/files/6b3f6af65d1088e1c7d66aec170b90ed1a50427e..150f7876a9dbf4c50674be292a330b2b5df0afd3
This is the diff between:
upstream/mainExplanation of three-way diff:
I usually take the upper third of the diff. Places where I don't are notable.
TODO: We should scan through the diff between merlin-jst's implementation of type-checking and flambda-backend's.
Notable diffs
src/ocaml/typing/typecore.mldrops upstream's approach for recovery when typing functions. Our approach is slightly different. We should make them converge, but this PR is not the place to do it: it's tricky, and there's a lot going on in this PR already.