From b9af400a4659f722843ac96c251ec52f87998dd2 Mon Sep 17 00:00:00 2001
From: Eduard-Mihai Burtescu <edy.burt@gmail.com>
Date: Sat, 5 May 2018 13:27:51 +0300
Subject: [PATCH 1/5] rustc_mir: generate an extra temporary during borrowed
rvalue promotion.
---
src/librustc_mir/transform/promote_consts.rs | 150 +++++++++++-------
.../end_region_destruction_extents_1.rs | 8 +-
2 files changed, 98 insertions(+), 60 deletions(-)
diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs
index b732eeb624c6d..fb7eb60545c97 100644
--- a/src/librustc_mir/transform/promote_consts.rs
+++ b/src/librustc_mir/transform/promote_consts.rs
@@ -25,14 +25,12 @@
use rustc::mir::*;
use rustc::mir::visit::{PlaceContext, MutVisitor, Visitor};
use rustc::mir::traversal::ReversePostorder;
-use rustc::ty::TyCtxt;
+use rustc::ty::{self, TyCtxt};
use syntax_pos::Span;
use rustc_data_structures::indexed_vec::{IndexVec, Idx};
-use std::iter;
-use std::mem;
-use std::usize;
+use std::{cmp, iter, mem, usize};
/// State of a temporary during collection and promotion.
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
@@ -150,9 +148,11 @@ pub fn collect_temps(mir: &Mir, rpo: &mut ReversePostorder) -> IndexVec<Local, T
}
struct Promoter<'a, 'tcx: 'a> {
+ tcx: TyCtxt<'a, 'tcx, 'tcx>,
source: &'a mut Mir<'tcx>,
promoted: Mir<'tcx>,
temps: &'a mut IndexVec<Local, TempState>,
+ extra_statements: &'a mut Vec<(Location, Statement<'tcx>)>,
/// If true, all nested temps are also kept in the
/// source MIR, not moved to the promoted MIR.
@@ -288,38 +288,78 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
}
fn promote_candidate(mut self, candidate: Candidate) {
- let span = self.promoted.span;
- let new_operand = Operand::Constant(box Constant {
- span,
- ty: self.promoted.return_ty(),
- literal: Literal::Promoted {
+ let mut rvalue = {
+ let promoted = &mut self.promoted;
+ let literal = Literal::Promoted {
index: Promoted::new(self.source.promoted.len())
- }
- });
- let mut rvalue = match candidate {
- Candidate::Ref(Location { block: bb, statement_index: stmt_idx }) => {
- let ref mut statement = self.source[bb].statements[stmt_idx];
- match statement.kind {
- StatementKind::Assign(_, ref mut rvalue) => {
- mem::replace(rvalue, Rvalue::Use(new_operand))
+ };
+ let operand = |ty, span| {
+ promoted.span = span;
+ promoted.local_decls[RETURN_PLACE] =
+ LocalDecl::new_return_place(ty, span);
+ Operand::Constant(box Constant {
+ span,
+ ty,
+ literal
+ })
+ };
+ let (blocks, local_decls) = self.source.basic_blocks_and_local_decls_mut();
+ match candidate {
+ Candidate::Ref(loc) => {
+ let ref mut statement = blocks[loc.block].statements[loc.statement_index];
+ match statement.kind {
+ StatementKind::Assign(_, Rvalue::Ref(r, bk, ref mut place)) => {
+ let ty = place.ty(local_decls, self.tcx).to_ty(self.tcx);
+ let ref_ty = self.tcx.mk_ref(r,
+ ty::TypeAndMut {
+ ty,
+ mutbl: bk.to_mutbl_lossy()
+ }
+ );
+ let span = statement.source_info.span;
+
+ // Create a temp to hold the promoted reference.
+ // This is because `*r` requires `r` to be a local,
+ // otherwise we would use the `promoted` directly.
+ let mut promoted_ref = LocalDecl::new_temp(ref_ty, span);
+ promoted_ref.source_info = statement.source_info;
+ let promoted_ref = local_decls.push(promoted_ref);
+ assert_eq!(self.temps.push(TempState::Unpromotable), promoted_ref);
+ self.extra_statements.push((loc, Statement {
+ source_info: statement.source_info,
+ kind: StatementKind::Assign(
+ Place::Local(promoted_ref),
+ Rvalue::Use(operand(ref_ty, span)),
+ )
+ }));
+ let promoted_place = Place::Local(promoted_ref).deref();
+
+ Rvalue::Ref(r, bk, mem::replace(place, promoted_place))
+ }
+ _ => bug!()
}
- _ => bug!()
}
- }
- Candidate::Argument { bb, index } => {
- match self.source[bb].terminator_mut().kind {
- TerminatorKind::Call { ref mut args, .. } => {
- Rvalue::Use(mem::replace(&mut args[index], new_operand))
+ Candidate::Argument { bb, index } => {
+ let terminator = blocks[bb].terminator_mut();
+ match terminator.kind {
+ TerminatorKind::Call { ref mut args, .. } => {
+ let ty = args[index].ty(local_decls, self.tcx);
+ let span = terminator.source_info.span;
+ Rvalue::Use(mem::replace(&mut args[index], operand(ty, span)))
+ }
+ _ => bug!()
}
- _ => bug!()
}
}
};
+
+ assert_eq!(self.new_block(), START_BLOCK);
self.visit_rvalue(&mut rvalue, Location {
block: BasicBlock::new(0),
statement_index: usize::MAX
});
+ let span = self.promoted.span;
self.assign(RETURN_PLACE, rvalue, span);
self.source.promoted.push(self.promoted);
}
@@ -343,43 +383,29 @@ pub fn promote_candidates<'a, 'tcx>(mir: &mut Mir<'tcx>,
candidates: Vec<Candidate>) {
// Visit candidates in reverse, in case they're nested.
debug!("promote_candidates({:?})", candidates);
+
+ let mut extra_statements = vec![];
for candidate in candidates.into_iter().rev() {
- let (span, ty) = match candidate {
- Candidate::Ref(Location { block: bb, statement_index: stmt_idx }) => {
- let statement = &mir[bb].statements[stmt_idx];
- let dest = match statement.kind {
- StatementKind::Assign(ref dest, _) => dest,
- _ => {
- span_bug!(statement.source_info.span,
- "expected assignment to promote");
- }
- };
- if let Place::Local(index) = *dest {
- if temps[index] == TempState::PromotedOut {
- // Already promoted.
- continue;
+ match candidate {
+ Candidate::Ref(Location { block, statement_index }) => {
+ match mir[block].statements[statement_index].kind {
+ StatementKind::Assign(Place::Local(local), _) => {
+ if temps[local] == TempState::PromotedOut {
+ // Already promoted.
+ continue;
+ }
}
+ _ => {}
}
- (statement.source_info.span, dest.ty(mir, tcx).to_ty(tcx))
- }
- Candidate::Argument { bb, index } => {
- let terminator = mir[bb].terminator();
- let ty = match terminator.kind {
- TerminatorKind::Call { ref args, .. } => {
- args[index].ty(mir, tcx)
- }
- _ => {
- span_bug!(terminator.source_info.span,
- "expected call argument to promote");
- }
- };
- (terminator.source_info.span, ty)
}
- };
+ Candidate::Argument { .. } => {}
+ }
+
- // Declare return place local
- let initial_locals = iter::once(LocalDecl::new_return_place(ty, span))
- .collect();
+ // Declare return place local so that `Mir::new` doesn't complain.
+ let initial_locals = iter::once(
+ LocalDecl::new_return_place(tcx.types.never, mir.span)
+ ).collect();
let mut promoter = Promoter {
promoted: Mir::new(
@@ -393,16 +419,24 @@ pub fn promote_candidates<'a, 'tcx>(mir: &mut Mir<'tcx>,
initial_locals,
0,
vec![],
- span
+ mir.span
),
+ tcx,
source: mir,
temps: &mut temps,
+ extra_statements: &mut extra_statements,
keep_original: false
};
- assert_eq!(promoter.new_block(), START_BLOCK);
promoter.promote_candidate(candidate);
}
+ // Insert each of `extra_statements` before its indicated location, which
+ // has to be done in reverse location order, to not invalidate the rest.
+ extra_statements.sort_by_key(|&(loc, _)| cmp::Reverse(loc));
+ for (loc, statement) in extra_statements {
+ mir[loc.block].statements.insert(loc.statement_index, statement);
+ }
+
// Eliminate assignments to, and drops of promoted temps.
let promoted = |index: Local| temps[index] == TempState::PromotedOut;
for block in mir.basic_blocks_mut() {
diff --git a/src/test/mir-opt/end_region_destruction_extents_1.rs b/src/test/mir-opt/end_region_destruction_extents_1.rs
index e189f2e3b34a3..15b104f6c2ff7 100644
--- a/src/test/mir-opt/end_region_destruction_extents_1.rs
+++ b/src/test/mir-opt/end_region_destruction_extents_1.rs
@@ -130,17 +130,21 @@ unsafe impl<'a, #[may_dangle] 'b> Drop for D1<'a, 'b> {
// let mut _7: &'10s S1;
// let mut _8: &'10s S1;
// let mut _9: S1;
+// let mut _10: &'10s S1;
+// let mut _11: &'12ds S1;
//
// bb0: {
// StorageLive(_2);
// StorageLive(_3);
// StorageLive(_4);
// StorageLive(_5);
-// _5 = promoted[1];
+// _11 = promoted[1];
+// _5 = &'12ds (*_11);
// _4 = &'12ds (*_5);
// StorageLive(_7);
// StorageLive(_8);
-// _8 = promoted[0];
+// _10 = promoted[0];
+// _8 = &'10s (*_10);
// _7 = &'10s (*_8);
// _3 = D1<'12ds, '10s>::{{constructor}}(move _4, move _7);
// EndRegion('10s);
From d79dee0b62fb33bcb2febac60ced71f28dccc602 Mon Sep 17 00:00:00 2001
From: Eduard-Mihai Burtescu <edy.burt@gmail.com>
Date: Thu, 10 May 2018 12:06:51 +0300
Subject: [PATCH 2/5] rustc_mir: also promote interior borrows, not just whole
temps.
---
src/librustc_mir/transform/qualify_consts.rs | 15 ++++++++--
src/test/run-pass/issue-49955.rs | 30 ++++++++++++++++++++
2 files changed, 43 insertions(+), 2 deletions(-)
create mode 100644 src/test/run-pass/issue-49955.rs
diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs
index 28ab3d6a8574f..544f122f25ae3 100644
--- a/src/librustc_mir/transform/qualify_consts.rs
+++ b/src/librustc_mir/transform/qualify_consts.rs
@@ -737,10 +737,21 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
// We might have a candidate for promotion.
let candidate = Candidate::Ref(location);
if self.can_promote() {
- // We can only promote direct borrows of temps.
+ // We can only promote interior borrows of non-drop temps.
+ let mut place = place;
+ while let Place::Projection(ref proj) = *place {
+ if proj.elem == ProjectionElem::Deref {
+ break;
+ }
+ place = &proj.base;
+ }
if let Place::Local(local) = *place {
if self.mir.local_kind(local) == LocalKind::Temp {
- self.promotion_candidates.push(candidate);
+ if let Some(qualif) = self.temp_qualif[local] {
+ if !qualif.intersects(Qualif::NEEDS_DROP) {
+ self.promotion_candidates.push(candidate);
+ }
+ }
}
}
}
diff --git a/src/test/run-pass/issue-49955.rs b/src/test/run-pass/issue-49955.rs
new file mode 100644
index 0000000000000..2d36806ef4f44
--- /dev/null
+++ b/src/test/run-pass/issue-49955.rs
@@ -0,0 +1,30 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// compile-flags: -Z borrowck=compare
+
+const ALL_THE_NUMS: [u32; 1] = [
+ 1
+];
+
+#[inline(never)]
+fn array(i: usize) -> &'static u32 {
+ return &ALL_THE_NUMS[i];
+}
+
+#[inline(never)]
+fn tuple_field() -> &'static u32 {
+ &(42,).0
+}
+
+fn main() {
+ assert_eq!(tuple_field().to_string(), "42");
+ // assert_eq!(array(0).to_string(), "1");
+}
From 4fec5ef81a24481dbe6f9637e772c0cc6c53568e Mon Sep 17 00:00:00 2001
From: Eduard-Mihai Burtescu <edy.burt@gmail.com>
Date: Thu, 10 May 2018 12:39:50 +0300
Subject: [PATCH 3/5] rustc_mir: promote borrows' underlying temps, and project
at runtime.
---
src/librustc_mir/transform/promote_consts.rs | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/src/librustc_mir/transform/promote_consts.rs b/src/librustc_mir/transform/promote_consts.rs
index fb7eb60545c97..5af19ab364696 100644
--- a/src/librustc_mir/transform/promote_consts.rs
+++ b/src/librustc_mir/transform/promote_consts.rs
@@ -309,6 +309,18 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
let ref mut statement = blocks[loc.block].statements[loc.statement_index];
match statement.kind {
StatementKind::Assign(_, Rvalue::Ref(r, bk, ref mut place)) => {
+ // Find the underlying local for this (necessarilly interior) borrow.
+ // HACK(eddyb) using a recursive function because of mutable borrows.
+ fn interior_base<'a, 'tcx>(place: &'a mut Place<'tcx>)
+ -> &'a mut Place<'tcx> {
+ if let Place::Projection(ref mut proj) = *place {
+ assert_ne!(proj.elem, ProjectionElem::Deref);
+ return interior_base(&mut proj.base);
+ }
+ place
+ }
+ let place = interior_base(place);
+
let ty = place.ty(local_decls, self.tcx).to_ty(self.tcx);
let ref_ty = self.tcx.mk_ref(r,
ty::TypeAndMut {
From 22275f46b2b6a9efe50c5a4485ed766fce3ac10a Mon Sep 17 00:00:00 2001
From: Eduard-Mihai Burtescu <edy.burt@gmail.com>
Date: Wed, 16 May 2018 14:15:29 +0300
Subject: [PATCH 4/5] rustc_mir: focus const-checking logic on whether mutation
is forbidden.
---
src/librustc_mir/transform/qualify_consts.rs | 40 +++++++++++++-------
1 file changed, 26 insertions(+), 14 deletions(-)
diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs
index 544f122f25ae3..6f90794ed896b 100644
--- a/src/librustc_mir/transform/qualify_consts.rs
+++ b/src/librustc_mir/transform/qualify_consts.rs
@@ -679,24 +679,31 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
}
let ty = place.ty(self.mir, self.tcx).to_ty(self.tcx);
+
+ // Default to forbidding the borrow and/or its promotion,
+ // due to the potential for direct or interior mutability,
+ // and only proceed by setting `forbidden_mut` to `false`.
+ let mut forbidden_mut = true;
+
if let BorrowKind::Mut { .. } = kind {
// In theory, any zero-sized value could be borrowed
// mutably without consequences. However, only &mut []
// is allowed right now, and only in functions.
- let allow = if self.mode == Mode::StaticMut {
+ if self.mode == Mode::StaticMut {
// Inside a `static mut`, &mut [...] is also allowed.
match ty.sty {
- ty::TyArray(..) | ty::TySlice(_) => true,
- _ => false
+ ty::TyArray(..) | ty::TySlice(_) => forbidden_mut = false,
+ _ => {}
}
} else if let ty::TyArray(_, len) = ty.sty {
- len.unwrap_usize(self.tcx) == 0 &&
- self.mode == Mode::Fn
- } else {
- false
- };
+ // FIXME(eddyb) the `self.mode == Mode::Fn` condition
+ // seems unnecessary, given that this is merely a ZST.
+ if len.unwrap_usize(self.tcx) == 0 && self.mode == Mode::Fn {
+ forbidden_mut = false;
+ }
+ }
- if !allow {
+ if forbidden_mut {
self.add(Qualif::NOT_CONST);
if self.mode != Mode::Fn {
let mut err = struct_span_err!(self.tcx.sess, self.span, E0017,
@@ -722,21 +729,26 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
// it means that our "silent insertion of statics" could change
// initializer values (very bad).
if self.qualif.intersects(Qualif::MUTABLE_INTERIOR) {
- // Replace MUTABLE_INTERIOR with NOT_CONST to avoid
+ // A reference of a MUTABLE_INTERIOR place is instead
+ // NOT_CONST (see `if forbidden_mut` below), to avoid
// duplicate errors (from reborrowing, for example).
self.qualif = self.qualif - Qualif::MUTABLE_INTERIOR;
- self.add(Qualif::NOT_CONST);
if self.mode != Mode::Fn {
span_err!(self.tcx.sess, self.span, E0492,
"cannot borrow a constant which may contain \
interior mutability, create a static instead");
}
+ } else {
+ // We allow immutable borrows of frozen data.
+ forbidden_mut = false;
}
}
- // We might have a candidate for promotion.
- let candidate = Candidate::Ref(location);
- if self.can_promote() {
+ if forbidden_mut {
+ self.add(Qualif::NOT_CONST);
+ } else if self.can_promote() {
+ // We might have a candidate for promotion.
+ let candidate = Candidate::Ref(location);
// We can only promote interior borrows of non-drop temps.
let mut place = place;
while let Place::Projection(ref proj) = *place {
From d1f117df0fd71d443742a455532e5f053f24a741 Mon Sep 17 00:00:00 2001
From: Eduard-Mihai Burtescu <edy.burt@gmail.com>
Date: Thu, 10 May 2018 13:11:36 +0300
Subject: [PATCH 5/5] rustc_mir: allow promotion of promotable temps indexed at
runtime.
---
src/librustc_mir/transform/qualify_consts.rs | 21 ++++++++++------
src/test/mir-opt/match_false_edges.rs | 3 ++-
src/test/run-pass/issue-49955-2.rs | 26 ++++++++++++++++++++
src/test/run-pass/issue-49955.rs | 2 +-
4 files changed, 42 insertions(+), 10 deletions(-)
create mode 100644 src/test/run-pass/issue-49955-2.rs
diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs
index 6f90794ed896b..fd4ba1d75625a 100644
--- a/src/librustc_mir/transform/qualify_consts.rs
+++ b/src/librustc_mir/transform/qualify_consts.rs
@@ -229,12 +229,12 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
}
/// Check if a Local with the current qualifications is promotable.
- fn can_promote(&mut self) -> bool {
+ fn can_promote(&self, qualif: Qualif) -> bool {
// References to statics are allowed, but only in other statics.
if self.mode == Mode::Static || self.mode == Mode::StaticMut {
- (self.qualif - Qualif::STATIC_REF).is_empty()
+ (qualif - Qualif::STATIC_REF).is_empty()
} else {
- self.qualif.is_empty()
+ qualif.is_empty()
}
}
@@ -746,10 +746,10 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
if forbidden_mut {
self.add(Qualif::NOT_CONST);
- } else if self.can_promote() {
+ } else {
// We might have a candidate for promotion.
let candidate = Candidate::Ref(location);
- // We can only promote interior borrows of non-drop temps.
+ // We can only promote interior borrows of promotable temps.
let mut place = place;
while let Place::Projection(ref proj) = *place {
if proj.elem == ProjectionElem::Deref {
@@ -760,7 +760,12 @@ impl<'a, 'tcx> Visitor<'tcx> for Qualifier<'a, 'tcx, 'tcx> {
if let Place::Local(local) = *place {
if self.mir.local_kind(local) == LocalKind::Temp {
if let Some(qualif) = self.temp_qualif[local] {
- if !qualif.intersects(Qualif::NEEDS_DROP) {
+ // `forbidden_mut` is false, so we can safely ignore
+ // `MUTABLE_INTERIOR` from the local's qualifications.
+ // This allows borrowing fields which don't have
+ // `MUTABLE_INTERIOR`, from a type that does, e.g.:
+ // `let _: &'static _ = &(Cell::new(1), 2).1;`
+ if self.can_promote(qualif - Qualif::MUTABLE_INTERIOR) {
self.promotion_candidates.push(candidate);
}
}
@@ -920,7 +925,7 @@ This does not pose a problem by itself because they can't be accessed directly."
}
let candidate = Candidate::Argument { bb, index: i };
if is_shuffle && i == 2 {
- if this.can_promote() {
+ if this.can_promote(this.qualif) {
this.promotion_candidates.push(candidate);
} else {
span_err!(this.tcx.sess, this.span, E0526,
@@ -936,7 +941,7 @@ This does not pose a problem by itself because they can't be accessed directly."
if !constant_arguments.contains(&i) {
return
}
- if this.can_promote() {
+ if this.can_promote(this.qualif) {
this.promotion_candidates.push(candidate);
} else {
this.tcx.sess.span_err(this.span,
diff --git a/src/test/mir-opt/match_false_edges.rs b/src/test/mir-opt/match_false_edges.rs
index a31298a0f5160..c2a40399efe3d 100644
--- a/src/test/mir-opt/match_false_edges.rs
+++ b/src/test/mir-opt/match_false_edges.rs
@@ -88,7 +88,8 @@ fn main() {
// }
// bb9: { // binding1 and guard
// StorageLive(_5);
-// _5 = &((_2 as Some).0: i32);
+// _11 = promoted[0];
+// _5 = &(((*_11) as Some).0: i32);
// StorageLive(_8);
// _8 = const guard() -> [return: bb10, unwind: bb1];
// }
diff --git a/src/test/run-pass/issue-49955-2.rs b/src/test/run-pass/issue-49955-2.rs
new file mode 100644
index 0000000000000..17e1de95dd30e
--- /dev/null
+++ b/src/test/run-pass/issue-49955-2.rs
@@ -0,0 +1,26 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// compile-flags: -Z borrowck=mir
+
+use std::cell::Cell;
+
+#[inline(never)]
+fn tuple_field() -> &'static u32 {
+ // This test is MIR-borrowck-only because the old borrowck
+ // doesn't agree that borrows of "frozen" (i.e. without any
+ // interior mutability) fields of non-frozen temporaries,
+ // should be promoted, while MIR promotion does promote them.
+ &(Cell::new(5), 42).1
+}
+
+fn main() {
+ assert_eq!(tuple_field().to_string(), "42");
+}
diff --git a/src/test/run-pass/issue-49955.rs b/src/test/run-pass/issue-49955.rs
index 2d36806ef4f44..57a1264aaee80 100644
--- a/src/test/run-pass/issue-49955.rs
+++ b/src/test/run-pass/issue-49955.rs
@@ -26,5 +26,5 @@ fn tuple_field() -> &'static u32 {
fn main() {
assert_eq!(tuple_field().to_string(), "42");
- // assert_eq!(array(0).to_string(), "1");
+ assert_eq!(array(0).to_string(), "1");
}