Immutably and implicitly borrow all pattern ids for their guards (NLL only) #49870

pnkfelix · 2018-04-11T10:38:08Z

This is an important piece of #27282.

It applies only to NLL mode. It is a change to MIR codegen that is currently toggled on only when NLL is turned on. It thus affect MIR-borrowck but not the earlier static analyses (such as the type checker).

This change makes it so that any pattern bindings of type T for a match arm will map to a &T within the context of the guard expression for that arm, but will continue to map to a T in the context of the arm body.

To avoid surfacing this type distinction in the user source code (which would be a severe change to the language and would also require far more revision to the compiler internals), any occurrence of such an identifier in the guard expression will automatically get a deref op applied to it.

So an input like:

let place = (1, Foo::new());
match place {
  (1, foo) if inspect(foo) => feed(foo), 
  ...  
}

will be treated as if it were really something like:

let place = (1, Foo::new());
match place {
   (1, Foo { .. }) if { let tmp1 = &place.1; inspect(*tmp1) }
                   => { let tmp2 = place.1; feed(tmp2) },
   ...
}

And an input like:

let place = (2, Foo::new());
match place {
    (2, ref mut foo) if inspect(foo) => feed(foo),
    ... 
}

will be treated as if it were really something like:

let place = (2, Foo::new());
match place {
    (2, Foo { .. }) if { let tmp1 = & &mut place.1; inspect(*tmp1) }
                    => { let tmp2 = &mut place.1; feed(tmp2) },
    ... 
}

In short, any pattern binding will always look like some kind of &T within the guard at least in terms of how the MIR-borrowck views it, and this will ensure that guard expressions cannot mutate their the match inputs via such bindings. (It also ensures that guard expressions can at most copy values from such bindings; non-Copy things cannot be moved via these pattern bindings in guard expressions, since one cannot move out of a &T.)

rust-highfive · 2018-04-11T10:38:13Z

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

pnkfelix · 2018-04-11T10:38:27Z

r? @nikomatsakis

pnkfelix · 2018-04-11T10:39:12Z

cc @arielb1 (normally I wouldn't ping someone who I think is quite busy with other parts of their life, but I've seen ariel chiming in on other PR's and I think he may be interested in doing so on this one.)

nikomatsakis

Did a first pass -- looking good -- but we need some tests, I suppose? (Or maybe I just didn't get to that commit, actually)

nikomatsakis · 2018-04-12T21:04:15Z

src/librustc_mir/build/block.rs

@@ -113,7 +114,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
                    // Declare the bindings, which may create a visibility scope.
                    let remainder_span = remainder_scope.span(this.hir.tcx(),
                                                              &this.hir.region_scope_tree);
-                    let scope = this.declare_bindings(None, remainder_span, lint_level, &pattern);
+                    let scope = this.declare_bindings(None, remainder_span, lint_level, &pattern,
+                                                      false);


Nit: a plain false like this is always a bit unclear to me as a reader. Maybe we could put a newtype here, like declare_bindings(..., InGuard(false)) (and struct InGuard(bool))?

Or I should probably just reuse the enum I introduced somewhere else in this PR...

(well, technically this boolean flag has a different meaning currently than the WithinGuard/OutsideGuard enum... but I'll figure something out. I too dislike plain booleans.)

nikomatsakis · 2018-04-12T21:09:05Z

src/librustc_mir/build/matches/mod.rs

+                        block, binding.var_id, binding.span, OutsideGuard);
+                    self.schedule_drop_for_binding(binding.var_id, binding.span, OutsideGuard);
+                    let rvalue = Rvalue::Ref(region, borrow_kind, binding.source.clone());
+                    self.cfg.push_assign(block, source_info, &local_for_arm_body, rvalue);


So before I thought we were going to need some sort of cast here -- I was trying to remember why -- I believe it was looking forward to the point where we try to maintain a shared borrow the value being matched over the whole match, in which case this first &mut borrow would be illegal...

okay let me look into this, see if I can make a test case that exposes the scenario you describe.

I believe it was looking forward to the point where we try to maintain a shared borrow the value being matched over the whole match, in which case this first &mut borrow would be illegal...

Isn't the answer here is "this is why we have 2-phase borrows"? There's this cute example, which compiles today, is not a soundness issue, and works in the 2phi-borrow interpretation, but does not work if 2phi is not used correctly.

EDIT: now example is a bit stronger.

use std::cell::Cell; fn main() { let mut b = 4; let a : Cell<Option<&u32>> = Cell::new(None); match b { ref mut btag if { a.set(Some(&*btag)); false } => { println!("[guard] a={:?} b={}", a, btag); } ref mut btag2 => { println!("[rest] a={:?} b={}", a, btag2); } } }

nikomatsakis · 2018-04-12T21:10:13Z

src/test/compile-fail/nll/issue-27282-guards-always-borrow.rs

+    match Some(&4) {
+        None => {},
+        mut foo if foo.is_some() && false => { foo.take(); () }
+        Some(s) => std::process::exit(*s),


never mind I see test now :)

the8472 · 2018-04-14T19:55:42Z

Does this solve rust-lang/rfcs#2036?

bors · 2018-04-17T17:00:12Z

☔ The latest upstream changes (presumably #49836) made this pull request unmergeable. Please resolve the merge conflicts.

pietroalbini · 2018-04-23T12:08:00Z

Ping from triage! Can @nikomatsakis (or someone else from @rust-lang/compiler) review this?

nikomatsakis · 2018-04-24T21:01:39Z

r=me once nits are addressed

arielb1 · 2018-04-25T12:58:17Z

src/librustc_mir/build/matches/mod.rs

+        //   MIR-borrowck did not complain with earlier (universally
+        //   eager) MIR codegen, laziness might not be *necessary*.
+
+        let autoref = self.hir.tcx().nll();


Why do we generate different MIR in NLL and non-NLL modes? Is this because we don't want to inject semantic changes in stable?

That was my assumption -- that said, thinking more about this, if we are going to try and merge NLL down into a warning period, we are going to need the MIR generation to be the same both with and without NLL enabled.

arielb1 · 2018-04-25T13:01:10Z

src/librustc_mir/build/matches/mod.rs

+        //   MIR-borrowck did not complain with earlier (universally
+        //   eager) MIR codegen, laziness might not be *necessary*.
+
+        let autoref = self.hir.tcx().nll();
        if let Some(guard) = candidate.guard {


Sorry for being late with this, but I feel that the "guard bit" mapping - figuring out whether a local should access the "arm" version or the dereference of the "guard" version - belongs more in HAIR than in MIR.

Hmm. I don't have a strong opinion, but that seems reasonable. How do you envision it working? I could imagine having e.g. distinct HAIR nodes for "reference variable from guard" or something...?

What I was thinking of was having variables in the HAIR refer to an enum

#[derive(Hash, PartialEq, Eq, Debug)] enum HairVar { Binding(ast::NodeId), GuardRef(ast::NodeId) }

MIR construction does not quite care about the exact NodeId of variables, only the scope of their definitions etc., so this should work out.

distinct HIR nodes for "reference variable from guard"

The above option would mean that we put it right in HAIR construction. Of course, we could also put it in resolution (using 2 separate NodeIds), but I understand the implications of that less.

(In case it wasn't clear, I meant "distinct HAIR nodes", I edited my comment -- but it sounds like we were thinking of similar things)

(at this point I would prefer to land this PR in the short term and come back later to the question of whether the guard bit mapping should part of the HAIR rather than the MIR... I will open an issue for that task once this has landed though...)

arielb1 · 2018-04-25T13:22:50Z

src/test/compile-fail/nll/issue-27282-guards-always-borrow.rs

+
+// Here is arielb1's basic example from rust-lang/rust#27282
+// that AST borrowck is flummoxed by:
+


This does not fix the core of the problem (which is that the match discriminant is not properly protected). For example, I don't think that this variant is prevented:

fn should_reject_destructive_mutate_in_guard() { let v = &mut Some(&4); match *v { None => {}, _ if { (|| { let bar = v; bar.take() })(); false } => { }, Some(s) => std::process::exit(*s), } }

The "discriminant protection" probably belongs in a separate PR, so I would just remove the "27282" tag from this test.

Yes, this was not intended to fix everything.

Just an FYI: I agree that the this did not put in the full fix to the core problem. However, the example outlined by @arielb1 is not accepted by the compiler (under NLL), even without this PR in place. In particular, you get these errors (playground):

error[E0382]: use of moved value: `*v` --> ../../ariel-example-2-27282.rs:11:9 | 8 | (|| { let bar = v; bar.take() })(); | -------------------------------- value moved here ... 11 | Some(s) => std::process::exit(*s), | ^^^^^^^ value used here after move error[E0382]: use of moved value: `v.0` --> ../../ariel-example-2-27282.rs:11:14 | 8 | (|| { let bar = v; bar.take() })(); | -------------------------------- value moved here ... 11 | Some(s) => std::process::exit(*s), | ^ value used here after move | = note: move occurs because `v.0` has type `&i32`, which does not implement the `Copy` trait

pnkfelix · 2018-05-02T15:22:36Z

I'm assuming that niko's r=me is retracted given arielb1's feedback.

(I will try to address that aforementioned feedback after I finish rebasing.)

…und in a mir-opt test.

…ed_value_does_not_live_long_enough`.

deref'ing such borrows within that guard. Review feedback: Add comment noting a point where we may or may not need to add a cast when we finish the work on rust-lang#27282. Review feedback: Pass a newtype'd `ArmHasGuard` rather than a raw boolean. Review feedback: toggle "ref binding in guards" semantics via specific method. (This should ease a follow-up PR that just unconditionally adopts the new semantics.)

guard expressions of matches (activated only when using new NLL mode). Review feedback: removed 27282 from filename. (The test still references it in a relevant comment in the file itself so that seemed like a reasonable compromise.)

pnkfelix · 2018-05-03T12:36:14Z

okay I've addressed the crucial feedback from arielb1, and I think I've done so in a way that does not perturb the PR too significantly... so I think I'll re-add niko's r+ once this passes travis... (famous last words...)

rust-highfive · 2018-05-03T13:33:14Z

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

travis_time:start:test_mir-opt
Check compiletest suite=mir-opt mode=mir-opt (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[00:57:47] 
[00:57:47] running 50 tests
[00:57:58] ERROR 2018-05-03T13:32:55Z: compiletest::runtest: None
[00:58:12] ...........................F......................
[00:58:12] 
[00:58:12] ---- [mir-opt] mir-opt/match_false_edges.rs stdout ----
[00:58:12] ---- [mir-opt] mir-opt/match_false_edges.rs stdout ----
[00:58:12]  thread '[mir-opt] mir-opt/match_false_edges.rs' panicked at 'Did not find expected line, error: Mismatch in lines
[00:58:12] Current block: None
[00:58:12] Actual Line: "        _7 = discriminant(_2);"
[00:58:12] Expected Line: "     _6 = discriminant(_2);"
[00:58:12] Test Name: rustc.full_tested_match.QualifyAndPromoteConstants.after.mir
[00:58:12] Expected:
[00:58:12] ... (elided)
[00:58:12]  bb0: {
[00:58:12] ... (elided)
[00:58:12]      _2 = std::option::Option<i32>::Some(const 42i32,);
[00:58:12]      _3 = discriminant(_2);
[00:58:12]      _6 = discriminant(_2);
[00:58:12]      switchInt(move _6) -> [0isize: bb6, 1isize: bb4, otherwise: bb8];
[00:58:12]  }
[00:58:12]  bb1: {
[00:58:12]      resume;
[00:58:12]  }
[00:58:12]  bb2: {  // arm1
[00:58:12]      StorageLive(_8);
[00:58:12]      _8 = _4;
[00:58:12]      _1 = (const 1i32, move _8);
[00:58:12]      StorageDead(_8);
[00:58:12]      goto -> bb13;
[00:58:12]  }
[00:58:12]  bb3: { // binding3(empty) and arm3
[00:58:12]      _1 = (const 3i32, const 3i32);
[00:58:12]      goto -> bb13;
[00:58:12]  }
[00:58:12]  bb4: {
[00:58:12]      falseEdges -> [real: bb9, imaginary: bb5]; //pre_binding1
[00:58:12]  }
[00:58:12]  bb5: {
[00:58:12]      falseEdges -> [real: bb12, imaginary: bb6]; //pre_binding2
[00:58:12]  }
[00:58:12]  bb6: {
[00:58:12]      falseEdges -> [real: bb3, imaginary: bb7]; //pre_binding3
[00:58:12]  }
[00:58:12]  bb7: {
[00:58:12]      unreachable;
[00:58:12]  }
[00:58:12]  bb8: {
[00:58:12]      unreachable;
[00:58:12]  }
[00:58:12]  bb9: { // binding1 and guard
[00:58:12]      StorageLive(_4);
[00:58:12]      _4 = ((_2 as Some).0: i32);
[00:58:12]      StorageLive(_7);
[00:58:12]      _7 = const guard() -> [return: bb10, unwind: bb1];
[00:58:12]  }
[00:58:12]  bb10: { // end of guard
[00:58:12]      switchInt(move _7) -> [false: bb11, otherwise: bb2];
[00:58:12]  }
[00:58:12]  bb11: { // to pre_binding2
[00:58:12]      falseEdges -> [real: bb5, imaginary: bb5];
[00:58:12]  }
[00:58:12]  bb12: { // bindingNoLandingPads.before.mir2 and arm2
[00:58:12]      StorageLive(_5);
[00:58:12]      _5 = ((_2 as Some).0: i32);
[00:58:12]      StorageLive(_9);
[00:58:12]      _9 = _5;
[00:58:12]      _1 = (const 2i32, move _9);
[00:58:12]      StorageDead(_9);
[00:58:12]      goto -> bb13;
[00:58:12]  }
[00:58:12]  bb13: {
[00:58:12] ... (elided)
[00:58:12]      return;
[00:58:12]  }
[00:58:12] Actual:
[00:58:12] fn full_tested_match() -> (){
[00:58:12]     let mut _0: ();
[00:58:12]     scope 1 {
[00:58:12]         let _4: i32;
[00:58:12]         let _5: &'<empty> i32;
[00:58:12]     scope 2 {
[00:58:12]         let _6: i32;
[00:58:12]     }
[00:58:12]     }
[00:58:12]     let mut _1: (i32, i32);
[00:58:12]     let mut _2: std::option::Option<i32>;
[00:58:12]     let mut _3: isize;
[00:58:12]     let mut _7: isize;
[00:58:12]     let mut _8: bool;
[00:58:12]     let mut _9: i32;
[00:58:12]     let mut _10: i32;
[00:58:12]     bb0: {                              
[00:58:12]         StorageLive(_1);
[00:58:12]         StorageLive(_2);
[00:58:12]         _2 = std::option::Option<i32>::Some(const 42i32,);
[00:58:12]         _3 = discriminant(_2);
[00:58:12]         _7 = discriminant(_2);
[00:58:12]         switchInt(move _7) -> [0isize: bb6, 1isize: bb4, otherwise: bb8];
[00:58:12]     bb1: {
[00:58:12]         resume;
[00:58:12]     }
[00:58:12]     }
[00:58:12]     bb2: {                              
[00:58:12]         StorageLive(_9);
[00:58:12]         _9 = _4;
[00:58:12]         _1 = (const 1i32, move _9);
[00:58:12]         StorageDead(_9);
[00:58:12]         goto -> bb13;
[00:58:12]     }
[00:58:12]     bb3: {                              
[00:58:12]         _1 = (const 3i32, const 3i32);
[00:58:12]         goto -> bb13;
[00:58:12]     }
[00:58:12]     bb4: {                              
[00:58:12]         falseEdges -> [real: bb9, imaginary: bb5];
[00:58:12]     }
[00:58:12]     bb5: {                              
[00:58:12]         falseEdges -> [real: bb12, imaginary: bb6];
[00:58:12]     }
[00:58:12]     bb6: {                              
[00:58:12]         falseEdges -> [real: bb3, imaginary: bb7];
[00:58:12]     }
[00:58:12]     bb7: {                              
[00:58:12]         unreachable;
[00:58:12]     }
[00:58:12]     bb8: {                              
[00:58:12]         unreachable;
[00:58:12]     }
[00:58:12]     bb9: {                              
[00:58:12]         StorageLive(_5);
[00:58:12]         _5 = &((_2 as Some).0: i32);
[00:58:12]         StorageLive(_8);
[00:58:12]         _8 = const guard() -> [return: bb10, unwind: bb1];
[00:58:12]     }
[00:58:12]     bb10: {                             
[00:58:12]         StorageLive(_4);
[00:58:12]         _4 = ((_2 as Some).0: i32);
[00:58:12]         switchInt(move _8) -> [false: bb11, otherwise: bb2];
[00:58:12]     }
[00:58:12]     bb11: {                             
[00:58:12]         falseEdges -> [real: bb5, imaginary: bb5];
[00:58:12]     }
[00:58:12]     bb12: {                             
[00:58:12]         StorageLive(_6);
[00:58:12]         _6 = ((_2 as Some).0: i32);
[00:58:12]         StorageLive(_10);
[00:58:12]         _10 = _6;
[00:58:12]         _1 = (const 2i32, move _10);
[00:58:12]         StorageDead(_10);
[00:58:12]         goto -> bb13;
[00:58:12]     }
[00:58:12]     bb13: {                             
[00:58:12]         StorageDead(_6);
[00:58:12]         StorageDead(_4);
[00:58:12]         StorageDead(_8);
[00:58:12]         StorageDead(_5);
[00:58:12]         StorageDead(_1);
[00:58:12]         StorageDead(_2);
[00:58:12]         _0 = ();
[00:58:12]         return;
[00:58:12] }', tools/compiletest/src/runtest.rs:2735:13
[00:58:12] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:58:12] 
[00:58:12] 
---
[00:58:12] 
[00:58:12] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:488:22
[00:58:12] 
[00:58:12] 
[00:58:12] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/mir-opt" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/mir-opt" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "mir-opt" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-3.9/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "3.9.1\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[00:58:12] 
[00:58:12] 
[00:58:12] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:58:12] Build completed unsuccessfully in 0:17:20
[00:58:12] Build completed unsuccessfully in 0:17:20
[00:58:12] Makefile:58: recipe for target 'check' failed
[00:58:12] make: *** [check] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:163a6649
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

pnkfelix · 2018-05-04T14:40:09Z

@bors r=nikomatsakis

bors · 2018-05-04T14:40:10Z

📌 Commit 930e76e has been approved by nikomatsakis

bors · 2018-05-04T15:00:21Z

⌛ Testing commit 930e76e with merge 91db9dc...

…in-guards, r=nikomatsakis Immutably and implicitly borrow all pattern ids for their guards (NLL only) This is an important piece of #27282. It applies only to NLL mode. It is a change to MIR codegen that is currently toggled on only when NLL is turned on. It thus affect MIR-borrowck but not the earlier static analyses (such as the type checker). This change makes it so that any pattern bindings of type T for a match arm will map to a `&T` within the context of the guard expression for that arm, but will continue to map to a `T` in the context of the arm body. To avoid surfacing this type distinction in the user source code (which would be a severe change to the language and would also require far more revision to the compiler internals), any occurrence of such an identifier in the guard expression will automatically get a deref op applied to it. So an input like: ```rust let place = (1, Foo::new()); match place { (1, foo) if inspect(foo) => feed(foo), ... } ``` will be treated as if it were really something like: ```rust let place = (1, Foo::new()); match place { (1, Foo { .. }) if { let tmp1 = &place.1; inspect(*tmp1) } => { let tmp2 = place.1; feed(tmp2) }, ... } ``` And an input like: ```rust let place = (2, Foo::new()); match place { (2, ref mut foo) if inspect(foo) => feed(foo), ... } ``` will be treated as if it were really something like: ```rust let place = (2, Foo::new()); match place { (2, Foo { .. }) if { let tmp1 = & &mut place.1; inspect(*tmp1) } => { let tmp2 = &mut place.1; feed(tmp2) }, ... } ``` In short, any pattern binding will always look like *some* kind of `&T` within the guard at least in terms of how the MIR-borrowck views it, and this will ensure that guard expressions cannot mutate their the match inputs via such bindings. (It also ensures that guard expressions can at most *copy* values from such bindings; non-Copy things cannot be moved via these pattern bindings in guard expressions, since one cannot move out of a `&T`.)

bors · 2018-05-04T17:44:43Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 91db9dc to master...

pnkfelix · 2018-05-07T13:36:07Z

I am now realizing that this change is probably very relevant to RFC 107 i.e. rust-lang/rfcs#107.

For some reason, allowing restricted mutation in match arms exposed an obvious case where a unique borrow can indeed fail, namely something like: ```rust match b { ... ref mut r if { (|| { let bar = &mut *r; **bar = false; })(); false } => { &mut *r } // ~~~~~~~ // | // This ends up holding a `&unique` borrow of `r`, but there ends up being an // implicit shared borrow in the guard thanks to rust-lang#49870 ... } ```

…ake-three, r=nikomatsakis every match arm reads the match's borrowed input This PR changes the `match` codegen under NLL (and just NLL, at least for now) to make the following adjustments: * It adds a `-Z disable-ast-check-for-mutation-in-guard` which, as described, turns off the naive (conservative but also not 100% sound) check for mutation in guards of match arms. * We now borrow the match input at the outset and emit a special `ReadForMatch` statement (that, according to the *static* semantics, reads that borrowed match input) at the start of each match arm. The intent here is to catch cases where the match guard mutates the match input, either via an independent borrow or via `ref mut` borrows in that arm's pattern. * In order to ensure that `ref mut` borrows do not actually conflict with the emitted `ReadForMatch` statements, I expanded the two-phase-borrow system slightly, and also changed the MIR code gen so that under NLL, when there is a guard on a match arm, then each pattern variable ends up having *three* temporaries associated with it: 1. The first temporary will hold the substructure being matched; this is what we will move the (substructural) value into *if* the guard succeeds. 2. The second temporary also corresponds to the same value as the first, but we are just constructing this temporarily for use during the scope of the guard; it is unaliased and its sole referrer is the third temporary. 3. The third temporary is a reference to the second temporary. * (This sounds complicated, I know, but its actually *simpler* than what I was doing before and had checked into the repo, which was to sometimes construct the final value and then take a reference to it before evaluating the guard. See also PR #49870.) Fix #27282 This also provides a path towards resolving #24535 aka rust-lang/rfcs#1006, at least once the `-Z disable-ast-check-for-mutation-in-guard` is just turned on by default (under NLL, that is. It is not sound under AST-borrowck). * But I did not want to make `#![feature(nll)]` imply that as part of this PR; that seemed like too drastic a change to me.

rust-highfive assigned cramertj Apr 11, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 11, 2018

rust-highfive assigned nikomatsakis and unassigned cramertj Apr 11, 2018

nikomatsakis requested changes Apr 12, 2018

View reviewed changes

nikomatsakis reviewed Apr 12, 2018

View reviewed changes

nikomatsakis approved these changes Apr 24, 2018

View reviewed changes

arielb1 reviewed Apr 25, 2018

View reviewed changes

nikomatsakis added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 25, 2018

nikomatsakis mentioned this pull request Apr 25, 2018

Can mutate in match-arm using a closure #27282

Closed

pnkfelix added 5 commits May 3, 2018 14:03

Include the test name when reporting that an expected line was not fo…

f67c683

…und in a mir-opt test.

Improve instrumentation for the bug reported during `fn report_borrow…

1c3fd02

…ed_value_does_not_live_long_enough`.

Instrument statement_effect_on_borrows for the lhs = &place case.

b00db7c

pnkfelix force-pushed the issue-27282-immut-borrow-all-pat-ids-in-guards branch from 1d37f66 to 28d18fa Compare May 3, 2018 12:33

Update mir-opt test to reflect change to MIR code-generation.

930e76e

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 4, 2018

bors merged commit 930e76e into rust-lang:master May 4, 2018

bors mentioned this pull request May 4, 2018

compiletest: Run revisions as independent tests. #50400

Merged

pnkfelix mentioned this pull request May 15, 2018

every match arm reads the match's borrowed input #50783

Merged

pnkfelix mentioned this pull request Jul 12, 2018

Tracking issue for Pattern Guards with Bind-By-Move #15287

Closed

3 tasks

pnkfelix mentioned this pull request Dec 19, 2018

Pattern errors are too imprecise and should be removed in favor of MIR borrowck #45600

Closed

Centril mentioned this pull request Aug 20, 2019

Stabilize bind_by_move_pattern_guards in Rust 1.39.0 #63118

Merged

pnkfelix mentioned this pull request May 31, 2024

Change ensures into closures model-checking/kani#3207

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Immutably and implicitly borrow all pattern ids for their guards (NLL only) #49870

Immutably and implicitly borrow all pattern ids for their guards (NLL only) #49870

pnkfelix commented Apr 11, 2018

rust-highfive commented Apr 11, 2018

pnkfelix commented Apr 11, 2018

pnkfelix commented Apr 11, 2018

nikomatsakis left a comment

nikomatsakis Apr 12, 2018

pnkfelix Apr 13, 2018

pnkfelix Apr 13, 2018

nikomatsakis Apr 12, 2018

pnkfelix Apr 13, 2018

arielb1 Apr 25, 2018 •

edited

Loading

nikomatsakis Apr 12, 2018

the8472 commented Apr 14, 2018

bors commented Apr 17, 2018

pietroalbini commented Apr 23, 2018

nikomatsakis commented Apr 24, 2018

arielb1 Apr 25, 2018 •

edited

Loading

nikomatsakis Apr 25, 2018

arielb1 Apr 25, 2018

nikomatsakis Apr 25, 2018 •

edited

Loading

arielb1 Apr 25, 2018 •

edited

Loading

arielb1 Apr 25, 2018 •

edited

Loading

nikomatsakis Apr 27, 2018

pnkfelix May 3, 2018

arielb1 Apr 25, 2018

nikomatsakis Apr 25, 2018

pnkfelix May 4, 2018 •

edited

Loading

pnkfelix commented May 2, 2018

pnkfelix commented May 3, 2018

rust-highfive commented May 3, 2018

pnkfelix commented May 4, 2018

bors commented May 4, 2018

bors commented May 4, 2018

bors commented May 4, 2018

pnkfelix commented May 7, 2018


		// Here is arielb1's basic example from rust-lang/rust#27282
		// that AST borrowck is flummoxed by:

Immutably and implicitly borrow all pattern ids for their guards (NLL only) #49870

Immutably and implicitly borrow all pattern ids for their guards (NLL only) #49870

Conversation

pnkfelix commented Apr 11, 2018

rust-highfive commented Apr 11, 2018

pnkfelix commented Apr 11, 2018

pnkfelix commented Apr 11, 2018

nikomatsakis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arielb1 Apr 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

the8472 commented Apr 14, 2018

bors commented Apr 17, 2018

pietroalbini commented Apr 23, 2018

nikomatsakis commented Apr 24, 2018

arielb1 Apr 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikomatsakis Apr 25, 2018 • edited Loading

Choose a reason for hiding this comment

arielb1 Apr 25, 2018 • edited Loading

Choose a reason for hiding this comment

arielb1 Apr 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pnkfelix May 4, 2018 • edited Loading

Choose a reason for hiding this comment

pnkfelix commented May 2, 2018

pnkfelix commented May 3, 2018

rust-highfive commented May 3, 2018

pnkfelix commented May 4, 2018

bors commented May 4, 2018

bors commented May 4, 2018

bors commented May 4, 2018

pnkfelix commented May 7, 2018

arielb1 Apr 25, 2018 •

edited

Loading

arielb1 Apr 25, 2018 •

edited

Loading

nikomatsakis Apr 25, 2018 •

edited

Loading

arielb1 Apr 25, 2018 •

edited

Loading

arielb1 Apr 25, 2018 •

edited

Loading

pnkfelix May 4, 2018 •

edited

Loading