Skip to content
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

Retire IsNotConst naming #59796

Merged
merged 7 commits into from
Apr 14, 2019
Merged

Retire IsNotConst naming #59796

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b8e9da7
`asm!` output can neither be promotable nor const
oli-obk Apr 8, 2019
6b01844
Generalize initial "not const" assignments
oli-obk Apr 8, 2019
ec52065
Explicit promotion is indistinguishable from explicit promotion
oli-obk Apr 8, 2019
d3d5673
Get rid of "is not const" naming
oli-obk Apr 8, 2019
c63b9ff
Pacify tidy
oli-obk Apr 8, 2019
ae4717d
Elaborate on implicit promotability
oli-obk Apr 8, 2019
f10394a
Documentation should have proper grammar
oli-obk Apr 10, 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
16 changes: 10 additions & 6 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 @@ -44,8 +44,8 @@ pub enum TempState {
impl TempState {
pub fn is_promotable(&self) -> bool {
debug!("is_promotable: self={:?}", self);
if let TempState::Defined { uses, .. } = *self {
uses > 0
if let TempState::Defined { .. } = *self {
true
} else {
false
}
Expand Down Expand Up @@ -80,9 +80,14 @@ impl<'tcx> Visitor<'tcx> for TempCollector<'tcx> {
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;
// We're only interested in temporaries and the return place
match self.mir.local_kind(index) {
| LocalKind::Temp
| LocalKind::ReturnPointer
=> {},
| LocalKind::Arg
| LocalKind::Var
=> return,
}

// Ignore drops, if the temp gets promoted,
Expand All @@ -101,7 +106,6 @@ impl<'tcx> Visitor<'tcx> for TempCollector<'tcx> {
if *temp == TempState::Undefined {
match context {
PlaceContext::MutatingUse(MutatingUseContext::Store) |
PlaceContext::MutatingUse(MutatingUseContext::AsmOutput) |
PlaceContext::MutatingUse(MutatingUseContext::Call) => {
*temp = TempState::Defined {
location,
Expand Down
75 changes: 37 additions & 38 deletions src/librustc_mir/transform/qualify_consts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,11 +365,11 @@ impl Qualif for NeedsDrop {
}
}

// Not constant at all - non-`const fn` calls, asm!,
// Not promotable at all - non-`const fn` calls, asm!,
// pointer comparisons, ptr-to-int casts, etc.
struct IsNotConst;
struct IsNotPromotable;

impl Qualif for IsNotConst {
impl Qualif for IsNotPromotable {
const IDX: usize = 2;

fn in_static(cx: &ConstCx<'_, 'tcx>, static_: &Static<'tcx>) -> bool {
Expand Down Expand Up @@ -508,13 +508,14 @@ impl Qualif for IsNotConst {
}
}

// Refers to temporaries which cannot be promoted as
// promote_consts decided they weren't simple enough.
// FIXME(oli-obk,eddyb): Remove this flag entirely and
// solely process this information via `IsNotConst`.
struct IsNotPromotable;
/// Refers to temporaries which cannot be promoted *implicitly*.
/// Explicit promotion e.g. for constant arguments declared via `rustc_args_required_const` or by
Copy link
Member

Choose a reason for hiding this comment

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

This still doesn't read like a complete sentence to me, did you mean to add an "is" before "e.g."?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm... that sentence sounded better when I wrote it XD I must have lost my train of thought while writing it. Better now?

/// happening inside a constant, static or const fn. Inside a const context all constness rules
/// apply, so implicit promotion simply has to follow the regular constant rules (modulo interior
/// mutability or `Drop` rules which are handled `HasMutInterior` and `NeedsDrop` respectively)
struct IsNotImplicitlyPromotable;

impl Qualif for IsNotPromotable {
impl Qualif for IsNotImplicitlyPromotable {
const IDX: usize = 3;

fn in_call(
Expand Down Expand Up @@ -550,33 +551,36 @@ macro_rules! static_assert_seq_qualifs {
static_assert!(SEQ_QUALIFS: QUALIF_COUNT == $i);
};
}
static_assert_seq_qualifs!(0 => HasMutInterior, NeedsDrop, IsNotConst, IsNotPromotable);
static_assert_seq_qualifs!(
0 => HasMutInterior, NeedsDrop, IsNotPromotable, IsNotImplicitlyPromotable
);

impl ConstCx<'_, 'tcx> {
fn qualifs_in_any_value_of_ty(&self, ty: Ty<'tcx>) -> PerQualif<bool> {
let mut qualifs = PerQualif::default();
qualifs[HasMutInterior] = HasMutInterior::in_any_value_of_ty(self, ty).unwrap_or(false);
qualifs[NeedsDrop] = NeedsDrop::in_any_value_of_ty(self, ty).unwrap_or(false);
qualifs[IsNotConst] = IsNotConst::in_any_value_of_ty(self, ty).unwrap_or(false);
qualifs[IsNotPromotable] = IsNotPromotable::in_any_value_of_ty(self, ty).unwrap_or(false);
qualifs[IsNotImplicitlyPromotable] =
IsNotImplicitlyPromotable::in_any_value_of_ty(self, ty).unwrap_or(false);
qualifs
}

fn qualifs_in_local(&self, local: Local) -> PerQualif<bool> {
let mut qualifs = PerQualif::default();
qualifs[HasMutInterior] = HasMutInterior::in_local(self, local);
qualifs[NeedsDrop] = NeedsDrop::in_local(self, local);
qualifs[IsNotConst] = IsNotConst::in_local(self, local);
qualifs[IsNotPromotable] = IsNotPromotable::in_local(self, local);
qualifs[IsNotImplicitlyPromotable] = IsNotImplicitlyPromotable::in_local(self, local);
qualifs
}

fn qualifs_in_value(&self, source: ValueSource<'_, 'tcx>) -> PerQualif<bool> {
let mut qualifs = PerQualif::default();
qualifs[HasMutInterior] = HasMutInterior::in_value(self, source);
qualifs[NeedsDrop] = NeedsDrop::in_value(self, source);
qualifs[IsNotConst] = IsNotConst::in_value(self, source);
qualifs[IsNotPromotable] = IsNotPromotable::in_value(self, source);
qualifs[IsNotImplicitlyPromotable] = IsNotImplicitlyPromotable::in_value(self, source);
qualifs
}
}
Expand Down Expand Up @@ -631,26 +635,21 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
};

for (local, decl) in mir.local_decls.iter_enumerated() {
match mir.local_kind(local) {
LocalKind::Arg => {
let qualifs = cx.qualifs_in_any_value_of_ty(decl.ty);
for (per_local, qualif) in &mut cx.per_local.as_mut().zip(qualifs).0 {
if *qualif {
per_local.insert(local);
}
if let LocalKind::Arg = mir.local_kind(local) {
let qualifs = cx.qualifs_in_any_value_of_ty(decl.ty);
for (per_local, qualif) in &mut cx.per_local.as_mut().zip(qualifs).0 {
if *qualif {
per_local.insert(local);
}
cx.per_local[IsNotConst].insert(local);
}

LocalKind::Var if mode == Mode::Fn => {
cx.per_local[IsNotConst].insert(local);
}

LocalKind::Temp if !temps[local].is_promotable() => {
cx.per_local[IsNotConst].insert(local);
}

_ => {}
}
if !temps[local].is_promotable() {
cx.per_local[IsNotPromotable].insert(local);
}
if let LocalKind::Var = mir.local_kind(local) {
// Sanity check to prevent implicit and explicit promotion of
// named locals
assert!(cx.per_local[IsNotPromotable].contains(local));
}
}

Expand Down Expand Up @@ -698,11 +697,11 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
// the borrowed place is disallowed from being borrowed,
// due to either a mutable borrow (with some exceptions),
// or an shared borrow of a value with interior mutability.
// Then `HasMutInterior` is replaced with `IsNotConst`,
// Then `HasMutInterior` is replaced with `IsNotPromotable`,
// to avoid duplicate errors (e.g. from reborrowing).
if qualifs[HasMutInterior] {
qualifs[HasMutInterior] = false;
qualifs[IsNotConst] = true;
qualifs[IsNotPromotable] = true;

if self.mode != Mode::Fn {
if let BorrowKind::Mut { .. } = kind {
Expand Down Expand Up @@ -817,15 +816,15 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {
}
}

// Ensure the `IsNotConst` qualification is preserved.
// Ensure the `IsNotPromotable` qualification is preserved.
// NOTE(eddyb) this is actually unnecessary right now, as
// we never replace the local's qualif, but we might in
// the future, and so it serves to catch changes that unset
// important bits (in which case, asserting `contains` could
// be replaced with calling `insert` to re-set the bit).
if kind == LocalKind::Temp {
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
if !self.temp_promotion_state[index].is_promotable() {
assert!(self.cx.per_local[IsNotConst].contains(index));
assert!(self.cx.per_local[IsNotPromotable].contains(index));
}
}
}
Expand Down Expand Up @@ -911,7 +910,7 @@ impl<'a, 'tcx> Checker<'a, 'tcx> {

// Account for errors in consts by using the
// conservative type qualification instead.
if qualifs[IsNotConst] {
if qualifs[IsNotPromotable] {
qualifs = self.qualifs_in_any_value_of_ty(mir.return_ty());
}

Expand Down Expand Up @@ -1326,7 +1325,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Checker<'a, 'tcx> {
// which happens even without the user requesting it.
// We can error out with a hard error if the argument is not
// constant here.
if !IsNotConst::in_operand(self, arg) {
if !IsNotPromotable::in_operand(self, arg) {
debug!("visit_terminator_kind: candidate={:?}", candidate);
self.promotion_candidates.push(candidate);
} else {
Expand Down Expand Up @@ -1444,7 +1443,7 @@ fn mir_const_qualif<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,

if mir.return_ty().references_error() {
tcx.sess.delay_span_bug(mir.span, "mir_const_qualif: Mir had errors");
return (1 << IsNotConst::IDX, Lrc::new(BitSet::new_empty(0)));
return (1 << IsNotPromotable::IDX, Lrc::new(BitSet::new_empty(0)));
}

Checker::new(tcx, def_id, mir, Mode::Const).check_const()
Expand Down