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.

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

Refinement in the presence of optional arguments #1800

Merged
merged 3 commits into from
Jul 26, 2024
Merged

Conversation

xvw
Copy link
Collaborator

@xvw xvw commented Jul 16, 2024

I don't know whether the fixed system might break some of the parameters generated by PPX. (Perhaps if there is a ghost on the parent expression, you can check that it is the None constructor?).

After a conversation with @pitag-ha , we came to the conclusion that the introduction of ghost location in this context (which, in PPX terms, should be reserved for derivers) is a problem of semantics ‘on the PPX side’, so that this fix, which is based on ghost location, is not problematic (for PPX).

Fix #1770

Copy link
Collaborator

@voodoos voodoos left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @xvw !

I just have a few understanding questions :-)
Maybe you could mention the issue in the comments for future reference

src/analysis/destruct.ml Show resolved Hide resolved
Comment on lines 617 to 618
let pexp_desc = Parsetree.Pexp_apply (expr, args) in
{ expr with pexp_desc }
Copy link
Collaborator

Choose a reason for hiding this comment

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

There seems to be a looping use of expr here, is that on purpose ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You are right, it is fixed!

@xvw
Copy link
Collaborator Author

xvw commented Jul 25, 2024

(Comment referencing the issue was added)

Copy link
Collaborator

@voodoos voodoos left a comment

Choose a reason for hiding this comment

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

Thanks !

@voodoos voodoos merged commit 0f54748 into ocaml:main Jul 26, 2024
1 check passed
@xvw xvw deleted the fix-1770 branch July 26, 2024 11:21
@liam923
Copy link
Contributor

liam923 commented Jul 26, 2024

Thank you!

@ncik-roberts
Copy link
Contributor

In janestreet/merlin-jst#73 (comment) I suggested only omitting a ghost-location argument when it's None. Maybe that's preferable here too?

@xvw
Copy link
Collaborator Author

xvw commented Jul 29, 2024

In janestreet/merlin-jst#73 (comment) I suggested only omitting a ghost-location argument when it's None. Maybe that's preferable here too?

Yep, I'll do a patch today! Thanks for the feedback.

voodoos added a commit to voodoos/merlin that referenced this pull request Sep 25, 2024
voodoos added a commit to voodoos/merlin that referenced this pull request Sep 25, 2024
voodoos added a commit to voodoos/merlin that referenced this pull request Sep 25, 2024
voodoos added a commit to voodoos/opam-repository that referenced this pull request Sep 26, 2024
CHANGES:

Thu Sep 26 18:48:42 CEST 2024

  + merlin binary
    - A new `WRAPPING_PREFIX` configuration directive that can be used to tell Merlin
      what to append to the current unit name in the presence of wrapping (ocaml/merlin#1788)
    - Add `-unboxed-types` and `-no-unboxed-types` as ocaml ignored flags (ocaml/merlin#1795, fixes ocaml/merlin#1794)
    - destruct: Refinement in the presence of optional arguments (ocaml/merlin#1800 ocaml/merlin#1807, fixes ocaml/merlin#1770)
    - Implement new expand-node command for expanding PPX annotations (ocaml/merlin#1745)
    - Implement new inlay-hints command for adding hints on a sourcetree (ocaml/merlin#1812)
    - Implement new search-by-type command for searching values by types (ocaml/merlin#1828)
    - Canonicalize paths in occurrences. This helps deduplicate the results and
      show more user-friendly paths. (ocaml/merlin#1840)
    - Fix dot-merlin-reader ignoring `SOURCE_ROOT` and `STDLIB` directives
      (ocaml/merlin#1839, ocaml/merlin#1803)
  + editor modes
    - vim: fix python-3.12 syntax warnings in merlin.py (ocaml/merlin#1798)
    - vim: Dead code / doc removal for previously deleted MerlinPhrase command (ocaml/merlin#1804)
    - emacs: Improve the way that result of polarity search is displayed (ocaml/merlin#1814)
    - emacs: Add `merlin-search-by-type`, `merlin-search-by-polarity` and change the
	  behaviour of `merlin-search` to switch between `by-type` or `by-polarity`
	  depending on the query (ocaml/merlin#1828)
voodoos added a commit to voodoos/opam-repository that referenced this pull request Sep 26, 2024
CHANGES:

Thu Sep 26 18:48:42 CEST 2024

  + merlin binary
    - A new `WRAPPING_PREFIX` configuration directive that can be used to tell Merlin
      what to append to the current unit name in the presence of wrapping (ocaml/merlin#1788)
    - Add `-unboxed-types` and `-no-unboxed-types` as ocaml ignored flags (ocaml/merlin#1795, fixes ocaml/merlin#1794)
    - destruct: Refinement in the presence of optional arguments (ocaml/merlin#1800 ocaml/merlin#1807, fixes ocaml/merlin#1770)
    - Implement new expand-node command for expanding PPX annotations (ocaml/merlin#1745)
    - Implement new inlay-hints command for adding hints on a sourcetree (ocaml/merlin#1812)
    - Add `signature-help` command (ocaml/merlin#1720)
    - Implement new search-by-type command for searching values by types (ocaml/merlin#1828)
    - Fix dot-merlin-reader ignoring `SOURCE_ROOT` and `STDLIB` directives
      (ocaml/merlin#1839, ocaml/merlin#1803)
  + editor modes
    - vim: fix python-3.12 syntax warnings in merlin.py (ocaml/merlin#1798)
    - vim: Dead code / doc removal for previously deleted MerlinPhrase command (ocaml/merlin#1804)
    - emacs: Improve the way that result of polarity search is displayed (ocaml/merlin#1814)
    - emacs: Add `merlin-search-by-type`, `merlin-search-by-polarity` and change the
	  behaviour of `merlin-search` to switch between `by-type` or `by-polarity`
	  depending on the query (ocaml/merlin#1828)
voodoos added a commit to voodoos/opam-repository that referenced this pull request Sep 26, 2024
CHANGES:

Thu Sep 26 18:48:42 CEST 2024

  + merlin binary
    - A new `WRAPPING_PREFIX` configuration directive that can be used to tell Merlin
      what to append to the current unit name in the presence of wrapping (ocaml/merlin#1788)
    - Add `-unboxed-types` and `-no-unboxed-types` as ocaml ignored flags (ocaml/merlin#1795, fixes ocaml/merlin#1794)
    - destruct: Refinement in the presence of optional arguments (ocaml/merlin#1800 ocaml/merlin#1807, fixes ocaml/merlin#1770)
    - Implement new expand-node command for expanding PPX annotations (ocaml/merlin#1745)
    - Implement new inlay-hints command for adding hints on a sourcetree (ocaml/merlin#1812)
    - Add `signature-help` command (ocaml/merlin#1720)
    - Implement new search-by-type command for searching values by types (ocaml/merlin#1828)
    - Fix dot-merlin-reader ignoring `SOURCE_ROOT` and `STDLIB` directives
      (ocaml/merlin#1839, ocaml/merlin#1803)
  + editor modes
    - vim: fix python-3.12 syntax warnings in merlin.py (ocaml/merlin#1798)
    - vim: Dead code / doc removal for previously deleted MerlinPhrase command (ocaml/merlin#1804)
    - emacs: Improve the way that result of polarity search is displayed (ocaml/merlin#1814)
    - emacs: Add `merlin-search-by-type`, `merlin-search-by-polarity` and change the
	  behaviour of `merlin-search` to switch between `by-type` or `by-polarity`
	  depending on the query (ocaml/merlin#1828)
voodoos added a commit to voodoos/opam-repository that referenced this pull request Sep 27, 2024
CHANGES:

Fri Sep 27 12:02:42 CEST 2024

  + merlin binary
    - A new `WRAPPING_PREFIX` configuration directive that can be used to tell Merlin
      what to append to the current unit name in the presence of wrapping (ocaml/merlin#1788)
    - Add `-unboxed-types` and `-no-unboxed-types` as ocaml ignored flags (ocaml/merlin#1795, fixes ocaml/merlin#1794)
    - destruct: Refinement in the presence of optional arguments (ocaml/merlin#1800 ocaml/merlin#1807, fixes ocaml/merlin#1770)
    - Implement new expand-node command for expanding PPX annotations (ocaml/merlin#1745)
    - Implement new inlay-hints command for adding hints on a sourcetree (ocaml/merlin#1812)
    - Implement new search-by-type command for searching values by types (ocaml/merlin#1828)
    - Canonicalize paths in occurrences. This helps deduplicate the results and
      show more user-friendly paths. (ocaml/merlin#1840)
    - Fix dot-merlin-reader ignoring `SOURCE_ROOT` and `STDLIB` directives
      (ocaml/merlin#1839, ocaml/merlin#1803)
  + editor modes
    - vim: fix python-3.12 syntax warnings in merlin.py (ocaml/merlin#1798)
    - vim: Dead code / doc removal for previously deleted MerlinPhrase command (ocaml/merlin#1804)
    - emacs: Improve the way that result of polarity search is displayed (ocaml/merlin#1814)
    - emacs: Add `merlin-search-by-type`, `merlin-search-by-polarity` and change the
	  behaviour of `merlin-search` to switch between `by-type` or `by-polarity`
	  depending on the query (ocaml/merlin#1828)
voodoos added a commit to voodoos/opam-repository that referenced this pull request Sep 27, 2024
CHANGES:

Fri Sep 27 12:02:42 CEST 2024

  + merlin binary
    - A new `WRAPPING_PREFIX` configuration directive that can be used to tell Merlin
      what to append to the current unit name in the presence of wrapping (ocaml/merlin#1788)
    - Add `-unboxed-types` and `-no-unboxed-types` as ocaml ignored flags (ocaml/merlin#1795, fixes ocaml/merlin#1794)
    - destruct: Refinement in the presence of optional arguments (ocaml/merlin#1800 ocaml/merlin#1807, fixes ocaml/merlin#1770)
    - Implement new expand-node command for expanding PPX annotations (ocaml/merlin#1745)
    - Implement new inlay-hints command for adding hints on a sourcetree (ocaml/merlin#1812)
    - Add `signature-help` command (ocaml/merlin#1720)
    - Implement new search-by-type command for searching values by types (ocaml/merlin#1828)
    - Fix dot-merlin-reader ignoring `SOURCE_ROOT` and `STDLIB` directives
      (ocaml/merlin#1839, ocaml/merlin#1803)
  + editor modes
    - vim: fix python-3.12 syntax warnings in merlin.py (ocaml/merlin#1798)
    - vim: Dead code / doc removal for previously deleted MerlinPhrase command (ocaml/merlin#1804)
    - emacs: Improve the way that result of polarity search is displayed (ocaml/merlin#1814)
    - emacs: Add `merlin-search-by-type`, `merlin-search-by-polarity` and change the
	  behaviour of `merlin-search` to switch between `by-type` or `by-polarity`
	  depending on the query (ocaml/merlin#1828)
voodoos added a commit to voodoos/opam-repository that referenced this pull request Sep 27, 2024
CHANGES:

Fri Sep 27 12:02:42 CEST 2024

  + merlin binary
    - A new `WRAPPING_PREFIX` configuration directive that can be used to tell Merlin
      what to append to the current unit name in the presence of wrapping (ocaml/merlin#1788)
    - Add `-unboxed-types` and `-no-unboxed-types` as ocaml ignored flags (ocaml/merlin#1795, fixes ocaml/merlin#1794)
    - destruct: Refinement in the presence of optional arguments (ocaml/merlin#1800 ocaml/merlin#1807, fixes ocaml/merlin#1770)
    - Implement new expand-node command for expanding PPX annotations (ocaml/merlin#1745)
    - Implement new inlay-hints command for adding hints on a sourcetree (ocaml/merlin#1812)
    - Add `signature-help` command (ocaml/merlin#1720)
    - Implement new search-by-type command for searching values by types (ocaml/merlin#1828)
    - Fix dot-merlin-reader ignoring `SOURCE_ROOT` and `STDLIB` directives
      (ocaml/merlin#1839, ocaml/merlin#1803)
  + editor modes
    - vim: fix python-3.12 syntax warnings in merlin.py (ocaml/merlin#1798)
    - vim: Dead code / doc removal for previously deleted MerlinPhrase command (ocaml/merlin#1804)
    - emacs: Improve the way that result of polarity search is displayed (ocaml/merlin#1814)
    - emacs: Add `merlin-search-by-type`, `merlin-search-by-polarity` and change the
	  behaviour of `merlin-search` to switch between `by-type` or `by-polarity`
	  depending on the query (ocaml/merlin#1828)
voodoos added a commit to voodoos/opam-repository that referenced this pull request Sep 27, 2024
CHANGES:

Fri Sep 27 12:02:42 CEST 2024

  + merlin binary
    - A new `WRAPPING_PREFIX` configuration directive that can be used to tell Merlin
      what to append to the current unit name in the presence of wrapping (ocaml/merlin#1788)
    - Add `-unboxed-types` and `-no-unboxed-types` as ocaml ignored flags (ocaml/merlin#1795, fixes ocaml/merlin#1794)
    - destruct: Refinement in the presence of optional arguments (ocaml/merlin#1800 ocaml/merlin#1807, fixes ocaml/merlin#1770)
    - Implement new expand-node command for expanding PPX annotations (ocaml/merlin#1745)
    - Implement new inlay-hints command for adding hints on a sourcetree (ocaml/merlin#1812)
    - Implement new search-by-type command for searching values by types (ocaml/merlin#1828)
    - Canonicalize paths in occurrences. This helps deduplicate the results and
      show more user-friendly paths. (ocaml/merlin#1840)
    - Fix dot-merlin-reader ignoring `SOURCE_ROOT` and `STDLIB` directives
      (ocaml/merlin#1839, ocaml/merlin#1803)
  + editor modes
    - vim: fix python-3.12 syntax warnings in merlin.py (ocaml/merlin#1798)
    - vim: Dead code / doc removal for previously deleted MerlinPhrase command (ocaml/merlin#1804)
    - emacs: Improve the way that result of polarity search is displayed (ocaml/merlin#1814)
    - emacs: Add `merlin-search-by-type`, `merlin-search-by-polarity` and change the
	  behaviour of `merlin-search` to switch between `by-type` or `by-polarity`
	  depending on the query (ocaml/merlin#1828)

[new release] merlin (3 packages) (4.17.1-501)

CHANGES:

Fri Sep 27 12:02:42 CEST 2024

  + merlin binary
    - A new `WRAPPING_PREFIX` configuration directive that can be used to tell Merlin
      what to append to the current unit name in the presence of wrapping (ocaml/merlin#1788)
    - Add `-unboxed-types` and `-no-unboxed-types` as ocaml ignored flags (ocaml/merlin#1795, fixes ocaml/merlin#1794)
    - destruct: Refinement in the presence of optional arguments (ocaml/merlin#1800 ocaml/merlin#1807, fixes ocaml/merlin#1770)
    - Implement new expand-node command for expanding PPX annotations (ocaml/merlin#1745)
    - Implement new inlay-hints command for adding hints on a sourcetree (ocaml/merlin#1812)
    - Add `signature-help` command (ocaml/merlin#1720)
    - Implement new search-by-type command for searching values by types (ocaml/merlin#1828)
    - Fix dot-merlin-reader ignoring `SOURCE_ROOT` and `STDLIB` directives
      (ocaml/merlin#1839, ocaml/merlin#1803)
  + editor modes
    - vim: fix python-3.12 syntax warnings in merlin.py (ocaml/merlin#1798)
    - vim: Dead code / doc removal for previously deleted MerlinPhrase command (ocaml/merlin#1804)
    - emacs: Improve the way that result of polarity search is displayed (ocaml/merlin#1814)
    - emacs: Add `merlin-search-by-type`, `merlin-search-by-polarity` and change the
	  behaviour of `merlin-search` to switch between `by-type` or `by-polarity`
	  depending on the query (ocaml/merlin#1828)

[new release] merlin (3 packages) (4.17.1-414)

CHANGES:

Fri Sep 27 12:02:42 CEST 2024

  + merlin binary
    - A new `WRAPPING_PREFIX` configuration directive that can be used to tell Merlin
      what to append to the current unit name in the presence of wrapping (ocaml/merlin#1788)
    - Add `-unboxed-types` and `-no-unboxed-types` as ocaml ignored flags (ocaml/merlin#1795, fixes ocaml/merlin#1794)
    - destruct: Refinement in the presence of optional arguments (ocaml/merlin#1800 ocaml/merlin#1807, fixes ocaml/merlin#1770)
    - Implement new expand-node command for expanding PPX annotations (ocaml/merlin#1745)
    - Implement new inlay-hints command for adding hints on a sourcetree (ocaml/merlin#1812)
    - Add `signature-help` command (ocaml/merlin#1720)
    - Implement new search-by-type command for searching values by types (ocaml/merlin#1828)
    - Fix dot-merlin-reader ignoring `SOURCE_ROOT` and `STDLIB` directives
      (ocaml/merlin#1839, ocaml/merlin#1803)
  + editor modes
    - vim: fix python-3.12 syntax warnings in merlin.py (ocaml/merlin#1798)
    - vim: Dead code / doc removal for previously deleted MerlinPhrase command (ocaml/merlin#1804)
    - emacs: Improve the way that result of polarity search is displayed (ocaml/merlin#1814)
    - emacs: Add `merlin-search-by-type`, `merlin-search-by-polarity` and change the
	  behaviour of `merlin-search` to switch between `by-type` or `by-polarity`
	  depending on the query (ocaml/merlin#1828)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready for backporting
Status: 4.17-501
Development

Successfully merging this pull request may close these issues.

Optional argument include by case-analysis
4 participants