Skip to content

Commit

Permalink
optimize let x: T = .. to avoid a temporary
Browse files Browse the repository at this point in the history
For some weird reason this fixes `intrinsic-move-val`. It also affects
various test heuristics. I removed one test (`reborrow_basic`) that
didn't seem to really be testing anything in particular anymore,
compared to all the other tests we've got.
  • Loading branch information
nikomatsakis committed Sep 10, 2018
1 parent 16f4e8a commit 2f6628e
Show file tree
Hide file tree
Showing 12 changed files with 105 additions and 122 deletions.
43 changes: 42 additions & 1 deletion src/librustc_mir/build/matches/mod.rs
Expand Up @@ -253,8 +253,8 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
irrefutable_pat: Pattern<'tcx>,
initializer: ExprRef<'tcx>,
) -> BlockAnd<()> {
// optimize the case of `let x = ...`
match *irrefutable_pat.kind {
// Optimize the case of `let x = ...` to write directly into `x`
PatternKind::Binding {
mode: BindingMode::ByValue,
var,
Expand All @@ -267,6 +267,47 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
self.schedule_drop_for_binding(var, irrefutable_pat.span, OutsideGuard);
block.unit()
}

// Optimize the case of `let x: T = ...` to write directly
// into `x` and then require that `T == typeof(x)`.
//
// Weirdly, this is needed to prevent the
// `intrinsic-move-val.rs` test case from crashing. That
// test works with uninitialized values in a rather
// dubious way, so it may be that the test is kind of
// broken.
PatternKind::AscribeUserType {
subpattern: Pattern {
kind: box PatternKind::Binding {
mode: BindingMode::ByValue,
var,
subpattern: None,
..
},
..
},
user_ty: ascription_user_ty,
} => {
let place =
self.storage_live_binding(block, var, irrefutable_pat.span, OutsideGuard);
unpack!(block = self.into(&place, block, initializer));

let source_info = self.source_info(irrefutable_pat.span);
self.cfg.push(
block,
Statement {
source_info,
kind: StatementKind::AscribeUserType(
place.clone(),
ty::Variance::Invariant,
ascription_user_ty,
),
},
);

self.schedule_drop_for_binding(var, irrefutable_pat.span, OutsideGuard);
block.unit()
}
_ => {
let place = unpack!(block = self.as_place(block, initializer));
self.place_into_pattern(block, irrefutable_pat, &place, true)
Expand Down
81 changes: 32 additions & 49 deletions src/test/mir-opt/basic_assignment.rs
Expand Up @@ -19,70 +19,53 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// check that codegen of assignment expressions is sane. Assignments
// tend to be absent in simple code, so subtle breakage in them can
// leave a quite hard-to-find trail of destruction.
// Check codegen for assignments (`a = b`) where the left-hand-side is
// not yet initialized. Assignments tend to be absent in simple code,
// so subtle breakage in them can leave a quite hard-to-find trail of
// destruction.

// ignore-tidy-linelength

fn main() {
let nodrop_x = false;
let nodrop_y;

// Since boolean does not require drop, this can be a simple
// assignment:
nodrop_y = nodrop_x;

let drop_x : Option<Box<u32>> = None;
let drop_y;

// Since the type of `drop_y` has drop, we generate a `replace`
// terminator:
drop_y = drop_x;
}

// END RUST SOURCE
// START rustc.main.SimplifyCfg-initial.after.mir
// bb0: {
// StorageLive(_1);
// _1 = const false;
// StorageLive(_2);
// StorageLive(_3);
// _3 = _1;
// _2 = move _3;
// StorageDead(_3);
// StorageLive(_4);
// AscribeUserType(_4, Canonical { variables: [], value: std::option::Option<std::boxed::Box<u32>> });
// _4 = std::option::Option<std::boxed::Box<u32>>::None;
// StorageLive(_5);
// StorageLive(_6);
// _6 = move _4;
// replace(_5 <-move _6) -> [return: bb2, unwind: bb5];
// }
// bb1: {
// resume;
// }
// bb2: {
// drop(_6) -> [return: bb6, unwind: bb4];
// }
// bb3: {
// drop(_4) -> bb1;
// }
// bb4: {
// drop(_5) -> bb3;
// }
// bb5: {
// drop(_6) -> bb4;
// }
// bb6: {
// StorageDead(_6);
// _0 = ();
// drop(_5) -> [return: bb7, unwind: bb3];
// }
// bb7: {
// StorageDead(_5);
// drop(_4) -> [return: bb8, unwind: bb1];
// }
// bb8: {
// StorageDead(_4);
// StorageDead(_2);
// StorageDead(_1);
// return;
// }
// bb0: {
// StorageLive(_1);
// _1 = const false;
// StorageLive(_2);
// StorageLive(_3);
// _3 = _1;
// _2 = move _3;
// StorageDead(_3);
// StorageLive(_4);
// _4 = std::option::Option<std::boxed::Box<u32>>::None;
// AscribeUserType(_4, o, Canonical { variables: [], value: std::option::Option<std::boxed::Box<u32>> });
// StorageLive(_5);
// StorageLive(_6);
// _6 = move _4;
// replace(_5 <- move _6) -> [return: bb2, unwind: bb5];
// }
// ...
// bb2: {
// drop(_6) -> [return: bb6, unwind: bb4];
// }
// ...
// bb5: {
// drop(_6) -> bb4;
// }
// END rustc.main.SimplifyCfg-initial.after.mir
38 changes: 0 additions & 38 deletions src/test/mir-opt/nll/reborrow-basic.rs

This file was deleted.

@@ -1,24 +1,24 @@
error: unsatisfied lifetime constraints
--> $DIR/associated-types-project-from-hrtb-in-fn-body.rs:32:19
--> $DIR/associated-types-project-from-hrtb-in-fn-body.rs:32:29
|
LL | fn bar<'a, 'b, I : for<'x> Foo<&'x isize>>(
| -- -- lifetime `'b` defined here
| |
| lifetime `'a` defined here
...
LL | let z: I::A = if cond { x } else { y };
| ^^^^^^^^^^^^^^^^^^^^^^^^ requires that `'a` must outlive `'b`
| ^ assignment requires that `'a` must outlive `'b`

error: unsatisfied lifetime constraints
--> $DIR/associated-types-project-from-hrtb-in-fn-body.rs:32:19
--> $DIR/associated-types-project-from-hrtb-in-fn-body.rs:32:40
|
LL | fn bar<'a, 'b, I : for<'x> Foo<&'x isize>>(
| -- -- lifetime `'b` defined here
| |
| lifetime `'a` defined here
...
LL | let z: I::A = if cond { x } else { y };
| ^^^^^^^^^^^^^^^^^^^^^^^^ requires that `'b` must outlive `'a`
| ^ assignment requires that `'b` must outlive `'a`

error: aborting due to 2 previous errors

Expand Up @@ -35,7 +35,7 @@ error[E0596]: cannot borrow `*ptr_x` as mutable, as it is behind a `*const` poin
--> $DIR/borrowck-access-permissions.rs:56:23
|
LL | let ptr_x : *const _ = &x;
| ----- help: consider changing this to be a mutable pointer: `*mut i32`
| -- help: consider changing this to be a mutable pointer: `&mut x`
...
LL | let _y1 = &mut *ptr_x; //[ast]~ ERROR [E0596]
| ^^^^^^^^^^^ `ptr_x` is a `*const` pointer, so the data it refers to cannot be borrowed as mutable
Expand Down
Expand Up @@ -35,7 +35,7 @@ error[E0596]: cannot borrow `*ptr_x` as mutable, as it is behind a `*const` poin
--> $DIR/borrowck-access-permissions.rs:56:23
|
LL | let ptr_x : *const _ = &x;
| ----- help: consider changing this to be a mutable pointer: `*mut i32`
| -- help: consider changing this to be a mutable pointer: `&mut x`
...
LL | let _y1 = &mut *ptr_x; //[ast]~ ERROR [E0596]
| ^^^^^^^^^^^ `ptr_x` is a `*const` pointer, so the data it refers to cannot be borrowed as mutable
Expand Down
12 changes: 10 additions & 2 deletions src/test/ui/nll/relate_tys/hr-fn-aaa-as-aba.rs
Expand Up @@ -20,9 +20,17 @@ fn make_it() -> for<'a> fn(&'a u32, &'a u32) -> &'a u32 {
panic!()
}

fn main() {
fn foo() {
let a: for<'a, 'b> fn(&'a u32, &'b u32) -> &'a u32 = make_it();
//~^ ERROR higher-ranked subtype error
//~| ERROR higher-ranked subtype error
drop(a);
}

fn bar() {
// The code path for patterns is mildly different, so go ahead and
// test that too:
let _: for<'a, 'b> fn(&'a u32, &'b u32) -> &'a u32 = make_it();
//~^ ERROR higher-ranked subtype error
}

fn main() { }
6 changes: 3 additions & 3 deletions src/test/ui/nll/relate_tys/hr-fn-aaa-as-aba.stderr
Expand Up @@ -5,10 +5,10 @@ LL | let a: for<'a, 'b> fn(&'a u32, &'b u32) -> &'a u32 = make_it();
| ^^^^^^^^^

error: higher-ranked subtype error
--> $DIR/hr-fn-aaa-as-aba.rs:24:9
--> $DIR/hr-fn-aaa-as-aba.rs:32:58
|
LL | let a: for<'a, 'b> fn(&'a u32, &'b u32) -> &'a u32 = make_it();
| ^
LL | let _: for<'a, 'b> fn(&'a u32, &'b u32) -> &'a u32 = make_it();
| ^^^^^^^^^

error: aborting due to 2 previous errors

12 changes: 3 additions & 9 deletions src/test/ui/nll/relate_tys/universe-violation.stderr
@@ -1,14 +1,8 @@
error: higher-ranked subtype error
--> $DIR/universe-violation.rs:14:9
|
LL | let a: fn(_) -> _ = make_it();
| ^

error: higher-ranked subtype error
--> $DIR/universe-violation.rs:15:9
--> $DIR/universe-violation.rs:15:31
|
LL | let b: fn(&u32) -> &u32 = a;
| ^
| ^

error: aborting due to 2 previous errors
error: aborting due to previous error

@@ -1,11 +1,10 @@
error[E0621]: explicit lifetime required in the type of `v`
--> $DIR/region-object-lifetime-in-coercion.rs:20:5
--> $DIR/region-object-lifetime-in-coercion.rs:18:33
|
LL | fn a(v: &[u8]) -> Box<Foo + 'static> {
| ----- help: add explicit lifetime `'static` to the type of `v`: `&'static [u8]`
...
LL | x
| ^ lifetime `'static` required
LL | let x: Box<Foo + 'static> = Box::new(v);
| ^^^^^^^^^^^ lifetime `'static` required

error[E0621]: explicit lifetime required in the type of `v`
--> $DIR/region-object-lifetime-in-coercion.rs:24:5
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/slice-mut-2.nll.stderr
Expand Up @@ -2,7 +2,7 @@ error[E0596]: cannot borrow `*x` as mutable, as it is behind a `&` reference
--> $DIR/slice-mut-2.rs:17:18
|
LL | let x: &[isize] = &[1, 2, 3, 4, 5];
| - help: consider changing this to be a mutable reference: `&mut [isize]`
| ---------------- help: consider changing this to be a mutable reference: `&mut [1, 2, 3, 4, 5]`
...
LL | let _ = &mut x[2..4]; //~ERROR cannot borrow immutable borrowed content `*x` as mutable
| ^ `x` is a `&` reference, so the data it refers to cannot be borrowed as mutable
Expand Down
Expand Up @@ -38,12 +38,9 @@ LL | let g = factorial.as_ref().unwrap();
| ^^^^^^^^^ borrowed value does not live long enough
...
LL | }
| -
| |
| `factorial` dropped here while still borrowed
| borrow later used here, when `factorial` is dropped
| - `factorial` dropped here while still borrowed
|
= note: values in a scope are dropped in the opposite order they are defined
= note: borrowed value must be valid for the static lifetime...

error[E0506]: cannot assign to `factorial` because it is borrowed
--> $DIR/unboxed-closures-failed-recursive-fn-1.rs:42:5
Expand All @@ -55,10 +52,9 @@ LL | let g = factorial.as_ref().unwrap();
| --------- borrow occurs due to use in closure
...
LL | factorial = Some(Box::new(f));
| ^^^^^^^^^
| |
| assignment to borrowed `factorial` occurs here
| borrow later used here
| ^^^^^^^^^ assignment to borrowed `factorial` occurs here
|
= note: borrowed value must be valid for the static lifetime...

error: aborting due to 4 previous errors

Expand Down

0 comments on commit 2f6628e

Please sign in to comment.