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.

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

Make dataflow-based const qualification the canonical one #66385

Merged
merged 17 commits into from
Nov 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2769,6 +2769,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,
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if these two fields should have similar doc comments to the HasMutInterior and NeedsDrop types in rustc_mir::transform::check_consts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would a link to the Qualif implementers suffice? I don't wanna copy-paste lest they get out of sync.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I wasn't thinking copy-pasting, maybe a few words and a link (or even just the rustc_mir::...::HasMutInterior etc. path).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a note on ConstQualifs that points the user to librustc_mir/transform/check_consts/qualifs.rs. Lemme know if you want something more here.

}

/// 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