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

Make left hand side of -> parse as a tuple of arguments #522

Merged
merged 1 commit into from
Dec 27, 2024

Conversation

c42f
Copy link
Member

@c42f c42f commented Dec 22, 2024

This makes argument lists on the left hand side of -> parse as tuples. In particular, this fixes a very inconsistent case where the left hand side previously parsed as a K"block"; we now have:

  • (a;x=1)->b ==> (-> (tuple-p a (parameters (= x 1))) b

rather than (block a (= x 1)) on the left hand side.

In addition, the following forms are treated consistently

  • a->b ==> (-> (tuple a) b)
  • (a)->b ==> (-> (tuple-p a) b)
  • (a,)->b ==> (-> (tuple-p-, a) b)

The upside of this is that expression processing of -> syntax should be much easier.

There's one aberrant case involving where which is particularly difficult and still not dealt with:

(x where T) -> b does not parse as (where (tuple x) T) on the left hand side. However, where precedence involving :: and -> is already horribly broken and this syntax will always be awful to write unless we make breaking changes. So I'm too tempted to call this a lost cause for now 😬.

Compatibility shims for converting the SyntaxNode form back to Expr in order to keep Expr stable are included. (At some point we should consider fixing this and deleting these shims because the new form is so much more consistent and would be reflected neatly into Expr form.)

This makes argument lists on the left hand side of `->` parse as tuples.
In particular, this fixes a very inconsistent case where the left hand
side previously parsed as a `K"block"`; we now have:

* `(a;x=1)->b` ==> `(-> (tuple-p a (parameters (= x 1))) b`

rather than `(block a (= x 1))` on the left hand side.

In addition, the following forms are treated consistently

* `a->b`       ==> `(-> (tuple a) b)`
* `(a)->b`     ==> `(-> (tuple-p a) b)`
* `(a,)->b`    ==> `(-> (tuple-p-, a) b)`

The upside of this is that expression processing of `->` syntax should
be much easier.

There's one aberrant case involving `where` which is particularly
difficult and still not dealt with:

`(x where T) -> b` does not parse as `(where (tuple x) T)` on the left
hand side. However, `where` precedence involving `::` and `->` is
already horribly broken and this syntax will always be awful to write
unless we make breaking changes. So I'm too tempted to call this a lost
cause for now 😬.

Compatibility shims for converting the `SyntaxNode` form back to `Expr`
in order to keep `Expr` stable are included. (At some point we should
consider fixing this and deleting these shims because the new form is so
much more consistent and would be reflected neatly into `Expr` form.)
Copy link

codecov bot commented Dec 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.19%. Comparing base (30109d4) to head (9573836).
Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #522      +/-   ##
==========================================
+ Coverage   96.18%   96.19%   +0.01%     
==========================================
  Files          13       13              
  Lines        4032     4071      +39     
==========================================
+ Hits         3878     3916      +38     
- Misses        154      155       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@c42f c42f merged commit 4a7e846 into main Dec 27, 2024
38 checks passed
@c42f c42f deleted the caf/arrow-args-as-tuple branch December 27, 2024 01:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant