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

Ref/& Destructuring Inconsistency in Structs #2819

Open
quantatic opened this issue Nov 17, 2019 · 24 comments
Open

Ref/& Destructuring Inconsistency in Structs #2819

quantatic opened this issue Nov 17, 2019 · 24 comments
Labels
A-patterns Pattern matching related proposals & ideas T-lang Relevant to the language team, which will review and decide on the RFC.

Comments

@quantatic
Copy link

Rust pattern matching and destructuring allows the use of & and ref.

In the example below, the use of these is intuitive.

fn main() {
    let a: (u32) = (1234);
    let (ref ref_a) = a;

    let b: (&u32) = (&1234);
    let (&deref_b) = b;
}

In this case, we end up with ref_a of type &u32, and deref_b of type u32. Simple enough -- we can go both ways with both ref and & in pattern matching.

In the instance with structs, this consistency breaks down when matching struct field names. In the example that follows, this is shown:

struct Borrowed {
    val: u32
}

fn main() {
    let a = Borrowed {
        val: 1234
    };

    let Borrowed {ref val} = a;
}

In this case, val is borrowed as &u32, mirroring tuple functionality. The following example on the other hand, does not work.

struct Derefed<'a> {
    val: &'a u32
}

fn main() {
    let a = Derefed {
        val: &1234
    };

    let Derefed {&val} = a; // error: expected identifier, found `&`
    let Derefed {val: &val} = a; // works as expected
}

While not functionality-breaking, it is extremely non-intuitive that this matching only works via ref and not via & for structs, when binding directly to the same variable name.

@jonas-schievink jonas-schievink added A-patterns Pattern matching related proposals & ideas T-lang Relevant to the language team, which will review and decide on the RFC. labels Nov 17, 2019
@hadronized
Copy link
Contributor

Capturing and destructuring are two different concepts. let Derefed { &val } = a is not a valid syntax for destructuring because val is a field. You cannot destructure a field the way you seem to want, for a very simple reason: imagine that the field was not a reference but an Option<_>. You wouldn’t use such a syntax, but more:

struct Foo {
    x: Option<u32>
}

fn main() {
  let f = Foo { x: Some(123) };
  
  if let Foo { x: Some(ref inner) } = f {
    println!("{}", inner);
  }
}

Why would it be any different for references?

@Pauan
Copy link

Pauan commented Nov 18, 2019

@phaazon I think you're misunderstanding their request. They are aware that & and ref are opposites.

What they are asking for is that the field short-hand syntax should be extended to support references.

Right now you can use the field short-hand syntax, such as Foo { bar }, which is equivalent to Foo { bar: bar }.

But that short-hand syntax does not work with references, so you cannot use Foo { &bar }, which would be equivalent to Foo { bar: &bar }.

@hadronized
Copy link
Contributor

Ah, yeah, then I’m still against it. 😄 If that gets accepted, in five months from now, someone will complain they can’t do Foo { Some(bar) or something along these lines.

@quantatic
Copy link
Author

Thank you for the clarification @Pauan -- apologies if my initial title was unclear/ambiguous.

Perhaps I'm unclear on the official naming convention, but as I understand it, the following example:

struct Test {
    a: i32
}

fn main() {
    let test = Test {
        a: 1234,
    };

    let Test {ref a} = test; //a is of type &i32 here.
}

Is an example of de-structuring the Test struct, and assigning the local variable a to the borrowed a field from the struct.

Currently, this short-hand syntax supports borrowing, but not de-referencing. To me, this seems unintuitive: almost everywhere else that a ref can be used for pattern matching/destructuring to borrow, a & can be used to de-reference.

I understand why this may confuse newcomers into believing this would work for an Option or similar enum, but even in that case, it is unclear to me why the fact that this currently supports ref is not be equally confusing.

@quantatic quantatic changed the title Ref/& Pattern Matching Inconsistency in Structs Ref/& Destructuring Inconsistency in Structs Nov 20, 2019
@Pauan
Copy link

Pauan commented Nov 20, 2019

@quixotrykd Actually, you can already use & with Option (and any other enum):

let a = 5;

if let Some(ref b) = Some(a) {}
if let Some(&b) = Some(&a) {}

let ref b = Some(a);
let &b = &Some(a);

Using & works anywhere that a pattern works. So you are right that there is some inconsistency with struct fields.

@hadronized
Copy link
Contributor

You are right about the fact that & can be used anywhere a pattern works. In let _ = _; the left _ is a pattern. Some(&b) is a pattern. Some(ref b) is a pattern (the Some(_)) but ref b states how we want to capture what we destructured in the pattern.

Deref { _ } is a pattern. However, what you put in the _ must be list of field-patterns. a: b is one and a is a short hand for a: a.

let Derefed {&val} = a; // error: expected identifier, found `&`

Here, &val is a pattern, not a field-pattern. If you want to make it a shorthand syntax for setting val as what’s behind the reference (i.e. &val), then since &val is a pattern, you’re also opening the gate to accepting any pattern here, such as:

struct Foo {
  a: Option<u32>
}

let Foo { Some(a) } = this_doe_not_make_sense;

Again, accepting &a here would be an exception — because &a means to destructure a field that is behind a reference and capturing its content at the same time — and is actually weirder than what you seem to think. What would you do with the Option example I gave you above?

@Pauan
Copy link

Pauan commented Nov 20, 2019

@phaazon Nobody is suggesting Foo { Some(a) } (and I would personally be against it if they did, that proposal makes no sense).

As for the inconsistency, I think you are misunderstanding something. Foo { _ } is not valid, because structs do not accept patterns, they only accept these special hard-coded things:

Foo { bar: <pattern> }
Foo { bar }          // equivalent to Foo { bar: bar }
Foo { ref bar }      // equivalent to Foo { bar: ref bar }
Foo { mut bar }      // equivalent to Foo { bar: mut bar }
Foo { ref mut bar }  // equivalent to Foo { bar: ref mut bar }
Foo { .. }           // ignores the fields

As you can see, structs do not use patterns, they have their own special struct-only syntax.

Notice that you can use Foo { ref bar } (which is equivalent to Foo { bar: ref bar }). This is using the field shorthand syntax, but with a ref (or mut) modifier. This syntax is an exception, it is special struct syntax, it is not a pattern.

So since we already have an exception for ref and mut, there is no problem with adding in another exception for &. And the behavior of Foo { &bar } is crystal clear, it behaves exactly like Foo { bar: &bar }.

And there is no risk of the slippery slope of "what if people ask to add in more exceptions later?" because there aren't any more exceptions: none of the other patterns make sense in field position, so & is the only field pattern currently missing from structs.

@hadronized
Copy link
Contributor

My point was: if you add Foo { &bar }, since &bar is a pattern, you’re adding more inconsistency, because if we can pattern-match on &bar, it would feel consistent to try to pattern match on Option<_>. That’s why I don’t really like that proposal and also why I don’t agree with the OP that the current situation is inconsistent.

@Pauan
Copy link

Pauan commented Nov 21, 2019

@phaazon As I explained, we already have ref and mut as special patterns which work on struct fields. So adding in &bar is not making things more inconsistent than the status quo. On the contrary, it fills in the only missing pattern, since none of the other patterns make sense.

And I have found situations where & would have been useful in practice, so this isn't just a hypothetical benefit.

@Centril
Copy link
Contributor

Centril commented Nov 21, 2019

So adding in &bar is not making things more inconsistent than the status quo. On the contrary, it fills in the only missing pattern, since none of the other patterns make sense.

I disagree with this perspective and agree with @phaazon. If you look at the pattern grammar (and indeed the implementation), you will find that we have:

pat ::=
  | "&" "mut"? pat
  | "ref"? "mut"? ident
  | other_stuff_in_pat
  ;

As you can see, the non-terminal in the case of & is pat whereas it is ident in the case of ref.
In field short-hand patterns, you can also use the "ref"? "mut"? ident grammar. However, the proposal here is to allow "&" "mut"? ident. The non-terminal here is inconsistent with the non-short-hand version, which allows full-blown patterns.

@Pauan
Copy link

Pauan commented Nov 22, 2019

@Centril That is true, but I personally don't view that as being a problem or an inconsistency, because field patterns are already a subset of patterns, so there's no issue with having "&" "mut"? ident which is a subset of "&" "mut"? pat.

Or to put it another way, ident, "ref" ident, "mut" ident (etc.) are patterns, but they are also field patterns, and the behavior of field patterns is different than the behavior of patterns. So the same would be true for &.

So I find the objection of "it only allows a subset of patterns" to be weak, since that's already true today.

@Centril
Copy link
Contributor

Centril commented Nov 22, 2019

The desirable property in the record short-hand is that it reuses the grammar from "ref"? "mut"? ident exactly (and so it is complete from the perspective of covering binding patterns as a short-hand). My point was that the same cannot be said of "&" "mut"? ident. If you add that, there's no good reason from the POV of grammatical consistency not to allow &&x. I would however understand the argument "this is inconsistent, but ergonomics trumps consistency in this case".

@Pauan
Copy link

Pauan commented Nov 22, 2019

If you add that, there's no good reason from the POV of grammatical consistency not to allow &&x.

I agree, I see no problem with having &&x as well. The reference pattern currently allows that, so it makes sense to allow it in field pattern as well.

The only issue is making it recursive, so that &&&x (etc.) would work, but that is doable with a slightly more elaborate grammar.

I would however understand the argument "this is inconsistent, but ergonomics trumps consistency in this case".

To be clear, I am looking at the consistency from the perspective of the user, not the grammar or the implementation. So I view it as both ergonomic and consistent (or at least, not inconsistent, just a restricted subset). But yes, I do think it is an ergonomic improvement, that would have benefited some of my programs in the past.

@hadronized
Copy link
Contributor

hadronized commented Nov 22, 2019

There cannot be an inconsistency because the short-hand syntax doesn’t exist for items but struct / enum structs. I kinda feel like you find an inconsistency in Some(&x) vs Some { &x }, but those are two different things and obviously the latter is not accepted. I think the grammar stating that each item (separated by ,) inside the curly braces should be field-records, and a field record start with a field name. With &field, you’re adding a new rule in the grammar, which is not consistent with the rest of the language.

To be clear, I am looking at the consistency from the perspective of the user.

I think I apply, as you do, as a Rust user, and I truly and honestly don’t agree with you that Rust is inconsistent regarding the short-hand syntax and that authorizing a syntax that is either a short-hand syntax or a destructuring syntax (i.e. &x) is wrong and doesn’t fit a sound grammar. You see an ergonomics and I see a rabbit hole that will not really provide lots of interesting use cases (i.e. why not just stick to a: &a?!). After all, the short-hand syntax was invented to prevent some duplication. In your case, you don’t have the same thing on the lhs vs. rhs. Furthermore, if we were to accept that change, I can already see waves of people complaining about the fact they cannot do the same thing with an item that implements Deref, or behind a Rc or why not raw *const.

@Pauan
Copy link

Pauan commented Nov 29, 2019

@phaazon I kinda feel like you find an inconsistency in Some(&x) vs Some { &x }, but those are two different things and obviously the latter is not accepted.

No, I have never confused those. In my previous post I even argued specifically that struct field patterns are special and different from regular patterns.

I think the grammar stating that each item (separated by ,) inside the curly braces should be field-records, and a field record start with a field name. With &field, you’re adding a new rule in the grammar, which is not consistent with the rest of the language.

But that's also true with ref foo (and mut foo), which is currently accepted. So how is &foo any different?

authorizing a syntax that is either a short-hand syntax or a destructuring syntax (i.e. &x) is wrong and doesn’t fit a sound grammar.

Then are you against ref foo and mut foo (which are currently accepted)?

(i.e. why not just stick to a: &a?!)

That same argument can be used for ref foo and mut foo (why not just stick to foo: ref foo and foo: mut foo?)

After all, the short-hand syntax was invented to prevent some duplication. In your case, you don’t have the same thing on the lhs vs. rhs.

That is also true for ref foo, since it is equivalent to foo: ref foo. So how is &foo being equivalent to foo: &foo any different?

Furthermore, if we were to accept that change, I can already see waves of people complaining about the fact they cannot do the same thing with an item that implements Deref, or behind a Rc or why not raw *const.

I don't really understand what you're saying. That seems like a red herring. The suggestion is to have &foo desugar to foo: &foo, so it will have the same semantics that foo: &foo has right now.

That's completely separate from whether the &foo pattern should work with Deref or Rc or *const.


Let me explain my position very clearly. Right now you can use ref foo in struct field patterns:

Foo { ref foo }

This is not a pattern, it is special syntax only for struct fields. It desugars to Foo { foo: ref foo }.

So this proposal is simply to add in Foo { &foo }, which is special syntax only for struct fields, and which desugars to Foo { foo: &foo }. In other words, it's the exact same situation as ref foo.

It's as simple as adding a new case to the StructPatternField grammar (which is where ref foo and mut foo are defined for structs). The grammar is completely sound.

Let's look at the arguments for why we should not add in &foo:

  • It isn't a pattern. But that's also true for ref foo and mut foo.

  • It is a special case. But that's already true for ref foo and mut foo.

  • The left-hand-side doesn't match with the right-hand-side. But that's already true for ref foo and mut foo.

  • It is restricted to identifiers only (which is @Centril 's point). This is true, but it just means that it's a subset of patterns, which is fine since field patterns aren't patterns, and field patterns are already a subset of patterns.

  • It will lead to a slippery slope of other suggestions. But that also applies to ref foo and mut foo. And in addition to that, the &foo syntax is the only missing pattern which makes sense for fields, the other patterns don't make sense, so there won't be any slippery slope.

All of the arguments against &foo also apply to ref foo. All of the arguments in favor of ref foo also apply to &foo. So since they are the same, why do we have one but not the other? That is the inconsistency that I have been referring to.

So you will need to make an argument for why ref foo is okay but &foo is not. Thus far you have not done so.

@hadronized
Copy link
Contributor

hadronized commented Nov 30, 2019

ref foo is okay because, as opposed to what you said, it doesn’t do as much as &foo would do. The reason for this is that ref foo doesn’t destructure anything. foo still remains the same, but we capture it as a ref. This is not true for &foo, which destructures the field. This is also why I said this:

Furthermore, if we were to accept that change, I can already see waves of people complaining about the fact they cannot do the same thing with an item that implements Deref, or behind a Rc or why not raw *const.

Regarding your reply:

I don't really understand what you're saying. That seems like a red herring. The suggestion is to have &foo desugar to foo: &foo, so it will have the same semantics that foo: &foo has right now.

That's completely separate from whether the &foo pattern should work with Deref or Rc or *const.

Everybody got what the suggestion is about. And I didn’t say that &foo should work with Rc. I said that people will start complaining about that fact that if you add a short-hand syntax to destructure a field, then it should be doable to destructure a pointer:

struct Foo {
  bar: *const u32
}

let Foo { *bar } = foo();

That would desugar to

let Foo { bar: *bar } = foo();

@RustyYato
Copy link

RustyYato commented Nov 30, 2019

I said that people will start complaining about that fact that if you add a short-hand syntax to destructure a field, then it should be doable to destructure a pointer:

Given that we can't destructure any pointer except for references right now, I don't see that happening.

@spunit262
Copy link

Foo { bar: &bar } only works if bar is Copy, that greatly limits the usefulness of Foo { &bar }. A general solution to ergonomically using refs to Copy types would be much better. The was a tiny bit of talk about copy ergonomic in #2005 (comment) , but I don't know of any other discussion of the topic.

@Pauan
Copy link

Pauan commented Dec 3, 2019

@spunit262 That's also true for &foo in general patterns, so how is that an argument against having &foo for structs?

A new feature for using non-Copy types might be useful, but it's separate from this feature request (which is solely about syntax sugar for an already existing feature, it's not a new feature). And any solution for non-Copy types should of course apply equally to both general patterns and struct patterns.

@spunit262
Copy link

That's also true for &foo in general patterns, so how is that an argument against having &foo for structs?

No it's not, &Some(ref foo) is valid for nonCopy and used to be required before #2005 , but &ref foo is just silly, even if technical valid.

A new feature for using non-Copy types might be useful, but it's separate from this feature request

The point is I don't see much use for this, and the bit I do see, is better served by something else.

@Pauan
Copy link

Pauan commented Dec 4, 2019

@spunit262 Using &foo only works for Copy types, and that is what this issue is about. I don't know why people keep bringing up general patterns (like Some) which aren't applicable to fields (which only accept identifiers).

The point is I don't see much use for this, and the bit I do see, is better served by something else.

That's fair, but obviously some people (such as the original poster) disagree. And this syntax sugar does not prohibit other new features from being added (such as improvements for non-Copy types).

@hadronized
Copy link
Contributor

Imagine this:

let &x = foo_that_returns_a_ref();

All the concepts behind this line are the same as the ones for this:

let Some(ref x) = foo_that_returns_an_option();

It’s plain pattern-matching. However, the x in Some(ref x) is a binding to what’s inside Some(_). Now:

let Foo { ref x } = foo();

Is still destructuring a Foo without destructuring its x field: we just bind it to a local x. However, this:

let Foo { &x } = foo();

Would imply destructuring both Foo AND its field. You might say whatever you want, that’s destructuring. So you’re adding a special syntax to destructure a field. Let’s imagine that we merge that. Why it doesn’t make sense is that there’s no identifier binding for the field to bind here. It’s like magic and that kind of magic is hurting IMHO. With Foo { ref s }, s is the name of a field and ref just says “just capture that field by ref”. With Foo { &s }, s is not the name of a field. It’s the content of a field named s, which is behind a reference. That is very confusing and you are wrong stating that Rust already has that and adding it would remove inconsistency (it would add to me).

Why we keep coming to Option is that either people will have to deal with a weird exception for references or they will think that since they can destructure &x, they should be able to destructure Some(x) too. And you can clearly see how weird it feels: what’s the field name? What’s the meaning? You said we cannot pattern-patch pointer-like types but that’s wrong: you can match on a Box<_>. So Foo { box x } should work too (and I’m against it for the same reasons).

I can clearly feel that you would like to have that feature to save some characters to type (i.e. &x instead of x: &x), but I think the bikeshedding is not worth it. You think it’s a good idea and I respect that, but I also think it’s going to add a lot of inconsistencies and you haven’t commented on that yet.

I think the syntax to destructure a field should stay x: ….

@Pauan
Copy link

Pauan commented Dec 5, 2019

Is still destructuring a Foo without destructuring its x field

That is incorrect. Foo { ref x } is syntax sugar for Foo { x: ref x }, which destructures the x field and then creates a new x binding which is a new reference to the x field. It is destructuring the field.

Why it doesn’t make sense is that there’s no identifier binding for the field to bind here. It’s like magic and that kind of magic is hurting IMHO.

As I have explained many times, Foo { &x } is equivalent to Foo { x: &x }, so it is creating an x binding, just the same as Foo { ref x }.

There is no new feature, and there is no magic, only syntax sugar for an already existing feature, just the same as how Foo { ref x } is syntax sugar for Foo { x: ref x }

s is the name of a field and ref just says “just capture that field by ref”. With Foo { &s }, s is not the name of a field. It’s the content of a field named s, which is behind a reference.

I'm not sure where you got that idea. In both cases s is the name of the field. And in both cases it's destructuring the field. And in both cases it's creating a new identifier.

The only difference is that in the case of ref it creates a new reference to the field, and in the case of & it unwraps the reference which is in the field. So they are perfectly symmetrical, and they do the same amount of work.

Your argument is the same as saying that Foo { ref x } is bad because it does more work than Foo { x }.

you can match on a Box<_>. So Foo { box x } should work too (and I’m against it for the same reasons).

That's a good point, box x should work as well, I missed that. Once again: why are you in favor of ref x but against box x? They are the same thing, the only difference is that one creates a new reference and one dereferences a reference. But so what? That's how patterns work.

Why we keep coming to Option is that either people will have to deal with a weird exception for references or they will think that since they can destructure &x, they should be able to destructure Some(x) too.

Your argument also applies to ref x. I don't know why you keep thinking of ref x as being special. It isn't. It is shorthand syntax for x: ref x. It destructures. It has all the same properties as &x. All the same arguments apply. You have not demonstrated any differences between them.

I also think it’s going to add a lot of inconsistencies and you haven’t commented on that yet.

I have clearly given arguments against all of your claims of inconsistency. You have failed to provide a single argument for why ref x and x are okay, but &x or box x is not okay.

Until you actually present an argument for why &x is bad but ref x is okay, I will not continue this discussion.

@hadronized
Copy link
Contributor

hadronized commented Dec 5, 2019

That is incorrect. Foo { ref x } is syntax sugar for Foo { x: ref x }, which destructures the x field and then creates a new x binding which is a new reference to the x field. It is destructuring the field.

It is not. You’re still “outside” of the field. A field has a name and a value. x: ref x just inspects the name and creates a reference to the value. The value is not destructured.

Until you actually present an argument for why &x is bad but ref x is okay, I will not continue this discussion.

Up to you, but you are the one wanting to make a change, not me. You should stop focusing on syntax and care more about semantics. It’s not because ref x -> x: ref x that we should authorize &x -> x: &x.

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 T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

No branches or pull requests

7 participants