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

Add RFC: Destructuring assignment #2649

Closed
wants to merge 1 commit into from
Closed

Add RFC: Destructuring assignment #2649

wants to merge 1 commit into from

Conversation

Walther
Copy link

@Walther Walther commented Feb 27, 2019

Rendered

RFC for #372: Destructuring assignment for tuples.

Example:

fn tuple(a: i32, b: i32) -> (i32, i32) {
    (a, b)
}

fn main() {
    let mut a: i32;
    let mut b: i32;

    (a, b) = tuple(1, 2);
    println!("{}", a);
    println!("{}", b);
}

Linking two other issues:


Happy to make any and all modifications that are necessary - thank you in advance for all the help <3
(This is my first RFC)

@Walther Walther mentioned this pull request Feb 27, 2019
@Ixrec
Copy link
Contributor

Ixrec commented Feb 27, 2019

From what I gathered in past threads, by far the biggest concern here is that allowing arbitrary patterns on the left side of an assignment involves a huge increase in grammar and parser complexity, so the vast majority of the argument here needs to be that the benefits of the feature justify whatever that complexity increase amounts to in practice. Especially after the heated discussion over on #2544 apparently failed to reach any consensus about whether LL(k) is important to us despite revealing strong opinions on both sides of the issue.

Unfortunately, one sentence in the Drawbacks section acknowledging that the problem exists simply is not enough to make a compelling argument that the benefits outweigh it. I suspect huge changes of this sort are blocked on a bunch of the stuff the grammar working group is doing.

@jonas-schievink jonas-schievink added T-lang Relevant to the language team, which will review and decide on the RFC. A-syntax Syntax related proposals & ideas A-patterns Pattern matching related proposals & ideas labels Feb 27, 2019
@Walther
Copy link
Author

Walther commented Feb 27, 2019

This comment by @Kimundi seemed interesting:

Not sure if this argument has been made elsewhere already, but this could probably still be made LL(k):

  • Still parse the the LHS of EXPR = EXPR as an expression, keeping the LL(k) property.
  • Instead of giving out an early with an error: invalid left-hand side expression, keep the Expr AST/HIR/MIR/etc node around (see http://is.gd/MoTsk5 for examples where this would error today)
  • For each at the earliest point in the compiler passes where it becomes possible,
    • check that the EXPR is of a valid "destructuring assignment" form and otherwise emit a useful error message like the current "invalid left-hand side" one.
    • check that the bindings/variables mentioned in it have the right types to assign to
    • translate it to code that destructures and assigns each value regularly.

This would mean not using any of the actual pattern matching parser/compiler parts, at least not inherently, but that seems fine since it wouldn't really need most of it, since it would be restricted as if it where a irrefutable pattern with only by_value bindings. And if needed it could still masquerade as pattern matching through error messages and docs.

@burdges
Copy link

burdges commented Feb 27, 2019

We worried about new declarations being intermixed with mutability too, but this RFC proposes only assignments to previously declared values, so no problems.

@Centril Centril added the A-tuples Proposals relating to tuples. label Feb 27, 2019
@ExpHP
Copy link

ExpHP commented Feb 27, 2019

If the LHS is an expression, then as noted in that issue we need to make _ and , .. valid in expressions.

(b[0], _) = a

(note: the linked comment also had a ref in there but I can't picture what that's supposed to mean in this context)

@ExpHP
Copy link

ExpHP commented Feb 27, 2019

As it stands, this RFC is far too incomplete to merit discussion. Reading through it raised dozens of questions that I can see were already brought up in past discussions that the author is aware of.

Compressing those discussions into their key points and making decisions off of them is the responsibility of the RFC author. Otherwise history repeats itself and we're all just talking in circles again.

@alexreg
Copy link

alexreg commented Feb 27, 2019

I'm struggling to see why this wouldn't be LL(k). Even so, we haven't yet decided whether that's a desirable property to uphold a) rigidly, b) mainly, but with some exceptions, c) not at all. I guess there's some controversy in WG-grammar about it.

@Centril
Copy link
Contributor

Centril commented Feb 28, 2019

I guess there's some controversy in WG-grammar about it.

Not to my knowledge.

@Walther
Copy link
Author

Walther commented Feb 28, 2019

If the LHS is an expression, then as noted in that issue we need to make _ and , .. valid in expressions.

(b[0], _) = a

(note: the linked comment also had a ref in there but I can't picture what that's supposed to mean in this context)

Do these comments in the thread address your concerns?

@ExpHP
Copy link

ExpHP commented Feb 28, 2019

@alexreg when people said it wouldn't be LL(k), it was when the LHS was proposed to be a pattern. If the LHS is parsed as an expression instead (which seems more versatile anyways, so long as we add e.g. _ to expression grammar), then there is no issue.

In particular, the problem with parsing it as a pattern is that these have completely different parse trees despite only differing in the last line.

Foo {
    bar: x,
    baz: y,
}.method()

Foo {
    bar: x,
    baz: y,
} = expr;

@ExpHP
Copy link

ExpHP commented Feb 28, 2019

@Walthier: These aren't my concerns, I was showing that there was more to the discussion than what you quoted.

As for those links, the first is useful in suggesting that this could be implemented as a syntactic transform. This could make a decent start for reference-level description if the transform really works.

The other two don't appear to add anything new to the conversation.

@ExpHP
Copy link

ExpHP commented Feb 28, 2019

While we're at it, allow me to add something new not in that conversation:

(a, b, ..) = (1, 2, 3, 4);

The meaning of .. seems to be very different here versus its usual meaning in an expression. Can it be supported or will it once again complicate the parser? (you can't nest subexpressions in .. so I think it might be fine?)

@alexreg
Copy link

alexreg commented Feb 28, 2019

@ExpHP Aha. Thanks for clarifying. That makes sense.

@withoutboats
Copy link
Contributor

@rfcbot fcp postpone

We discussed this in the lang team meeting today and overall we consider this a nice-to-have feature. However, there's a fair bit of work figuring out the exact semantics of destructuring reassignment to make sure there's no surprising behavior.

We're in the process of setting up language team working groups, allowing members of the lang team to work with interested members of the community to solve targeted problems. This seems to us like an ideal task for an ergonomics working group - a relatively uncontroversial improvements to the ergonomics of the language that mostly just needs some time and attention from people to hash out the finer details. We thought we should postpone the RFC until a working group is set up, and then that working group can work on this idea and post a new RFC with all of the details.

I probably don't personally have much time to work on setting up an ergonomics working group, but I'd be glad to see other people doing this work. If anyone on the lang team interested in being involved on this, please post your interest and hopefully you can work with @Walther and others on getting this going.

@rfcbot
Copy link
Collaborator

rfcbot commented Mar 7, 2019

Team member @withoutboats has proposed to postpone this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-postpone This RFC is in PFCP or FCP with a disposition to postpone it. labels Mar 7, 2019
@joshtriplett
Copy link
Member

I'd be interested in being part of any ergonomics group, and working with whoever ends up starting it.

@alexreg
Copy link

alexreg commented Mar 7, 2019

@joshtriplett @Centril seems like a good person to get involved on the design-side, but I don't want to actually volunteer him, because I know he's already a busy guy. I've been involved with implementing a bunch of ergonomics-related RFCs/features lately, so I could possibly contribute too.

@rfcbot rfcbot added final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. and removed proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. labels Mar 27, 2019
@rfcbot
Copy link
Collaborator

rfcbot commented Mar 27, 2019

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added the finished-final-comment-period The final comment period is finished for this RFC. label Apr 6, 2019
@rfcbot
Copy link
Collaborator

rfcbot commented Apr 6, 2019

The final comment period, with a disposition to postpone, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC is now postponed.

@rfcbot rfcbot added postponed RFCs that have been postponed and may be revisited at a later time. and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. disposition-postpone This RFC is in PFCP or FCP with a disposition to postpone it. labels Apr 6, 2019
@rfcbot rfcbot closed this Apr 6, 2019
@kartva
Copy link

kartva commented Dec 8, 2020

Can we now formally close the issue as this now being discussed over here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-patterns Pattern matching related proposals & ideas A-syntax Syntax related proposals & ideas A-tuples Proposals relating to tuples. finished-final-comment-period The final comment period is finished for this RFC. postponed RFCs that have been postponed and may be revisited at a later time. T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.