Skip to content
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.

Sign up for GitHub

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

Jump to bottom

test: test enclosing type for let+ ... and+ ... in expression #1653

Merged
merged 6 commits into from
Aug 24, 2023
Merged

test: test enclosing type for let+ ... and+ ... in expression #1653

merged 6 commits into from
Aug 24, 2023

Conversation

Alizter
Copy link
Contributor

@Alizter Alizter commented Jul 5, 2023

This demonstrates that type enclosing has the incorrect behaviour for letop's with multiple ands.

@voodoos
Copy link
Collaborator

voodoos commented Jul 24, 2023

First diagnostic: The issue is due to the typedtree nodes not respecting location inclusion / disjointness rules. Some locations issues by the parser are also very imprecise.

@voodoos
Copy link
Collaborator

voodoos commented Jul 27, 2023

I have a first improvement: it is now possible to get the type of the patterns !
On the let+ and and+ however the whole expression is selected and it's type shown.
That's coherent but not very informative.

@voodoos
Copy link
Collaborator

voodoos commented Jul 27, 2023

Now the operator's types are shown too.

@voodoos voodoos requested a review from trefis July 27, 2023 15:32
@voodoos voodoos force-pushed the ps/branch/test__test_enclosing_type_for_let______and______in_expression branch 2 times, most recently from 74bd57b to ae6b50b Compare July 27, 2023 16:08
@voodoos voodoos mentioned this pull request Jul 27, 2023
Open
Alizter
Alizter commented Jul 27, 2023
View reviewed changes
src/analysis/type_utils.ml
@@ -30,6 +30,10 @@ open Std

module Verbosity = Mconfig.Verbosity

let protect expr =
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does protect do?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To get the "generic" type of the operator Merlin needs to parse it and type it. But let++ is not valid syntax so adding parenthesis (let++) is required for it to parse correctly. The protect function uses a function from compiler libs to determine is the expression requires parenthesizing before parsing.

Alizter and others added 6 commits August 24, 2023 11:15
@Alizter @voodoos
test: test enclosing type for let+ ... and+ ... in expression
a719e71 
Signed-off-by: Ali Caglayan <[email protected]>
@voodoos
Improve letop's nodes ordering in browse_raw
d9451f8 
This should allow Merlin to get the correct node when the cursor is on a pattern or it's rhs.

Right now it is not possible to get the type of the operator.
@voodoos
Add a binding_op node to Browse_raw
d61b78d
@voodoos
Protect before parsing in type_expr
e1bb3a4
@voodoos
Update test with more branches
b3f4096
@voodoos
Add changelog entry
761822b
@voodoos voodoos force-pushed the ps/branch/test__test_enclosing_type_for_let______and______in_expression branch from ae6b50b to 761822b Compare August 24, 2023 09:17
@voodoos voodoos merged commit cb4a973 into ocaml:master Aug 24, 2023
6 of 7 checks passed
@Alizter Alizter deleted the ps/branch/test__test_enclosing_type_for_let______and______in_expression branch August 24, 2023 10:27
@Alizter
Copy link
Contributor Author

Alizter commented Aug 24, 2023

Thanks @voodoos!

voodoos added a commit to voodoos/merlin that referenced this pull request Aug 24, 2023
@voodoos
[B] Fix type enclosing for binding operators ocaml#1653
a5996a2 
test: test enclosing type for let+ ... and+ ... in expression
from Alizter/ps/branch/test__test_enclosing_type_for_let______and______in_expression
voodoos added a commit to voodoos/merlin that referenced this pull request Aug 24, 2023
@voodoos
[B] Fix type enclosing for binding operators ocaml#1653
b7973d5 
test: test enclosing type for let+ ... and+ ... in expression
from Alizter/ps/branch/test__test_enclosing_type_for_let______and______in_expression
voodoos added a commit to voodoos/opam-repository that referenced this pull request Aug 24, 2023
@voodoos
[new release] merlin, merlin-lib and dot-merlin-reader (4.10-414)
2bc5e35 
CHANGES:

Thu Aug 24 17:17:42 CEST 2023

  + merlin binary
    - Constrain socket path buffer size to avoid build warnings (ocaml/merlin#1631)
    - Handle concurrent server start (ocaml/merlin#1622)
    - Omit module prefixes for constructors and record fields in the
      `construct` command (ocaml/merlin#1618).  Prefixes are still produced when
      warning 42 (disambiguated name) is active.
    - Correctly invalidate PPX cache when pipeline ran partially (ocaml/merlin#1650,
      fixes ocaml/merlin#1647)
    - Prevent `short-path` from looping in some cases related to recursive type
      definitions (ocaml/merlin#1645)
    - Support parsing negative numbers in sexps (ocaml/merlin#1655)
    - Fix construct not working with inline records (ocaml/merlin#1658)
    - Improve behavior of `type-enclosing` on let/and operators (ocaml/merlin#1653)
    - Fix occurrences of extension constructors (ocaml/merlin#1662)
    - Improve node selection when ghosts are present (ocaml/merlin#1664, fixes ocaml/merlin#1660)
  + editor modes
    - emacs: call merlin-client-logger with "interrupted" if the
      merlin binary itself is interrupted, not just the parsing of the
      result (ocaml/merlin#1626).
    - emacs: merlin-construct, with a prefix argument, now includes
      local values in the completion options.  Alternatively, this
      behavior can be enabled permanently by customizing
      `merlin-construct-with-local-values` (ocaml/merlin#1644)
    - emacs: add support for opam-switch-mode (ocaml/merlin#1654, fixes ocaml/merlin#1591).
      See <https://github.com/ProofGeneral/opam-switch-mode>
voodoos added a commit to voodoos/opam-repository that referenced this pull request Aug 24, 2023
@voodoos
[new release] merlin, merlin-lib and dot-merlin-reader (4.10-500)
e56b3a0 
CHANGES:

Thu Aug 24 17:17:42 CEST 2023

  + merlin binary
    - Constrain socket path buffer size to avoid build warnings (ocaml/merlin#1631)
    - Handle concurrent server start (ocaml/merlin#1622)
    - Omit module prefixes for constructors and record fields in the
      `construct` command (ocaml/merlin#1618).  Prefixes are still produced when
      warning 42 (disambiguated name) is active.
    - Correctly invalidate PPX cache when pipeline ran partially (ocaml/merlin#1650,
      fixes ocaml/merlin#1647)
    - Prevent `short-path` from looping in some cases related to recursive type
      definitions (ocaml/merlin#1645)
    - Support parsing negative numbers in sexps (ocaml/merlin#1655)
    - Fix construct not working with inline records (ocaml/merlin#1658)
    - Improve behavior of `type-enclosing` on let/and operators (ocaml/merlin#1653)
    - Fix occurrences of extension constructors (ocaml/merlin#1662)
    - Improve node selection when ghosts are present (ocaml/merlin#1664, fixes ocaml/merlin#1660)
  + editor modes
    - emacs: call merlin-client-logger with "interrupted" if the
      merlin binary itself is interrupted, not just the parsing of the
      result (ocaml/merlin#1626).
    - emacs: merlin-construct, with a prefix argument, now includes
      local values in the completion options.  Alternatively, this
      behavior can be enabled permanently by customizing
      `merlin-construct-with-local-values` (ocaml/merlin#1644)
    - emacs: add support for opam-switch-mode (ocaml/merlin#1654, fixes ocaml/merlin#1591).
      See <https://github.com/ProofGeneral/opam-switch-mode>
voodoos added a commit to voodoos/opam-repository that referenced this pull request Aug 24, 2023
@voodoos
[new release] merlin, merlin-lib (4.10-414)
f445238 
CHANGES:

Thu Aug 24 17:17:42 CEST 2023

  + merlin binary
    - Constrain socket path buffer size to avoid build warnings (ocaml/merlin#1631)
    - Handle concurrent server start (ocaml/merlin#1622)
    - Omit module prefixes for constructors and record fields in the
      `construct` command (ocaml/merlin#1618).  Prefixes are still produced when
      warning 42 (disambiguated name) is active.
    - Correctly invalidate PPX cache when pipeline ran partially (ocaml/merlin#1650,
      fixes ocaml/merlin#1647)
    - Prevent `short-path` from looping in some cases related to recursive type
      definitions (ocaml/merlin#1645)
    - Support parsing negative numbers in sexps (ocaml/merlin#1655)
    - Fix construct not working with inline records (ocaml/merlin#1658)
    - Improve behavior of `type-enclosing` on let/and operators (ocaml/merlin#1653)
    - Fix occurrences of extension constructors (ocaml/merlin#1662)
    - Improve node selection when ghosts are present (ocaml/merlin#1664, fixes ocaml/merlin#1660)
  + editor modes
    - emacs: call merlin-client-logger with "interrupted" if the
      merlin binary itself is interrupted, not just the parsing of the
      result (ocaml/merlin#1626).
    - emacs: merlin-construct, with a prefix argument, now includes
      local values in the completion options.  Alternatively, this
      behavior can be enabled permanently by customizing
      `merlin-construct-with-local-values` (ocaml/merlin#1644)
    - emacs: add support for opam-switch-mode (ocaml/merlin#1654, fixes ocaml/merlin#1591).
      See <https://github.com/ProofGeneral/opam-switch-mode>
voodoos added a commit to voodoos/opam-repository that referenced this pull request Aug 24, 2023
@voodoos
[new release] merlin, merlin-lib (4.10-500)
38b91b3 
CHANGES:

Thu Aug 24 17:17:42 CEST 2023

  + merlin binary
    - Constrain socket path buffer size to avoid build warnings (ocaml/merlin#1631)
    - Handle concurrent server start (ocaml/merlin#1622)
    - Omit module prefixes for constructors and record fields in the
      `construct` command (ocaml/merlin#1618).  Prefixes are still produced when
      warning 42 (disambiguated name) is active.
    - Correctly invalidate PPX cache when pipeline ran partially (ocaml/merlin#1650,
      fixes ocaml/merlin#1647)
    - Prevent `short-path` from looping in some cases related to recursive type
      definitions (ocaml/merlin#1645)
    - Support parsing negative numbers in sexps (ocaml/merlin#1655)
    - Fix construct not working with inline records (ocaml/merlin#1658)
    - Improve behavior of `type-enclosing` on let/and operators (ocaml/merlin#1653)
    - Fix occurrences of extension constructors (ocaml/merlin#1662)
    - Improve node selection when ghosts are present (ocaml/merlin#1664, fixes ocaml/merlin#1660)
  + editor modes
    - emacs: call merlin-client-logger with "interrupted" if the
      merlin binary itself is interrupted, not just the parsing of the
      result (ocaml/merlin#1626).
    - emacs: merlin-construct, with a prefix argument, now includes
      local values in the completion options.  Alternatively, this
      behavior can be enabled permanently by customizing
      `merlin-construct-with-local-values` (ocaml/merlin#1644)
    - emacs: add support for opam-switch-mode (ocaml/merlin#1654, fixes ocaml/merlin#1591).
      See <https://github.com/ProofGeneral/opam-switch-mode>
@voodoos voodoos mentioned this pull request Aug 24, 2023
Merged
@Alizter Alizter mentioned this pull request Oct 16, 2023
Closed
nberth pushed a commit to nberth/opam-repository that referenced this pull request Jun 18, 2024
@voodoos @nberth
[new release] merlin, merlin-lib (4.10-414)
dccf2b8 
CHANGES:

Thu Aug 24 17:17:42 CEST 2023

  + merlin binary
    - Constrain socket path buffer size to avoid build warnings (ocaml/merlin#1631)
    - Handle concurrent server start (ocaml/merlin#1622)
    - Omit module prefixes for constructors and record fields in the
      `construct` command (ocaml/merlin#1618).  Prefixes are still produced when
      warning 42 (disambiguated name) is active.
    - Correctly invalidate PPX cache when pipeline ran partially (ocaml/merlin#1650,
      fixes ocaml/merlin#1647)
    - Prevent `short-path` from looping in some cases related to recursive type
      definitions (ocaml/merlin#1645)
    - Support parsing negative numbers in sexps (ocaml/merlin#1655)
    - Fix construct not working with inline records (ocaml/merlin#1658)
    - Improve behavior of `type-enclosing` on let/and operators (ocaml/merlin#1653)
    - Fix occurrences of extension constructors (ocaml/merlin#1662)
    - Improve node selection when ghosts are present (ocaml/merlin#1664, fixes ocaml/merlin#1660)
  + editor modes
    - emacs: call merlin-client-logger with "interrupted" if the
      merlin binary itself is interrupted, not just the parsing of the
      result (ocaml/merlin#1626).
    - emacs: merlin-construct, with a prefix argument, now includes
      local values in the completion options.  Alternatively, this
      behavior can be enabled permanently by customizing
      `merlin-construct-with-local-values` (ocaml/merlin#1644)
    - emacs: add support for opam-switch-mode (ocaml/merlin#1654, fixes ocaml/merlin#1591).
      See <https://github.com/ProofGeneral/opam-switch-mode>
nberth pushed a commit to nberth/opam-repository that referenced this pull request Jun 18, 2024
@voodoos @nberth
[new release] merlin, merlin-lib (4.10-500)
bc02e79 
CHANGES:

Thu Aug 24 17:17:42 CEST 2023

  + merlin binary
    - Constrain socket path buffer size to avoid build warnings (ocaml/merlin#1631)
    - Handle concurrent server start (ocaml/merlin#1622)
    - Omit module prefixes for constructors and record fields in the
      `construct` command (ocaml/merlin#1618).  Prefixes are still produced when
      warning 42 (disambiguated name) is active.
    - Correctly invalidate PPX cache when pipeline ran partially (ocaml/merlin#1650,
      fixes ocaml/merlin#1647)
    - Prevent `short-path` from looping in some cases related to recursive type
      definitions (ocaml/merlin#1645)
    - Support parsing negative numbers in sexps (ocaml/merlin#1655)
    - Fix construct not working with inline records (ocaml/merlin#1658)
    - Improve behavior of `type-enclosing` on let/and operators (ocaml/merlin#1653)
    - Fix occurrences of extension constructors (ocaml/merlin#1662)
    - Improve node selection when ghosts are present (ocaml/merlin#1664, fixes ocaml/merlin#1660)
  + editor modes
    - emacs: call merlin-client-logger with "interrupted" if the
      merlin binary itself is interrupted, not just the parsing of the
      result (ocaml/merlin#1626).
    - emacs: merlin-construct, with a prefix argument, now includes
      local values in the completion options.  Alternatively, this
      behavior can be enabled permanently by customizing
      `merlin-construct-with-local-values` (ocaml/merlin#1644)
    - emacs: add support for opam-switch-mode (ocaml/merlin#1654, fixes ocaml/merlin#1591).
      See <https://github.com/ProofGeneral/opam-switch-mode>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@voodoos voodoos voodoos left review comments

@trefis trefis Awaiting requested review from trefis

Assignees
No one assigned
Labels
None yet
Projects
Backport to 501
Status: 4.10-500
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Alizter @voodoos