Change the &mut T pattern to &mut.

[huonw]

No description provided.

[netvl]

Increasing consistency is always great. +1

BTW, isn't the fact that x in let &mut x = ... is mutable is just a consequence of parsing it as let &(mut x) = ... (parentheses are for the emphasis only)? This certainly can be surprising; &mut mut x is much better.

[huonw]

Yes, that's exactly what causes that.

[glaebhoerl]

+1 for the same reasons mentioned in the proposal.

[pnkfelix]

another potential "drawback" (though certainly some might see it as a feature of the RFC, not a drawback): a macro that expands into a pattern &id can currently do so without knowing whether the input value is a &T or a &mut T. With this change in place, such a macro would need to handle the two cases separately.

[huonw]

(Updated from feedback.)

[lilyball]

👍

[Kimundi]

+1 from me, though for consistency it would be nice if supporting &(mut T) would still be possible somehow, eg by adding explicit parens to disambiguate.

[pnkfelix]

+1 to Kimundi's suggestion (which is mentioned in passing in the RFC Drawbacks section where it says that "disambiguating like &(mut x) doesn't yet work"). I'm not so sure I would be happy if we forced people to write rebindings via let mut x = x as proposed in the current RFC draft. But then again maybe I would live with it.

[lilyball]

Agreed, I'd also like to see &(mut x) work.

[huonw]

rust-lang/rust#11144

I found exactly one instance of &mut ... being &(mut ...) in the whole codebase, via git grep '\bfor.*&mut.*\bin\b\|\blet\b[^:=;]*&mut, so I think this is rare enough not be a huge consideration for RFC atm (i.e. no more contentless responses about it, please :) ).

[huonw]

(Note to self: it was decided that I would collect a bit more data about how often &mut is matched on.)

[nikomatsakis]

I remain divided on this. On the one hand, I recognize that this has the potential to be a future wart, and it has only a minor impact on existing code.

On the other hand, it just seems repetitive to have to write let &mut (ref mut x, ref mut y) = tuple (or something like that). Moreover, in every other part of Rust, we allow &mut to be invisibly used where & whenever we can (i.e., reborrowed and frozen). So it would be legal to do e.g. let &mut (ref x, ref y) = tuple, in which case the mut, while true, is less relevant (since in fact the pointer is frozen by taking immutable refs into it).

Finally, I was thinking about the plans for the box pattern, and it seems like a similar concern would arise there, but one that is not easily rectified. The current plan was to extend box so that it could be applied to any value which implements the Deref trait (which, incidentally, would presumably include &T as well!). This means that box patterns would have a similar imprecision, where the pattern form does not precisely match the type of the input.

For example, one could write:

let x = box(Rc) somevalue;
match x {
    box somepat => { ... }
}

Now, we could (maybe?) force you to write box(Rc) in the pattern, but (a) that again gets fairly wordy and (b) I'm not actually sure if that will work -- box the expression uses Rc as an expression to get at something that implements the box method, but box as a pattern would presumably reference a type. Now, usually, Rc will be punned as both a type and value name, but that is not necessary.

I see a couple of options:

• Live with it. Keep system as currently planned. In general, then, patterns are not required to precisely specify type of what they match.
• Fix &, leave box. Basically what the RFC proposes. Everything but box pattern precisely specifies type of what is matches.
• Embrace the pun. Remove both box and & patterns and have a single deref pattern of some kind or another.
• Use the type name with box pattern. Long and unrealiable, as I wrote above.
• Generalized unapply. Move away from the idea of a single box pattern and instead have some way to write (e.g.) Rc(value) where Rc is a reference to a constant that implements the Unapply trait in some way. I have no particular design planned out and don't really want to make one at this time because that seems like a lot of work. It also means that box(Rc) expr would not be analogous to the destructuring form.

[nikomatsakis]

(I should note though that the box proposal does not share the parser ambiguity hazard, so perhaps it is not so serious. In any case, we already intended to write up the full details of the box proposal in an RFC sooner rather than later, and this just suggests to me that that needs to get done ASAP.)

[glaebhoerl]

For a while early on, I kept writing &foo (rather than &mut foo) when intending to create an &mut reference, partly because of C/C++ habits, but partly because I thought that & was just an operator and the type of reference (shared or mut) would be determined by type inference. But if that's not the case (which it's not), and the correct analogy is to think of & and &mut as data constructors for their respective types, then I feel we should be consistent and stick to the same logic in patterns, i.e. adopt this proposal.

(Of the alternatives, a single unified deref pattern also sounds appealing, if there's some satisfying syntax we could use for it, which I'm not sure of.)

[nikomatsakis]

On Sat, Aug 09, 2014 at 08:48:57AM -0700, Gábor Lehel wrote:

I feel we should be consistent and stick to the same logic in
patterns, i.e. adopt this proposal. ... (Of the alternatives, a
single unified deref pattern also sounds appealing, if there's some
satisfying syntax we could use for it, which I'm not sure of.)

Having thought about it for a bit, I think you expressed my current
sentiments perfectly.

[pczarn]

+1, find the bug in this code:

let mut foo = 1u;
let bar: |&mut uint| = |&mut n| n += 100;
bar(&mut foo);
foo

One data point is not much, but someone came with this issue to the IRC.

[ftxqxd]

👍 It’s great to see a proposal that makes patterns mirror their expression counterparts more closely. One of my favourite things about pattern matching (usually regardless of language) is that in an assignment, you can usually modify each side in the same way and not change the behaviour (unless the borrow checker or refutability gets in your way):

let x = 1i;
assert_eq!(x, 1i);
let &x = &1i;
assert_eq!(x, 1i);
let TupleStruct(&x) = TupleStruct(&1i);
assert_eq!(x, 1i);
// With this RFC:
let &mut TupleStruct(&x) = &mut TupleStruct(&1i);
assert_eq!(x, 1i);

// You can even work in reverse to ‘solve’ an assignment like a mathematical equation:
let &mut TupleStruct(&x) = &mut TupleStruct(&1i);
let      TupleStruct(&x) = TupleStruct(&1i);
let                   &x = &1i;
let                    x = 1i;

It’s sad to see that discussion on this RFC has stalled, though—what exactly would it take to get this RFC to move further through the RFC process? This is a backwards-incompatible change, after all.

[huonw]

Oh, whoops, forgot about this. I'll try to make some measurement and discuss with people tomorrow.

[huonw]

Here's the things that were detected in stage1 of a plain make:

src/libcore/iter.rs:2397         let &(ref mut f, ref mut val, ref mut first) = st;
                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/libcollections/bit.rs:1022         let &BitvSet(ref mut self_bitv) = self;
                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~
src/libcollections/bit.rs:1137         let &BitvSet(ref mut bitv) = self;
                                           ^~~~~~~~~~~~~~~~~~~~~~
src/libcollections/bit.rs:1190         let &BitvSet(ref mut self_bitv) = self;
                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~
src/libcollections/bit.rs:1228         let &BitvSet(ref mut bitv) = self;
                                           ^~~~~~~~~~~~~~~~~~~~~~
src/libcollections/bit.rs:1511         let &BitvSet(ref mut bitv) = self;
                                           ^~~~~~~~~~~~~~~~~~~~~~
src/libcollections/bit.rs:1566         let &BitvSet(ref mut bitv) = self;
                                           ^~~~~~~~~~~~~~~~~~~~~~
src/libcollections/bit.rs:1578         let &BitvSet(ref mut bitv) = self;
                                           ^~~~~~~~~~~~~~~~~~~~~~
src/libstd/comm/stream.rs:343             Some(&GoUp(..)) => {
                                               ^~~~~~~~~
src/libstd/comm/stream.rs:372                     Some(&GoUp(..)) => {
                                                       ^~~~~~~~~
src/libstd/comm/stream.rs:462                 Some(&GoUp(..)) => {
                                                   ^~~~~~~~~
src/libsyntax/ast_map/mod.rs:82         let &Values(ref mut items) = self;
                                            ^~~~~~~~~~~~~~~~~~~~~~
src/librustc_typeck/check/mod.rs:1636         for (_, &ref ty) in self.inh.node_types.borrow_mut().iter_mut() {
                                                      ^~~~~~~
src/librustc_driver/pretty.rs:331:13: 331:47 note: & pattern with mut
src/librustc_driver/pretty.rs:331             &NodesMatchingDirect(ref mut iter) => iter.next(),
                                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/librustc_driver/pretty.rs:332             &NodesMatchingSuffix(ref mut iter) => iter.next(),
                                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

And here's what was detected in the crates in servo that use their plugin crate:

dom/node.rs:221             &None => None,
                            ^~~~~
dom/node.rs:222             &Some(ref mut layout_data) => Some(layout_data.chan.take().unwrap()),
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~
script_task.rs:640             &Some((_, ref mut needs_reflow)) => *needs_reflow = true,
                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
script_task.rs:641             &None => (),
                               ^~~~~
fragment.rs:569             &ScannedTextFragment(ref mut info) => {
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
fragment.rs:580             &ScannedTextFragment(ref mut info) => {
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
inline.rs:658                     (&ScannedTextFragment(ref mut left_info),
                                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
text.rs:163             let &NewLinePositions(ref mut new_line_positions) =
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
wrapper.rs:917             &Some(ref mut layout_data) => layout_data.data.restyle_damage = damage,
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~
wrapper.rs:936             &Some(ref mut layout_data) => layout_data.data.flags.insert(new_flags),
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~
wrapper.rs:945             &Some(ref mut layout_data) => layout_data.data.flags.remove(flags),
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~
css/matching.rs:605             &None => panic!("no layout data"),
                                ^~~~~
css/matching.rs:606             &Some(ref mut layout_data) => {
                                ^~~~~~~~~~~~~~~~~~~~~~~~~~

I.e. not that many. (Those are using the type checker so should detect everything in the crate, modulo conditional compilation.)

[huonw]

(Implemented here, staging means that most of the library changes cannot be done yet, but there is two instances of the &mut ident parsed as &(mut ident) ambiguity that had to be changed... but only two.)

[brson]

Seems to be consensus. Merged. Tracking.