Skip to content

Commit

Permalink
Auto merge of #55385 - davidtwco:issue-55288, r=oli-obk
Browse files Browse the repository at this point in the history
NLL: cast causes failure to promote to static

Fixes #55288. See commit messages for more details.

r? @oli-obk
cc @nikomatsakis
cc @pnkfelix
cc @RalfJung
  • Loading branch information
bors committed Oct 27, 2018
2 parents f32f111 + 6208bd8 commit b3b8760
Show file tree
Hide file tree
Showing 14 changed files with 356 additions and 194 deletions.
290 changes: 191 additions & 99 deletions src/librustc/mir/visit.rs

Large diffs are not rendered by default.

49 changes: 32 additions & 17 deletions src/librustc_codegen_llvm/mir/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use rustc_data_structures::bit_set::BitSet;
use rustc_data_structures::graph::dominators::Dominators;
use rustc_data_structures::indexed_vec::{Idx, IndexVec};
use rustc::mir::{self, Location, TerminatorKind};
use rustc::mir::visit::{Visitor, PlaceContext};
use rustc::mir::visit::{Visitor, PlaceContext, MutatingUseContext, NonMutatingUseContext};
use rustc::mir::traversal;
use rustc::ty;
use rustc::ty::layout::LayoutOf;
Expand Down Expand Up @@ -116,7 +116,11 @@ impl Visitor<'tcx> for LocalAnalyzer<'mir, 'a, 'll, 'tcx> {
self.not_ssa(index);
}
} else {
self.visit_place(place, PlaceContext::Store, location);
self.visit_place(
place,
PlaceContext::MutatingUse(MutatingUseContext::Store),
location
);
}

self.visit_rvalue(rvalue, location);
Expand All @@ -142,7 +146,11 @@ impl Visitor<'tcx> for LocalAnalyzer<'mir, 'a, 'll, 'tcx> {
// is not guaranteed to be statically dominated by the
// definition of x, so x must always be in an alloca.
if let mir::Operand::Move(ref place) = args[0] {
self.visit_place(place, PlaceContext::Drop, location);
self.visit_place(
place,
PlaceContext::MutatingUse(MutatingUseContext::Drop),
location
);
}
}
}
Expand All @@ -160,7 +168,8 @@ impl Visitor<'tcx> for LocalAnalyzer<'mir, 'a, 'll, 'tcx> {
if let mir::Place::Projection(ref proj) = *place {
// Allow uses of projections that are ZSTs or from scalar fields.
let is_consume = match context {
PlaceContext::Copy | PlaceContext::Move => true,
PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy) |
PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) => true,
_ => false
};
if is_consume {
Expand Down Expand Up @@ -190,7 +199,11 @@ impl Visitor<'tcx> for LocalAnalyzer<'mir, 'a, 'll, 'tcx> {

// A deref projection only reads the pointer, never needs the place.
if let mir::ProjectionElem::Deref = proj.elem {
return self.visit_place(&proj.base, PlaceContext::Copy, location);
return self.visit_place(
&proj.base,
PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy),
location
);
}
}

Expand All @@ -202,16 +215,14 @@ impl Visitor<'tcx> for LocalAnalyzer<'mir, 'a, 'll, 'tcx> {
context: PlaceContext<'tcx>,
location: Location) {
match context {
PlaceContext::Call => {
PlaceContext::MutatingUse(MutatingUseContext::Call) => {
self.assign(local, location);
}

PlaceContext::StorageLive |
PlaceContext::StorageDead |
PlaceContext::Validate => {}
PlaceContext::NonUse(_) => {}

PlaceContext::Copy |
PlaceContext::Move => {
PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy) |
PlaceContext::NonMutatingUse(NonMutatingUseContext::Move) => {
// Reads from uninitialized variables (e.g. in dead code, after
// optimizations) require locals to be in (uninitialized) memory.
// NB: there can be uninitialized reads of a local visited after
Expand All @@ -227,15 +238,19 @@ impl Visitor<'tcx> for LocalAnalyzer<'mir, 'a, 'll, 'tcx> {
}
}

PlaceContext::Inspect |
PlaceContext::Store |
PlaceContext::AsmOutput |
PlaceContext::Borrow { .. } |
PlaceContext::Projection(..) => {
PlaceContext::NonMutatingUse(NonMutatingUseContext::Inspect) |
PlaceContext::MutatingUse(MutatingUseContext::Store) |
PlaceContext::MutatingUse(MutatingUseContext::AsmOutput) |
PlaceContext::MutatingUse(MutatingUseContext::Borrow(..)) |
PlaceContext::MutatingUse(MutatingUseContext::Projection) |
PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow(..)) |
PlaceContext::NonMutatingUse(NonMutatingUseContext::UniqueBorrow(..)) |
PlaceContext::NonMutatingUse(NonMutatingUseContext::ShallowBorrow(..)) |
PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection) => {
self.not_ssa(local);
}

PlaceContext::Drop => {
PlaceContext::MutatingUse(MutatingUseContext::Drop) => {
let ty = mir::Place::Local(local).ty(self.fx.mir, self.fx.cx.tcx);
let ty = self.fx.monomorphize(&ty.to_ty(self.fx.cx.tcx));

Expand Down
17 changes: 10 additions & 7 deletions src/librustc_mir/borrow_check/borrow_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ use borrow_check::place_ext::PlaceExt;
use dataflow::indexes::BorrowIndex;
use dataflow::move_paths::MoveData;
use rustc::mir::traversal;
use rustc::mir::visit::{PlaceContext, Visitor};
use rustc::mir::visit::{
PlaceContext, Visitor, NonUseContext, MutatingUseContext, NonMutatingUseContext
};
use rustc::mir::{self, Location, Mir, Place, Local};
use rustc::ty::{Region, TyCtxt};
use rustc::util::nodemap::{FxHashMap, FxHashSet};
Expand Down Expand Up @@ -116,7 +118,7 @@ impl LocalsStateAtExit {

impl<'tcx> Visitor<'tcx> for HasStorageDead {
fn visit_local(&mut self, local: &Local, ctx: PlaceContext<'tcx>, _: Location) {
if ctx == PlaceContext::StorageDead {
if ctx == PlaceContext::NonUse(NonUseContext::StorageDead) {
self.0.insert(*local);
}
}
Expand Down Expand Up @@ -266,7 +268,9 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {

// Watch out: the use of TMP in the borrow itself
// doesn't count as an activation. =)
if borrow_data.reserve_location == location && context == PlaceContext::Store {
if borrow_data.reserve_location == location &&
context == PlaceContext::MutatingUse(MutatingUseContext::Store)
{
return;
}

Expand All @@ -287,10 +291,9 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'gcx, 'tcx> {
borrow_data.activation_location = match context {
// The use of TMP in a shared borrow does not
// count as an actual activation.
PlaceContext::Borrow { kind: mir::BorrowKind::Shared, .. }
| PlaceContext::Borrow { kind: mir::BorrowKind::Shallow, .. } => {
TwoPhaseActivation::NotActivated
}
PlaceContext::NonMutatingUse(NonMutatingUseContext::SharedBorrow(..)) |
PlaceContext::NonMutatingUse(NonMutatingUseContext::ShallowBorrow(..)) =>
TwoPhaseActivation::NotActivated,
_ => {
// Double check: This borrow is indeed a two-phase borrow (that is,
// we are 'transitioning' from `NotActivated` to `ActivatedAt`) and
Expand Down
8 changes: 4 additions & 4 deletions src/librustc_mir/borrow_check/nll/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use rustc::infer::outlives::env::RegionBoundPairs;
use rustc::infer::{InferCtxt, InferOk, LateBoundRegionConversionTime};
use rustc::mir::interpret::EvalErrorKind::BoundsCheck;
use rustc::mir::tcx::PlaceTy;
use rustc::mir::visit::{PlaceContext, Visitor};
use rustc::mir::visit::{PlaceContext, Visitor, MutatingUseContext, NonMutatingUseContext};
use rustc::mir::*;
use rustc::traits::query::type_op;
use rustc::traits::query::type_op::custom::CustomTypeOp;
Expand Down Expand Up @@ -472,9 +472,9 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> {
}
Place::Projection(ref proj) => {
let base_context = if context.is_mutating_use() {
PlaceContext::Projection(Mutability::Mut)
PlaceContext::MutatingUse(MutatingUseContext::Projection)
} else {
PlaceContext::Projection(Mutability::Not)
PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection)
};
let base_ty = self.sanitize_place(&proj.base, location, base_context);
if let PlaceTy::Ty { ty } = base_ty {
Expand All @@ -488,7 +488,7 @@ impl<'a, 'b, 'gcx, 'tcx> TypeVerifier<'a, 'b, 'gcx, 'tcx> {
self.sanitize_projection(base_ty, &proj.elem, place, location)
}
};
if let PlaceContext::Copy = context {
if let PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy) = context {
let tcx = self.tcx();
let trait_ref = ty::TraitRef {
def_id: tcx.lang_items().copy_trait().unwrap(),
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_mir/borrow_check/used_muts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use rustc::mir::{Local, Location, Place};
use rustc_data_structures::fx::FxHashSet;

use borrow_check::MirBorrowckCtxt;
use util::collect_writes::is_place_assignment;

impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
/// Walks the MIR looking for assignments to a set of locals, as part of the unused mutable
Expand Down Expand Up @@ -46,7 +45,7 @@ impl<'visit, 'cx, 'gcx, 'tcx> Visitor<'tcx> for GatherUsedMutsVisitor<'visit, 'c
return;
}

if is_place_assignment(&place_context) {
if place_context.is_place_assignment() {
// Propagate the Local assigned at this Location as a used mutable local variable
for moi in &self.mbcx.move_data.loc_map[location] {
let mpi = &self.mbcx.move_data.moves[*moi].path;
Expand Down
12 changes: 7 additions & 5 deletions src/librustc_mir/transform/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use rustc::hir::Node;
use rustc::hir::def_id::DefId;
use rustc::lint::builtin::{SAFE_EXTERN_STATICS, SAFE_PACKED_BORROWS, UNUSED_UNSAFE};
use rustc::mir::*;
use rustc::mir::visit::{PlaceContext, Visitor};
use rustc::mir::visit::{PlaceContext, Visitor, MutatingUseContext};

use syntax::ast;
use syntax::symbol::Symbol;
Expand Down Expand Up @@ -152,7 +152,7 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
place: &Place<'tcx>,
context: PlaceContext<'tcx>,
location: Location) {
if let PlaceContext::Borrow { .. } = context {
if context.is_borrow() {
if util::is_disaligned(self.tcx, self.mir, self.param_env, place) {
let source_info = self.source_info;
let lint_root =
Expand Down Expand Up @@ -193,9 +193,11 @@ impl<'a, 'tcx> Visitor<'tcx> for UnsafetyChecker<'a, 'tcx> {
}
ty::Adt(adt, _) => {
if adt.is_union() {
if context == PlaceContext::Store ||
context == PlaceContext::AsmOutput ||
context == PlaceContext::Drop
if context == PlaceContext::MutatingUse(MutatingUseContext::Store) ||
context == PlaceContext::MutatingUse(MutatingUseContext::Drop) ||
context == PlaceContext::MutatingUse(
MutatingUseContext::AsmOutput
)
{
let elem_ty = match elem {
&ProjectionElem::Field(_, ty) => ty,
Expand Down
15 changes: 8 additions & 7 deletions src/librustc_mir/transform/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use rustc::hir::def::Def;
use rustc::mir::{Constant, Location, Place, Mir, Operand, Rvalue, Local};
use rustc::mir::{NullOp, UnOp, StatementKind, Statement, BasicBlock, LocalKind};
use rustc::mir::{TerminatorKind, ClearCrossCrate, SourceInfo, BinOp, ProjectionElem};
use rustc::mir::visit::{Visitor, PlaceContext};
use rustc::mir::visit::{Visitor, PlaceContext, MutatingUseContext, NonMutatingUseContext};
use rustc::mir::interpret::{
ConstEvalErr, EvalErrorKind, Scalar, GlobalId, EvalResult,
};
Expand Down Expand Up @@ -533,17 +533,18 @@ impl<'tcx> Visitor<'tcx> for CanConstProp {
// Constants must have at most one write
// FIXME(oli-obk): we could be more powerful here, if the multiple writes
// only occur in independent execution paths
Store => if self.found_assignment[local] {
MutatingUse(MutatingUseContext::Store) => if self.found_assignment[local] {
self.can_const_prop[local] = false;
} else {
self.found_assignment[local] = true
},
// Reading constants is allowed an arbitrary number of times
Copy | Move |
StorageDead | StorageLive |
Validate |
Projection(_) |
Inspect => {},
NonMutatingUse(NonMutatingUseContext::Copy) |
NonMutatingUse(NonMutatingUseContext::Move) |
NonMutatingUse(NonMutatingUseContext::Inspect) |
NonMutatingUse(NonMutatingUseContext::Projection) |
MutatingUse(MutatingUseContext::Projection) |
NonUse(_) => {},
_ => self.can_const_prop[local] = false,
}
}
Expand Down
25 changes: 15 additions & 10 deletions src/librustc_mir/transform/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
//! move analysis runs after promotion on broken MIR.

use rustc::mir::*;
use rustc::mir::visit::{PlaceContext, MutVisitor, Visitor};
use rustc::mir::visit::{PlaceContext, MutatingUseContext, MutVisitor, Visitor};
use rustc::mir::traversal::ReversePostorder;
use rustc::ty::TyCtxt;
use syntax_pos::Span;
Expand Down Expand Up @@ -53,6 +53,7 @@ pub enum TempState {

impl TempState {
pub fn is_promotable(&self) -> bool {
debug!("is_promotable: self={:?}", self);
if let TempState::Defined { uses, .. } = *self {
uses > 0
} else {
Expand Down Expand Up @@ -88,24 +89,30 @@ impl<'tcx> Visitor<'tcx> for TempCollector<'tcx> {
&index: &Local,
context: PlaceContext<'tcx>,
location: Location) {
debug!("visit_local: index={:?} context={:?} location={:?}", index, context, location);
// We're only interested in temporaries
if self.mir.local_kind(index) != LocalKind::Temp {
return;
}

// Ignore drops, if the temp gets promoted,
// then it's constant and thus drop is noop.
// Storage live ranges are also irrelevant.
if context.is_drop() || context.is_storage_marker() {
// Non-uses are also irrelevent.
if context.is_drop() || !context.is_use() {
debug!(
"visit_local: context.is_drop={:?} context.is_use={:?}",
context.is_drop(), context.is_use(),
);
return;
}

let temp = &mut self.temps[index];
debug!("visit_local: temp={:?}", temp);
if *temp == TempState::Undefined {
match context {
PlaceContext::Store |
PlaceContext::AsmOutput |
PlaceContext::Call => {
PlaceContext::MutatingUse(MutatingUseContext::Store) |
PlaceContext::MutatingUse(MutatingUseContext::AsmOutput) |
PlaceContext::MutatingUse(MutatingUseContext::Call) => {
*temp = TempState::Defined {
location,
uses: 0
Expand All @@ -117,10 +124,8 @@ impl<'tcx> Visitor<'tcx> for TempCollector<'tcx> {
} else if let TempState::Defined { ref mut uses, .. } = *temp {
// We always allow borrows, even mutable ones, as we need
// to promote mutable borrows of some ZSTs e.g. `&mut []`.
let allowed_use = match context {
PlaceContext::Borrow {..} => true,
_ => context.is_nonmutating_use()
};
let allowed_use = context.is_borrow() || context.is_nonmutating_use();
debug!("visit_local: allowed_use={:?}", allowed_use);
if allowed_use {
*uses += 1;
return;
Expand Down
Loading

0 comments on commit b3b8760

Please sign in to comment.