-
Notifications
You must be signed in to change notification settings - Fork 233
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix wrong type information for constructors under type coercions #1091
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactor the
Browse_raw.fold_node
so thatexp_extra
nodes are considered siblings of their corresponding expression and not parents.
You meant children right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good!
Although I wonder if the changes to the browse tree are actually necessary, have you tried without them? What diff does it produce on your tests if you remove them.
If switching back to the original browse tree the wrong type is returned: Done: 1608/1621 (jobs: 4)File "tests/test-dirs/type-enclosing/cons.t", line 1, characters 0-0:
git (internal) (exit 1)
(cd _build/default && /usr/bin/git diff --no-index --color=always -u tests/test-dirs/type-enclosing/cons.t tests/test-dirs/type-enclosing/cons.t.corrected)
diff --git a/tests/test-dirs/type-enclosing/cons.t b/tests/test-dirs/type-enclosing/cons.t.corrected
index 2eb1f699..e04a6da2 100644
--- a/tests/test-dirs/type-enclosing/cons.t
+++ b/tests/test-dirs/type-enclosing/cons.t.corrected
@@ -13,7 +13,7 @@ Various parts of the cons.ml:
"line": 4,
"col": 14
},
- "type": "t",
+ "type": "t'",
"tail": "no"
},
{
@@ -108,7 +108,7 @@ We aim to fix that in the future.
"line": 15,
"col": 15
},
- "type": "M.t",
+ "type": "M.u",
"tail": "no"
},
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Do you want to clean-up the history or should I just squash everything?
14b83ad
to
e014287
Compare
PR "Fix wrong type information for constructors under type coercions #1091" did the same to expressions, but it turns out that we have the same issue with pattern. When doing type enclosing on `def` in: let def : float = float_of_int 3 The browse structure looks like: [ core_type; core_type; pattern; value_binding; structure_item; structure ] The two core types come from a `Tpat_constraint` which is misplaced because of the typedtree representation.
PR "Fix wrong type information for constructors under type coercions #1091" did the same to expressions, but it turns out that we have the same issue with pattern. When doing type enclosing on `def` in: let def : float = float_of_int 3 The browse structure looks like: [ core_type; core_type; pattern; value_binding; structure_item; structure ] The two core types come from a `Tpat_constraint` which is misplaced because of the typedtree representation.
CHANGES: Tue Apr 14 15:25:05 CEST 2020 + backend - full support from OCaml 4.02 to OCaml 4.10 (ocaml/merlin#1117, ocaml/merlin#1127) - fix desynchronized cache (ocaml/merlin#1120) - short path for OCaml 4.09 and OCaml 4.10 (ocaml/merlin#1082, ocaml/merlin#1117) - catch and test environment initialization errors (ocaml/merlin#1083, ocaml/merlin#1130) - restore type levels after recovery (ocaml/merlin#1092) + frontend - fix syntax errors in 4.08 and 4.09 (ocaml/merlin#1081) - complete-prefix command accepts -kind option to filter results (ocaml/merlin#1071) - code cleanup (ocaml/merlin#1093, ocaml/merlin#1079, ocaml/merlin#1112) - better handling of expression and pattern extra nodes during browse tree traversal (ocaml/merlin#1091, ocaml/merlin#1121) - improve context detection (e.g. appropriate namespace for lookup) for various queries (ocaml/merlin#1104, ocaml/merlin#1110) - add stdlib to locate source path (ocaml/merlin#1085) + editor modes - vim: tweak heuristic to select python version (ocaml/merlin#1111) - emacs: marlin/call - lsp: move server to its own repository (ocaml/merlin#1069), https://github.com/ocaml/ocaml-lsp + testsuite - dune rules for the testsuite are now generated, deterministic and can be run individually (ocaml/merlin#1068, ocaml/merlin#1070, ocaml/merlin#1072) - fix incorrect command-line arguments in tests (ocaml/merlin#1073) - better coverage of frontend features (ocaml/merlin#1075, ocaml/merlin#1078, ocaml/merlin#1088, ocaml/merlin#1089, ocaml/merlin#1126) Build no longer relies on implicit transitive_deps (ocaml/merlin#1065).
CHANGES: Tue Apr 14 15:25:05 CEST 2020 + ocaml support - full support from OCaml 4.02 to OCaml 4.10 (ocaml/merlin#1117, ocaml/merlin#1127) - fix desynchronized cache (ocaml/merlin#1120) - short path for OCaml 4.09 and OCaml 4.10 (ocaml/merlin#1082, ocaml/merlin#1117) - catch and test environment initialization errors (ocaml/merlin#1083, ocaml/merlin#1130) - restore type levels after recovery (ocaml/merlin#1092) + merlin binary - fix syntax errors in 4.08 and 4.09 (ocaml/merlin#1081) - complete-prefix command accepts -kind option to filter results (ocaml/merlin#1071) - code cleanup (ocaml/merlin#1093, ocaml/merlin#1079, ocaml/merlin#1112) - better handling of expression and pattern extra nodes during browse tree traversal (ocaml/merlin#1091, ocaml/merlin#1121) - improve context detection (e.g. appropriate namespace for lookup) for various queries (ocaml/merlin#1104, ocaml/merlin#1110) - add stdlib to locate source path (ocaml/merlin#1085) + editor modes - vim: tweak heuristic to select python version (ocaml/merlin#1111) - emacs: marlin/call - lsp: move server to its own repository (ocaml/merlin#1069), https://github.com/ocaml/ocaml-lsp + test suite - dune rules for the test suite are now generated, deterministic and can be run individually (ocaml/merlin#1068, ocaml/merlin#1070, ocaml/merlin#1072) - fix incorrect command-line arguments in tests (ocaml/merlin#1073) - better coverage of frontend features (ocaml/merlin#1075, ocaml/merlin#1078, ocaml/merlin#1088, ocaml/merlin#1089, ocaml/merlin#1126) Build no longer relies on implicit transitive_deps (ocaml/merlin#1065).
PR "Fix wrong type information for constructors under type coercions ocaml#1091" did the same to expressions, but it turns out that we have the same issue with pattern. When doing type enclosing on `def` in: let def : float = float_of_int 3 The browse structure looks like: [ core_type; core_type; pattern; value_binding; structure_item; structure ] The two core types come from a `Tpat_constraint` which is misplaced because of the typedtree representation.
Bug description:
Given the following file:
The type hint given by Merlin for the
U
occuring inf
ist'
instead oft
.The same problem happens in patterns.
Solution:
Browse_raw.fold_node
so thatexp_extra
nodes are considered children of their corresponding expression and not parents.