Skip to content

Use new promoter to emit errors when required promotion fails #65942

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

Use new promoter to emit errors when required promotion fails #65942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
75a39a8
Add method to `Candidate` that determines its promotability rules
ecstatic-morse Oct 29, 2019
605b0d8
Set explicit flag inside `validate_candidate`
ecstatic-morse Oct 29, 2019
613c4b7
Emit errors in `promote_consts` when explicit promotion fails
ecstatic-morse Oct 29, 2019
1602709
Promote the same basic blocks as `qualify_consts` does
ecstatic-morse Oct 29, 2019
d99b7b4
Consider the const context when determining `explicit`
ecstatic-morse Oct 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
205 changes: 139 additions & 66 deletions src/librustc_mir/transform/promote_consts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use syntax::symbol::sym;
use syntax_pos::{Span, DUMMY_SP};

use rustc_index::vec::{IndexVec, Idx};
use rustc_index::bit_set::BitSet;
use rustc_target::spec::abi::Abi;

use std::{iter, mem, usize};
Expand Down Expand Up @@ -80,6 +81,17 @@ pub enum Candidate {
Argument { bb: BasicBlock, index: usize },
}

impl Candidate {
/// Returns `true` if we should use the "explicit" rules for promotability for this `Candidate`.
fn is_explicit_context(&self) -> bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"context" doesn't feel right here. Maybe forces_explicit_promotion?

match self {
Candidate::Ref(_) |
Candidate::Repeat(_) => false,
Candidate::Argument { .. } => true,
}
}
}

fn args_required_const(tcx: TyCtxt<'_>, def_id: DefId) -> Option<Vec<usize>> {
let attrs = tcx.get_attrs(def_id);
let attr = attrs.iter().find(|a| a.check_name(sym::rustc_args_required_const))?;
Expand All @@ -93,15 +105,12 @@ fn args_required_const(tcx: TyCtxt<'_>, def_id: DefId) -> Option<Vec<usize>> {
Some(ret)
}

struct Collector<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
struct TempStateCollector<'a, 'tcx> {
body: &'a Body<'tcx>,
temps: IndexVec<Local, TempState>,
candidates: Vec<Candidate>,
span: Span,
}

impl<'tcx> Visitor<'tcx> for Collector<'_, 'tcx> {
impl<'tcx> Visitor<'tcx> for TempStateCollector<'_, 'tcx> {
fn visit_local(&mut self,
&index: &Local,
context: PlaceContext,
Expand Down Expand Up @@ -155,18 +164,34 @@ impl<'tcx> Visitor<'tcx> for Collector<'_, 'tcx> {
}
*temp = TempState::Unpromotable;
}
}

struct PromotionCandidateCollector<'mir, 'tcx> {
validator: Validator<'mir, 'tcx>,
candidates: Vec<Candidate>,
span: Span,
}

impl<'tcx> Visitor<'tcx> for PromotionCandidateCollector<'_, 'tcx> {
fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, location: Location) {
self.super_rvalue(rvalue, location);

let tcx = self.validator.tcx;

match *rvalue {
Rvalue::Ref(..) => {
self.candidates.push(Candidate::Ref(location));
let candidate = Candidate::Ref(location);
if self.validator.validate_candidate(candidate).is_ok() {
self.candidates.push(candidate);
}
}
Rvalue::Repeat(..) if self.tcx.features().const_in_array_repeat_expressions => {
Rvalue::Repeat(..) if tcx.features().const_in_array_repeat_expressions => {
// FIXME(#49147) only promote the element when it isn't `Copy`
// (so that code that can copy it at runtime is unaffected).
self.candidates.push(Candidate::Repeat(location));
let candidate = Candidate::Repeat(location);
if self.validator.validate_candidate(candidate).is_ok() {
self.candidates.push(candidate);
}
}
_ => {}
}
Expand All @@ -177,23 +202,40 @@ impl<'tcx> Visitor<'tcx> for Collector<'_, 'tcx> {
location: Location) {
self.super_terminator_kind(kind, location);

let Item { tcx, body, .. } = *self.validator.item;

if let TerminatorKind::Call { ref func, .. } = *kind {
if let ty::FnDef(def_id, _) = func.ty(self.body, self.tcx).kind {
let fn_sig = self.tcx.fn_sig(def_id);
if let ty::FnDef(def_id, _) = func.ty(body, tcx).kind {
let fn_sig = tcx.fn_sig(def_id);
if let Abi::RustIntrinsic | Abi::PlatformIntrinsic = fn_sig.abi() {
let name = self.tcx.item_name(def_id);
let name = tcx.item_name(def_id);
// FIXME(eddyb) use `#[rustc_args_required_const(2)]` for shuffles.
if name.as_str().starts_with("simd_shuffle") {
self.candidates.push(Candidate::Argument {
bb: location.block,
index: 2,
});
let candidate = Candidate::Argument { bb: location.block, index: 2 };

if self.validator.validate_candidate(candidate).is_ok() {
self.candidates.push(candidate);
} else {
span_err!(
tcx.sess, self.span, E0526,
"shuffle indices are not constant",
);
}
}
}

if let Some(constant_args) = args_required_const(self.tcx, def_id) {
if let Some(constant_args) = args_required_const(tcx, def_id) {
for index in constant_args {
self.candidates.push(Candidate::Argument { bb: location.block, index });
let candidate = Candidate::Argument { bb: location.block, index };

if self.validator.validate_candidate(candidate).is_ok() {
self.candidates.push(candidate);
} else {
tcx.sess.span_err(
self.span,
&format!("argument {} is required to be a constant", index + 1),
);
}
}
}
}
Expand All @@ -205,29 +247,90 @@ impl<'tcx> Visitor<'tcx> for Collector<'_, 'tcx> {
}
}

pub fn collect_temps_and_candidates(
tcx: TyCtxt<'tcx>,
body: &Body<'tcx>,
rpo: &mut ReversePostorder<'_, 'tcx>,
pub fn collect_temps_and_valid_candidates(
item: &Item<'_, 'tcx>,
mut rpo: &mut ReversePostorder<'_, 'tcx>,
) -> (IndexVec<Local, TempState>, Vec<Candidate>) {
let mut collector = Collector {
tcx,
body,
temps: IndexVec::from_elem(TempState::Undefined, &body.local_decls),
let mut temp_state = TempStateCollector {
body: item.body,
temps: IndexVec::from_elem(TempState::Undefined, &item.body.local_decls),
};

for (bb, data) in &mut rpo {
temp_state.visit_basic_block_data(bb, data);
}

let mut promotion = PromotionCandidateCollector {
span: item.body.span,
candidates: vec![],
span: body.span,
validator: Validator {
item,
temps: &temp_state.temps,
explicit: false,
},
};
for (bb, data) in rpo {
collector.visit_basic_block_data(bb, data);

if item.const_kind.is_none() {
rpo.reset();
for (bb, data) in &mut rpo {
promotion.visit_basic_block_data(bb, data);
}
} else {
// FIXME: This strange MIR traversal is a temporary measure to ensure that we try to
// promote the same things as the old promotion logic in `qualify_consts`.
let mut seen_blocks = BitSet::new_empty(item.body.basic_blocks().len());
let mut bb = START_BLOCK;
loop {
seen_blocks.insert(bb.index());

promotion.visit_basic_block_data(bb, &item.body[bb]);

let target = match item.body[bb].terminator().kind {
TerminatorKind::Goto { target } |
TerminatorKind::FalseUnwind { real_target: target, .. } |
TerminatorKind::Drop { target, .. } |
TerminatorKind::DropAndReplace { target, .. } |
TerminatorKind::Assert { target, .. } |
TerminatorKind::Call { destination: Some((_, target)), .. } => {
Some(target)
}

// Non-terminating calls cannot produce any value.
TerminatorKind::Call { destination: None, .. } => {
break;
}

TerminatorKind::SwitchInt {..} |
TerminatorKind::Resume |
TerminatorKind::Abort |
TerminatorKind::GeneratorDrop |
TerminatorKind::Yield { .. } |
TerminatorKind::Unreachable |
TerminatorKind::FalseEdges { .. } => None,

TerminatorKind::Return => {
break;
}
};

match target {
Some(target) if !seen_blocks.contains(target.index()) => {
bb = target;
}
_ => break,
}
}
}
(collector.temps, collector.candidates)

let candidates = promotion.candidates;
(temp_state.temps, candidates)
}

/// Checks whether locals that appear in a promotion context (`Candidate`) are actually promotable.
///
/// This wraps an `Item`, and has access to all fields of that `Item` via `Deref` coercion.
struct Validator<'a, 'tcx> {
item: Item<'a, 'tcx>,
item: &'a Item<'a, 'tcx>,
temps: &'a IndexVec<Local, TempState>,

/// Explicit promotion happens e.g. for constant arguments declared via
Expand All @@ -249,12 +352,14 @@ impl std::ops::Deref for Validator<'a, 'tcx> {

struct Unpromotable;

// FIXME(eddyb) remove the differences for promotability in `static`, `const`, `const fn`.
impl<'tcx> Validator<'_, 'tcx> {
fn validate_candidate(&self, candidate: Candidate) -> Result<(), Unpromotable> {
fn validate_candidate(&mut self, candidate: Candidate) -> Result<(), Unpromotable> {
// Always use "explicit" rules for promotability inside a `const`, `static` or `const fn`.
self.explicit = candidate.is_explicit_context() || self.const_kind.is_some();

match candidate {
Candidate::Ref(loc) => {
assert!(!self.explicit);

let statement = &self.body[loc.block].statements[loc.statement_index];
match &statement.kind {
StatementKind::Assign(box(_, Rvalue::Ref(_, kind, place))) => {
Expand Down Expand Up @@ -345,8 +450,6 @@ impl<'tcx> Validator<'_, 'tcx> {
}
}
Candidate::Repeat(loc) => {
assert!(!self.explicit);

let statement = &self.body[loc.block].statements[loc.statement_index];
match &statement.kind {
StatementKind::Assign(box(_, Rvalue::Repeat(ref operand, _))) => {
Expand All @@ -360,8 +463,6 @@ impl<'tcx> Validator<'_, 'tcx> {
}
},
Candidate::Argument { bb, index } => {
assert!(self.explicit);

let terminator = self.body[bb].terminator();
match &terminator.kind {
TerminatorKind::Call { args, .. } => {
Expand Down Expand Up @@ -681,7 +782,7 @@ impl<'tcx> Validator<'_, 'tcx> {
) -> Result<(), Unpromotable> {
let fn_ty = callee.ty(self.body, self.tcx);

if !self.explicit && self.const_kind.is_none() {
if !self.explicit {
if let ty::FnDef(def_id, _) = fn_ty.kind {
// Never promote runtime `const fn` calls of
// functions without `#[rustc_promotable]`.
Expand Down Expand Up @@ -712,34 +813,6 @@ impl<'tcx> Validator<'_, 'tcx> {
}
}

// FIXME(eddyb) remove the differences for promotability in `static`, `const`, `const fn`.
pub fn validate_candidates(
tcx: TyCtxt<'tcx>,
body: &Body<'tcx>,
def_id: DefId,
temps: &IndexVec<Local, TempState>,
candidates: &[Candidate],
) -> Vec<Candidate> {
let mut validator = Validator {
item: Item::new(tcx, def_id, body),
temps,
explicit: false,
};

candidates.iter().copied().filter(|&candidate| {
validator.explicit = match candidate {
Candidate::Ref(_) |
Candidate::Repeat(_) => false,
Candidate::Argument { .. } => true,
};

// FIXME(eddyb) also emit the errors for shuffle indices
// and `#[rustc_args_required_const]` arguments here.

validator.validate_candidate(candidate).is_ok()
}).collect()
}

struct Promoter<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
source: &'a mut Body<'tcx>,
Expand Down
Loading