Skip to content

Commit

Permalink
Auto merge of #66385 - ecstatic-morse:check-only-pass2, r=eddyb
Browse files Browse the repository at this point in the history
Make dataflow-based const qualification the canonical one

For over a month, dataflow-based const qualification has been running in parallel with `qualify_consts` to check the bodies of `const` and `static`s. This PR removes the old qualification pass completely in favor of the dataflow-based one.

**edit:**
This PR also stops checking `QUALIF_ERROR_BIT` during promotion. This check appears to no longer serve a purpose now that the CTFE engine is more robust.

As a side-effect, this resolves #66167.

r? @eddyb
  • Loading branch information
bors committed Nov 17, 2019
2 parents d801458 + a1135cc commit 0f0c640
Show file tree
Hide file tree
Showing 20 changed files with 296 additions and 1,650 deletions.
11 changes: 11 additions & 0 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2763,6 +2763,17 @@ pub struct BorrowCheckResult<'tcx> {
pub used_mut_upvars: SmallVec<[Field; 8]>,
}

/// The result of the `mir_const_qualif` query.
///
/// Each field corresponds to an implementer of the `Qualif` trait in
/// `librustc_mir/transform/check_consts/qualifs.rs`. See that file for more information on each
/// `Qualif`.
#[derive(Clone, Copy, Debug, Default, RustcEncodable, RustcDecodable, HashStable)]
pub struct ConstQualifs {
pub has_mut_interior: bool,
pub needs_drop: bool,
}

/// After we borrow check a closure, we are left with various
/// requirements that we have inferred between the free regions that
/// appear in the closure's signature or on its field types. These
Expand Down
6 changes: 3 additions & 3 deletions src/librustc/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ rustc_queries! {
}

/// Maps DefId's that have an associated `mir::Body` to the result
/// of the MIR qualify_consts pass. The actual meaning of
/// the value isn't known except to the pass itself.
query mir_const_qualif(key: DefId) -> u8 {
/// of the MIR const-checking pass. This is the set of qualifs in
/// the final value of a `const`.
query mir_const_qualif(key: DefId) -> mir::ConstQualifs {
desc { |tcx| "const checking `{}`", tcx.def_path_str(key) }
cache_on_disk_if { key.is_local() }
}
Expand Down
2 changes: 0 additions & 2 deletions src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1373,8 +1373,6 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
"describes how to render the `rendered` field of json diagnostics"),
unleash_the_miri_inside_of_you: bool = (false, parse_bool, [TRACKED],
"take the breaks off const evaluation. NOTE: this is unsound"),
suppress_const_validation_back_compat_ice: bool = (false, parse_bool, [TRACKED],
"silence ICE triggered when the new const validator disagrees with the old"),
osx_rpath_install_name: bool = (false, parse_bool, [TRACKED],
"pass `-install_name @rpath/...` to the macOS linker"),
sanitizer: Option<Sanitizer> = (None, parse_sanitizer, [TRACKED],
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_metadata/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -952,12 +952,12 @@ impl<'a, 'tcx> CrateMetadata {
.decode((self, tcx))
}

fn mir_const_qualif(&self, id: DefIndex) -> u8 {
fn mir_const_qualif(&self, id: DefIndex) -> mir::ConstQualifs {
match self.kind(id) {
EntryKind::Const(qualif, _) |
EntryKind::AssocConst(AssocContainer::ImplDefault, qualif, _) |
EntryKind::AssocConst(AssocContainer::ImplFinal, qualif, _) => {
qualif.mir
qualif
}
_ => bug!(),
}
Expand Down
21 changes: 13 additions & 8 deletions src/librustc_metadata/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -875,7 +875,11 @@ impl EncodeContext<'tcx> {
hir::print::to_string(self.tcx.hir(), |s| s.print_trait_item(ast_item));
let rendered_const = self.lazy(RenderedConst(rendered));

EntryKind::AssocConst(container, ConstQualif { mir: 0 }, rendered_const)
EntryKind::AssocConst(
container,
Default::default(),
rendered_const,
)
}
ty::AssocKind::Method => {
let fn_data = if let hir::TraitItemKind::Method(m_sig, m) = &ast_item.kind {
Expand Down Expand Up @@ -955,10 +959,11 @@ impl EncodeContext<'tcx> {
record!(self.per_def.kind[def_id] <- match impl_item.kind {
ty::AssocKind::Const => {
if let hir::ImplItemKind::Const(_, body_id) = ast_item.kind {
let mir = self.tcx.at(ast_item.span).mir_const_qualif(def_id);
let qualifs = self.tcx.at(ast_item.span).mir_const_qualif(def_id);

EntryKind::AssocConst(container,
ConstQualif { mir },
EntryKind::AssocConst(
container,
qualifs,
self.encode_rendered_const_for_body(body_id))
} else {
bug!()
Expand Down Expand Up @@ -1089,9 +1094,9 @@ impl EncodeContext<'tcx> {
hir::ItemKind::Static(_, hir::Mutability::Mutable, _) => EntryKind::MutStatic,
hir::ItemKind::Static(_, hir::Mutability::Immutable, _) => EntryKind::ImmStatic,
hir::ItemKind::Const(_, body_id) => {
let mir = self.tcx.at(item.span).mir_const_qualif(def_id);
let qualifs = self.tcx.at(item.span).mir_const_qualif(def_id);
EntryKind::Const(
ConstQualif { mir },
qualifs,
self.encode_rendered_const_for_body(body_id)
)
}
Expand Down Expand Up @@ -1368,9 +1373,9 @@ impl EncodeContext<'tcx> {
let id = self.tcx.hir().as_local_hir_id(def_id).unwrap();
let body_id = self.tcx.hir().body_owned_by(id);
let const_data = self.encode_rendered_const_for_body(body_id);
let mir = self.tcx.mir_const_qualif(def_id);
let qualifs = self.tcx.mir_const_qualif(def_id);

record!(self.per_def.kind[def_id] <- EntryKind::Const(ConstQualif { mir }, const_data));
record!(self.per_def.kind[def_id] <- EntryKind::Const(qualifs, const_data));
record!(self.per_def.visibility[def_id] <- ty::Visibility::Public);
record!(self.per_def.span[def_id] <- self.tcx.def_span(def_id));
self.encode_item_type(def_id);
Expand Down
10 changes: 2 additions & 8 deletions src/librustc_metadata/rmeta/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ crate struct LazyPerDefTables<'tcx> {

#[derive(Copy, Clone, RustcEncodable, RustcDecodable)]
enum EntryKind<'tcx> {
Const(ConstQualif, Lazy<RenderedConst>),
Const(mir::ConstQualifs, Lazy<RenderedConst>),
ImmStatic,
MutStatic,
ForeignImmStatic,
Expand Down Expand Up @@ -288,16 +288,10 @@ enum EntryKind<'tcx> {
Method(Lazy<MethodData>),
AssocType(AssocContainer),
AssocOpaqueTy(AssocContainer),
AssocConst(AssocContainer, ConstQualif, Lazy<RenderedConst>),
AssocConst(AssocContainer, mir::ConstQualifs, Lazy<RenderedConst>),
TraitAlias,
}

/// Additional data for EntryKind::Const and EntryKind::AssocConst
#[derive(Clone, Copy, RustcEncodable, RustcDecodable)]
struct ConstQualif {
mir: u8,
}

/// Contains a constant which has been rendered to a String.
/// Used by rustdoc.
#[derive(RustcEncodable, RustcDecodable)]
Expand Down
1 change: 0 additions & 1 deletion src/librustc_mir/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ Rust MIR: a lowered representation of Rust. Also: an experiment!

#[macro_use] extern crate log;
#[macro_use] extern crate rustc;
#[macro_use] extern crate rustc_data_structures;
#[macro_use] extern crate syntax;

mod borrow_check;
Expand Down
10 changes: 0 additions & 10 deletions src/librustc_mir/transform/check_consts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,16 +95,6 @@ impl ConstKind {
ConstKind::ConstFn | ConstKind::Const => false,
}
}

/// Returns `true` if the value returned by this item must be `Sync`.
///
/// This returns false for `StaticMut` since all accesses to one are `unsafe` anyway.
pub fn requires_sync(self) -> bool {
match self {
ConstKind::Static => true,
ConstKind::ConstFn | ConstKind::Const | ConstKind::StaticMut => false,
}
}
}

impl fmt::Display for ConstKind {
Expand Down
20 changes: 18 additions & 2 deletions src/librustc_mir/transform/check_consts/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,15 @@ impl NonConstOp for HeapAllocation {

#[derive(Debug)]
pub struct IfOrMatch;
impl NonConstOp for IfOrMatch {}
impl NonConstOp for IfOrMatch {
fn emit_error(&self, item: &Item<'_, '_>, span: Span) {
// This should be caught by the HIR const-checker.
item.tcx.sess.delay_span_bug(
span,
"complex control flow is forbidden in a const context",
);
}
}

#[derive(Debug)]
pub struct LiveDrop;
Expand All @@ -154,7 +162,15 @@ impl NonConstOp for LiveDrop {

#[derive(Debug)]
pub struct Loop;
impl NonConstOp for Loop {}
impl NonConstOp for Loop {
fn emit_error(&self, item: &Item<'_, '_>, span: Span) {
// This should be caught by the HIR const-checker.
item.tcx.sess.delay_span_bug(
span,
"complex control flow is forbidden in a const context",
);
}
}

#[derive(Debug)]
pub struct MutBorrow(pub BorrowKind);
Expand Down
29 changes: 16 additions & 13 deletions src/librustc_mir/transform/check_consts/qualifs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,10 @@ use syntax_pos::DUMMY_SP;

use super::{ConstKind, Item as ConstCx};

#[derive(Clone, Copy)]
pub struct QualifSet(u8);

impl QualifSet {
fn contains<Q: ?Sized + Qualif>(self) -> bool {
self.0 & (1 << Q::IDX) != 0
pub fn in_any_value_of_ty(cx: &ConstCx<'_, 'tcx>, ty: Ty<'tcx>) -> ConstQualifs {
ConstQualifs {
has_mut_interior: HasMutInterior::in_any_value_of_ty(cx, ty),
needs_drop: NeedsDrop::in_any_value_of_ty(cx, ty),
}
}

Expand All @@ -22,14 +20,14 @@ impl QualifSet {
///
/// The default implementations proceed structurally.
pub trait Qualif {
const IDX: usize;

/// The name of the file used to debug the dataflow analysis that computes this qualif.
const ANALYSIS_NAME: &'static str;

/// Whether this `Qualif` is cleared when a local is moved from.
const IS_CLEARED_ON_MOVE: bool = false;

fn in_qualifs(qualifs: &ConstQualifs) -> bool;

/// Return the qualification that is (conservatively) correct for any value
/// of the type.
fn in_any_value_of_ty(_cx: &ConstCx<'_, 'tcx>, _ty: Ty<'tcx>) -> bool;
Expand Down Expand Up @@ -122,9 +120,8 @@ pub trait Qualif {
if cx.tcx.trait_of_item(def_id).is_some() {
Self::in_any_value_of_ty(cx, constant.literal.ty)
} else {
let bits = cx.tcx.at(constant.span).mir_const_qualif(def_id);

let qualif = QualifSet(bits).contains::<Self>();
let qualifs = cx.tcx.at(constant.span).mir_const_qualif(def_id);
let qualif = Self::in_qualifs(&qualifs);

// Just in case the type is more specific than
// the definition, e.g., impl associated const
Expand Down Expand Up @@ -210,9 +207,12 @@ pub trait Qualif {
pub struct HasMutInterior;

impl Qualif for HasMutInterior {
const IDX: usize = 0;
const ANALYSIS_NAME: &'static str = "flow_has_mut_interior";

fn in_qualifs(qualifs: &ConstQualifs) -> bool {
qualifs.has_mut_interior
}

fn in_any_value_of_ty(cx: &ConstCx<'_, 'tcx>, ty: Ty<'tcx>) -> bool {
!ty.is_freeze(cx.tcx, cx.param_env, DUMMY_SP)
}
Expand Down Expand Up @@ -275,10 +275,13 @@ impl Qualif for HasMutInterior {
pub struct NeedsDrop;

impl Qualif for NeedsDrop {
const IDX: usize = 1;
const ANALYSIS_NAME: &'static str = "flow_needs_drop";
const IS_CLEARED_ON_MOVE: bool = true;

fn in_qualifs(qualifs: &ConstQualifs) -> bool {
qualifs.needs_drop
}

fn in_any_value_of_ty(cx: &ConstCx<'_, 'tcx>, ty: Ty<'tcx>) -> bool {
ty.needs_drop(cx.tcx, cx.param_env)
}
Expand Down
Loading

0 comments on commit 0f0c640

Please sign in to comment.