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

Const qualification for StructuralEq #67343

Merged
merged 10 commits into from
Apr 29, 2020
1 change: 1 addition & 0 deletions src/librustc_middle/mir/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ pub struct BorrowCheckResult<'tcx> {
pub struct ConstQualifs {
pub has_mut_interior: bool,
pub needs_drop: bool,
pub custom_eq: bool,
}

/// After we borrow check a closure, we are left with various
Expand Down
52 changes: 46 additions & 6 deletions src/librustc_mir/transform/check_consts/qualifs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,19 @@
//!
//! See the `Qualif` trait for more info.

use rustc_infer::infer::TyCtxtInferExt;
use rustc_middle::mir::*;
use rustc_middle::ty::{self, AdtDef, Ty};
use rustc_middle::ty::{self, subst::SubstsRef, AdtDef, Ty};
use rustc_span::DUMMY_SP;
use rustc_trait_selection::traits;

use super::ConstCx;

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),
custom_eq: CustomEq::in_any_value_of_ty(cx, ty),
}
}

Expand Down Expand Up @@ -53,7 +56,11 @@ pub trait Qualif {
/// with a custom `Drop` impl is inherently `NeedsDrop`.
///
/// Returning `true` for `in_adt_inherently` but `false` for `in_any_value_of_ty` is unsound.
fn in_adt_inherently(cx: &ConstCx<'_, 'tcx>, adt: &AdtDef) -> bool;
fn in_adt_inherently(
cx: &ConstCx<'_, 'tcx>,
adt: &'tcx AdtDef,
substs: SubstsRef<'tcx>,
) -> bool;
}

/// Constant containing interior mutability (`UnsafeCell<T>`).
Expand All @@ -74,7 +81,7 @@ impl Qualif for HasMutInterior {
!ty.is_freeze(cx.tcx, cx.param_env, DUMMY_SP)
}

fn in_adt_inherently(cx: &ConstCx<'_, 'tcx>, adt: &AdtDef) -> bool {
fn in_adt_inherently(cx: &ConstCx<'_, 'tcx>, adt: &'tcx AdtDef, _: SubstsRef<'tcx>) -> bool {
// Exactly one type, `UnsafeCell`, has the `HasMutInterior` qualif inherently.
// It arises structurally for all other types.
Some(adt.did) == cx.tcx.lang_items().unsafe_cell_type()
Expand All @@ -99,11 +106,44 @@ impl Qualif for NeedsDrop {
ty.needs_drop(cx.tcx, cx.param_env)
}

fn in_adt_inherently(cx: &ConstCx<'_, 'tcx>, adt: &AdtDef) -> bool {
fn in_adt_inherently(cx: &ConstCx<'_, 'tcx>, adt: &'tcx AdtDef, _: SubstsRef<'tcx>) -> bool {
ecstatic-morse marked this conversation as resolved.
Show resolved Hide resolved
adt.has_dtor(cx.tcx)
}
}

/// A constant that cannot be used as part of a pattern in a `match` expression.
pub struct CustomEq;

impl Qualif for CustomEq {
const ANALYSIS_NAME: &'static str = "flow_custom_eq";

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

fn in_any_value_of_ty(cx: &ConstCx<'_, 'tcx>, ty: Ty<'tcx>) -> bool {
// If *any* component of a composite data type does not implement `Structural{Partial,}Eq`,
// we know that at least some values of that type are not structural-match. I say "some"
// because that component may be part of an enum variant (e.g.,
// `Option::<NonStructuralMatchTy>::Some`), in which case some values of this type may be
// structural-match (`Option::None`).
let id = cx.tcx.hir().local_def_id_to_hir_id(cx.def_id.as_local().unwrap());
traits::search_for_structural_match_violation(id, cx.body.span, cx.tcx, ty).is_some()
ecstatic-morse marked this conversation as resolved.
Show resolved Hide resolved
}

fn in_adt_inherently(
cx: &ConstCx<'_, 'tcx>,
adt: &'tcx AdtDef,
substs: SubstsRef<'tcx>,
) -> bool {
let ty = cx.tcx.mk_ty(ty::Adt(adt, substs));
let id = cx.tcx.hir().local_def_id_to_hir_id(cx.def_id.as_local().unwrap());
cx.tcx
.infer_ctxt()
.enter(|infcx| !traits::type_marked_structural(id, cx.body.span, &infcx, ty))
}
}

// FIXME: Use `mir::visit::Visitor` for the `in_*` functions if/when it supports early return.

/// Returns `true` if this `Rvalue` contains qualif `Q`.
Expand Down Expand Up @@ -147,8 +187,8 @@ where
Rvalue::Aggregate(kind, operands) => {
// Return early if we know that the struct or enum being constructed is always
// qualified.
if let AggregateKind::Adt(def, ..) = **kind {
if Q::in_adt_inherently(cx, def) {
if let AggregateKind::Adt(def, _, substs, ..) = **kind {
if Q::in_adt_inherently(cx, def, substs) {
return true;
}
}
Expand Down
28 changes: 27 additions & 1 deletion src/librustc_mir/transform/check_consts/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use std::borrow::Cow;
use std::ops::Deref;

use super::ops::{self, NonConstOp};
use super::qualifs::{self, HasMutInterior, NeedsDrop};
use super::qualifs::{self, CustomEq, HasMutInterior, NeedsDrop};
use super::resolver::FlowSensitiveAnalysis;
use super::{is_lang_panic_fn, ConstCx, ConstKind, Qualif};
use crate::const_eval::{is_const_fn, is_unstable_const_fn};
Expand Down Expand Up @@ -142,9 +142,35 @@ impl Qualifs<'mir, 'tcx> {

let return_loc = ccx.body.terminator_loc(return_block);

let custom_eq = match ccx.const_kind() {
// We don't care whether a `const fn` returns a value that is not structurally
// matchable. Functions calls are opaque and always use type-based qualification, so
// this value should never be used.
ConstKind::ConstFn => true,

// If we know that all values of the return type are structurally matchable, there's no
// need to run dataflow.
ConstKind::Const | ConstKind::Static | ConstKind::StaticMut
if !CustomEq::in_any_value_of_ty(ccx, ccx.body.return_ty()) =>
{
false
}

ConstKind::Const | ConstKind::Static | ConstKind::StaticMut => {
let mut cursor = FlowSensitiveAnalysis::new(CustomEq, ccx)
.into_engine(ccx.tcx, &ccx.body, ccx.def_id)
.iterate_to_fixpoint()
.into_results_cursor(&ccx.body);

cursor.seek_after(return_loc);
cursor.contains(RETURN_PLACE)
}
};

ConstQualifs {
needs_drop: self.needs_drop(ccx, RETURN_PLACE, return_loc),
has_mut_interior: self.has_mut_interior(ccx, RETURN_PLACE, return_loc),
custom_eq,
}
}
}
Expand Down
21 changes: 18 additions & 3 deletions src/librustc_mir_build/hair/pattern/const_to_pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,14 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
cv: &'tcx ty::Const<'tcx>,
id: hir::HirId,
span: Span,
mir_structural_match_violation: bool,
) -> Pat<'tcx> {
debug!("const_to_pat: cv={:#?} id={:?}", cv, id);
debug!("const_to_pat: cv.ty={:?} span={:?}", cv.ty, span);

self.tcx.infer_ctxt().enter(|infcx| {
let mut convert = ConstToPat::new(self, id, span, infcx);
convert.to_pat(cv)
convert.to_pat(cv, mir_structural_match_violation)
})
}
}
Expand Down Expand Up @@ -81,7 +82,11 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
traits::type_marked_structural(self.id, self.span, &self.infcx, ty)
}

fn to_pat(&mut self, cv: &'tcx ty::Const<'tcx>) -> Pat<'tcx> {
fn to_pat(
&mut self,
cv: &'tcx ty::Const<'tcx>,
mir_structural_match_violation: bool,
) -> Pat<'tcx> {
// This method is just a wrapper handling a validity check; the heavy lifting is
// performed by the recursive `recur` method, which is not meant to be
// invoked except by this method.
Expand All @@ -100,6 +105,11 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
"search_for_structural_match_violation cv.ty: {:?} returned: {:?}",
cv.ty, structural
);

if structural.is_none() && mir_structural_match_violation {
bug!("MIR const-checker found novel structural match violation");
}

if let Some(non_sm_ty) = structural {
let adt_def = match non_sm_ty {
traits::NonStructuralMatchTy::Adt(adt_def) => adt_def,
Expand Down Expand Up @@ -146,13 +156,18 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {
if !ty_is_partial_eq {
// span_fatal avoids ICE from resolution of non-existent method (rare case).
self.tcx().sess.span_fatal(self.span, &make_msg());
} else {
} else if mir_structural_match_violation {
self.tcx().struct_span_lint_hir(
lint::builtin::INDIRECT_STRUCTURAL_MATCH,
self.id,
self.span,
|lint| lint.build(&make_msg()).emit(),
);
} else {
debug!(
"`search_for_structural_match_violation` found one, but `CustomEq` was \
not in the qualifs for that `const`"
);
}
}
}
Expand Down
131 changes: 71 additions & 60 deletions src/librustc_mir_build/hair/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
use rustc_hir::pat_util::EnumerateAndAdjustIterator;
use rustc_hir::RangeEnd;
use rustc_index::vec::Idx;
use rustc_middle::mir::interpret::{get_slice_bytes, sign_extend, ConstValue, ErrorHandled};
use rustc_middle::mir::interpret::{get_slice_bytes, sign_extend, ConstValue};
use rustc_middle::mir::interpret::{LitToConstError, LitToConstInput};
use rustc_middle::mir::UserTypeProjection;
use rustc_middle::mir::{BorrowKind, Field, Mutability};
Expand Down Expand Up @@ -762,69 +762,80 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
fn lower_path(&mut self, qpath: &hir::QPath<'_>, id: hir::HirId, span: Span) -> Pat<'tcx> {
let ty = self.tables.node_type(id);
let res = self.tables.qpath_res(qpath, id);
let is_associated_const = match res {
Res::Def(DefKind::AssocConst, _) => true,
_ => false,

let pat_from_kind = |kind| Pat { span, ty, kind: Box::new(kind) };

let (def_id, is_associated_const) = match res {
Res::Def(DefKind::Const, def_id) => (def_id, false),
Res::Def(DefKind::AssocConst, def_id) => (def_id, true),

_ => return pat_from_kind(self.lower_variant_or_leaf(res, id, span, ty, vec![])),
};
let kind = match res {
Res::Def(DefKind::Const | DefKind::AssocConst, def_id) => {
let substs = self.tables.node_substs(id);
// Use `Reveal::All` here because patterns are always monomorphic even if their function isn't.
match self.tcx.const_eval_resolve(
self.param_env.with_reveal_all(),
def_id,
substs,
None,
Some(span),
) {
Ok(value) => {
let const_ =
ty::Const::from_value(self.tcx, value, self.tables.node_type(id));

let pattern = self.const_to_pat(&const_, id, span);
if !is_associated_const {
return pattern;
}

let user_provided_types = self.tables().user_provided_types();
return if let Some(u_ty) = user_provided_types.get(id) {
let user_ty = PatTyProj::from_user_type(*u_ty);
Pat {
span,
kind: Box::new(PatKind::AscribeUserType {
subpattern: pattern,
ascription: Ascription {
/// Note that use `Contravariant` here. See the
/// `variance` field documentation for details.
variance: ty::Variance::Contravariant,
user_ty,
user_ty_span: span,
},
}),
ty: const_.ty,
}
} else {
pattern
};
}
Err(ErrorHandled::TooGeneric) => {
self.errors.push(if is_associated_const {
PatternError::AssocConstInPattern(span)
} else {
PatternError::StaticInPattern(span)
});
PatKind::Wild
}
Err(_) => {
self.tcx.sess.span_err(span, "could not evaluate constant pattern");
PatKind::Wild
}
}
// Use `Reveal::All` here because patterns are always monomorphic even if their function
// isn't.
let param_env_reveal_all = self.param_env.with_reveal_all();
let substs = self.tables.node_substs(id);
let instance = match ty::Instance::resolve(self.tcx, param_env_reveal_all, def_id, substs) {
Ok(Some(i)) => i,
Ok(None) => {
self.errors.push(if is_associated_const {
PatternError::AssocConstInPattern(span)
} else {
PatternError::StaticInPattern(span)
});

return pat_from_kind(PatKind::Wild);
}

Err(_) => {
self.tcx.sess.span_err(span, "could not evaluate constant pattern");
return pat_from_kind(PatKind::Wild);
}
_ => self.lower_variant_or_leaf(res, id, span, ty, vec![]),
};

Pat { span, ty, kind: Box::new(kind) }
// `mir_const_qualif` must be called with the `DefId` of the item where the const is
// defined, not where it is declared. The difference is significant for associated
// constants.
let mir_structural_match_violation = self.tcx.mir_const_qualif(instance.def_id()).custom_eq;
debug!("mir_structural_match_violation({:?}) -> {}", qpath, mir_structural_match_violation);

match self.tcx.const_eval_instance(param_env_reveal_all, instance, Some(span)) {
Ok(value) => {
let const_ = ty::Const::from_value(self.tcx, value, self.tables.node_type(id));

let pattern = self.const_to_pat(&const_, id, span, mir_structural_match_violation);

if !is_associated_const {
return pattern;
}

let user_provided_types = self.tables().user_provided_types();
if let Some(u_ty) = user_provided_types.get(id) {
let user_ty = PatTyProj::from_user_type(*u_ty);
Pat {
span,
kind: Box::new(PatKind::AscribeUserType {
subpattern: pattern,
ascription: Ascription {
/// Note that use `Contravariant` here. See the
/// `variance` field documentation for details.
variance: ty::Variance::Contravariant,
user_ty,
user_ty_span: span,
},
}),
ty: const_.ty,
}
} else {
pattern
}
}
Err(_) => {
self.tcx.sess.span_err(span, "could not evaluate constant pattern");
pat_from_kind(PatKind::Wild)
}
}
}

/// Converts literals, paths and negation of literals to patterns.
Expand All @@ -849,7 +860,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {

let lit_input = LitToConstInput { lit: &lit.node, ty: self.tables.expr_ty(expr), neg };
match self.tcx.at(expr.span).lit_to_const(lit_input) {
Ok(val) => *self.const_to_pat(val, expr.hir_id, lit.span).kind,
Ok(val) => *self.const_to_pat(val, expr.hir_id, lit.span, false).kind,
Err(LitToConstError::UnparseableFloat) => {
self.errors.push(PatternError::FloatBug);
PatKind::Wild
Expand Down
Loading