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

Expr walking structural match (reopened) #67088

Closed
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
2 changes: 1 addition & 1 deletion src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ declare_lint! {
declare_lint! {
pub INDIRECT_STRUCTURAL_MATCH,
// defaulting to allow until rust-lang/rust#62614 is fixed.
Allow,
Warn,
"pattern with const indirectly referencing non-`#[structural_match]` type",
@future_incompatible = FutureIncompatibleInfo {
reference: "issue #62411 <https://github.com/rust-lang/rust/issues/62411>",
Expand Down
4 changes: 3 additions & 1 deletion src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,10 @@ pub use self::context::{

pub use self::instance::{Instance, InstanceDef};

pub use self::structural_match::search_for_structural_match_violation;
pub use self::structural_match::search_type_for_structural_match_violation;
pub use self::structural_match::type_marked_structural;
pub use self::structural_match::register_structural_match_bounds;
pub use self::structural_match::report_structural_match_violation;
pub use self::structural_match::NonStructuralMatchTy;

pub use self::trait_def::TraitDef;
Expand Down
75 changes: 55 additions & 20 deletions src/librustc/ty/structural_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::hir;
use rustc::infer::InferCtxt;
use rustc::traits::{self, ConstPatternStructural, TraitEngine};
use rustc::traits::ObligationCause;
use rustc::lint;

use rustc_data_structures::fx::{FxHashSet};

Expand All @@ -16,6 +17,30 @@ pub enum NonStructuralMatchTy<'tcx> {
Param,
}

pub fn report_structural_match_violation(tcx: TyCtxt<'tcx>,
non_sm_ty: NonStructuralMatchTy<'tcx>,
id: hir::HirId,
span: Span,
warn_instead_of_hard_error: bool) {
let adt_def = match non_sm_ty {
ty::NonStructuralMatchTy::Adt(adt_def) => adt_def,
ty::NonStructuralMatchTy::Param =>
bug!("use of constant whose type is a parameter inside a pattern"),
};
let path = tcx.def_path_str(adt_def.did);
let msg = format!("to use a constant of type `{}` in a pattern, \
`{}` must be annotated with `#[derive(PartialEq, Eq)]`",
path, path);


if warn_instead_of_hard_error {
tcx.lint_hir(lint::builtin::INDIRECT_STRUCTURAL_MATCH, id, span, &msg);
} else {
// span_fatal avoids ICE from resolution of non-existent method (rare case).
tcx.sess.span_fatal(span, &msg);
}
}

/// This method traverses the structure of `ty`, trying to find an
/// instance of an ADT (i.e. struct or enum) that was declared without
/// the `#[structural_match]` attribute, or a generic type parameter
Expand All @@ -41,20 +66,39 @@ pub enum NonStructuralMatchTy<'tcx> {
/// For more background on why Rust has this requirement, and issues
/// that arose when the requirement was not enforced completely, see
/// Rust RFC 1445, rust-lang/rust#61188, and rust-lang/rust#62307.
pub fn search_for_structural_match_violation<'tcx>(
pub fn search_type_for_structural_match_violation<'tcx>(
id: hir::HirId,
span: Span,
tcx: TyCtxt<'tcx>,
ty: Ty<'tcx>,
) -> Option<NonStructuralMatchTy<'tcx>> {
// FIXME: we should instead pass in an `infcx` from the outside.
// FIXME: consider passing in an `infcx` from the outside.
tcx.infer_ctxt().enter(|infcx| {
let mut search = Search { id, span, infcx, found: None, seen: FxHashSet::default() };
let mut search = SearchTy { id, span, infcx, found: None, seen: FxHashSet::default() };
ty.visit_with(&mut search);
search.found
})
}

pub fn register_structural_match_bounds(fulfillment_cx: &mut traits::FulfillmentContext<'tcx>,
id: hir::HirId,
span: Span,
infcx: &InferCtxt<'_, 'tcx>,
adt_ty: Ty<'tcx>)
{
let cause = ObligationCause::new(span, id, ConstPatternStructural);
// require `#[derive(PartialEq)]`
let structural_peq_def_id = infcx.tcx.lang_items().structural_peq_trait().unwrap();
fulfillment_cx.register_bound(
infcx, ty::ParamEnv::empty(), adt_ty, structural_peq_def_id, cause);
// for now, require `#[derive(Eq)]`. (Doing so is a hack to work around
// the type `for<'a> fn(&'a ())` failing to implement `Eq` itself.)
let cause = ObligationCause::new(span, id, ConstPatternStructural);
let structural_teq_def_id = infcx.tcx.lang_items().structural_teq_trait().unwrap();
fulfillment_cx.register_bound(
infcx, ty::ParamEnv::empty(), adt_ty, structural_teq_def_id, cause);
}

/// This method returns true if and only if `adt_ty` itself has been marked as
/// eligible for structural-match: namely, if it implements both
/// `StructuralPartialEq` and `StructuralEq` (which are respectively injected by
Expand All @@ -69,17 +113,8 @@ pub fn type_marked_structural(id: hir::HirId,
-> bool
{
let mut fulfillment_cx = traits::FulfillmentContext::new();
let cause = ObligationCause::new(span, id, ConstPatternStructural);
// require `#[derive(PartialEq)]`
let structural_peq_def_id = infcx.tcx.lang_items().structural_peq_trait().unwrap();
fulfillment_cx.register_bound(
infcx, ty::ParamEnv::empty(), adt_ty, structural_peq_def_id, cause);
// for now, require `#[derive(Eq)]`. (Doing so is a hack to work around
// the type `for<'a> fn(&'a ())` failing to implement `Eq` itself.)
let cause = ObligationCause::new(span, id, ConstPatternStructural);
let structural_teq_def_id = infcx.tcx.lang_items().structural_teq_trait().unwrap();
fulfillment_cx.register_bound(
infcx, ty::ParamEnv::empty(), adt_ty, structural_teq_def_id, cause);

register_structural_match_bounds(&mut fulfillment_cx, id, span, infcx, adt_ty);

// We deliberately skip *reporting* fulfillment errors (via
// `report_fulfillment_errors`), for two reasons:
Expand All @@ -96,7 +131,7 @@ pub fn type_marked_structural(id: hir::HirId,
/// This implements the traversal over the structure of a given type to try to
/// find instances of ADTs (specifically structs or enums) that do not implement
/// the structural-match traits (`StructuralPartialEq` and `StructuralEq`).
struct Search<'a, 'tcx> {
struct SearchTy<'a, 'tcx> {
id: hir::HirId,
span: Span,

Expand All @@ -110,7 +145,7 @@ struct Search<'a, 'tcx> {
seen: FxHashSet<hir::def_id::DefId>,
}

impl Search<'a, 'tcx> {
impl SearchTy<'a, 'tcx> {
fn tcx(&self) -> TyCtxt<'tcx> {
self.infcx.tcx
}
Expand All @@ -120,9 +155,9 @@ impl Search<'a, 'tcx> {
}
}

impl<'a, 'tcx> TypeVisitor<'tcx> for Search<'a, 'tcx> {
impl<'a, 'tcx> TypeVisitor<'tcx> for SearchTy<'a, 'tcx> {
fn visit_ty(&mut self, ty: Ty<'tcx>) -> bool {
debug!("Search visiting ty: {:?}", ty);
debug!("SearchTy visiting ty: {:?}", ty);

let (adt_def, substs) = match ty.kind {
ty::Adt(adt_def, substs) => (adt_def, substs),
Expand Down Expand Up @@ -170,13 +205,13 @@ impl<'a, 'tcx> TypeVisitor<'tcx> for Search<'a, 'tcx> {
};

if !self.seen.insert(adt_def.did) {
debug!("Search already seen adt_def: {:?}", adt_def);
debug!("SearchTy already seen adt_def: {:?}", adt_def);
// let caller continue its search
return false;
}

if !self.type_marked_structural(ty) {
debug!("Search found ty: {:?}", ty);
debug!("SearchTy found ty: {:?}", ty);
self.found = Some(NonStructuralMatchTy::Adt(&adt_def));
return true; // Halt visiting!
}
Expand Down
133 changes: 65 additions & 68 deletions src/librustc_mir/hair/pattern/const_to_pat.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::const_eval::const_variant_index;

use rustc::hir;
use rustc::lint;
use rustc::hir::def_id::DefId;
use rustc::mir::Field;
use rustc::infer::InferCtxt;
use rustc::traits::{ObligationCause, PredicateObligation};
Expand All @@ -15,23 +15,28 @@ use syntax_pos::Span;
use std::cell::Cell;

use super::{FieldPat, Pat, PatCtxt, PatKind};
use super::structural_match::search_const_rhs_for_structural_match_violation;

impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
/// Converts an evaluated constant to a pattern (if possible).
/// This means aggregate values (like structs and enums) are converted
/// to a pattern that matches the value (as if you'd compared via structural equality).
///
/// For literals, pass `None` as the `opt_const_def_id`; for a const
/// identifier, pass its `DefId`.
pub(super) fn const_to_pat(
&self,
cv: &'tcx ty::Const<'tcx>,
opt_const_def_id: Option<DefId>,
id: hir::HirId,
span: Span,
) -> Pat<'tcx> {
debug!("const_to_pat: cv={:#?} id={:?}", cv, id);
debug!("const_to_pat: cv.ty={:?} span={:?}", cv.ty, span);
debug!("const_def_to_pat: cv={:#?} const_def_id: {:?} id={:?}", cv, opt_const_def_id, id);
debug!("const_def_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, opt_const_def_id)
})
}
}
Expand Down Expand Up @@ -67,85 +72,77 @@ impl<'a, 'tcx> ConstToPat<'a, 'tcx> {

fn tcx(&self) -> TyCtxt<'tcx> { self.infcx.tcx }

fn search_for_structural_match_violation(&self,
ty: Ty<'tcx>)
-> Option<ty::NonStructuralMatchTy<'tcx>>
fn search_const_def_for_structural_match_violation(&self, const_def_id: DefId)
{
ty::search_for_structural_match_violation(self.id, self.span, self.tcx(), ty)
assert!(const_def_id.is_local());
self.tcx().infer_ctxt().enter(|infcx| {
search_const_rhs_for_structural_match_violation(
&infcx, self.param_env, const_def_id, self.id, self.span);
});
}

fn search_ty_for_structural_match_violation(&self, ty: Ty<'tcx>)
{
let structural = ty::search_type_for_structural_match_violation(
self.id, self.span, self.tcx(), ty);
debug!("search_ty_for_structural_match_violation ty: {:?} returned: {:?}", ty, structural);
if let Some(non_sm_ty) = structural {

// double-check there even *is* a semantic `PartialEq` to dispatch to.
//
// (If there isn't, then we can safely issue a hard
// error, because that's never worked, due to compiler
// using `PartialEq::eq` in this scenario in the past.)
//
// Note: To fix rust-lang/rust#65466, one could lift this check
// *before* any structural-match checking, and unconditionally error
// if `PartialEq` is not implemented. However, that breaks stable
// code at the moment, because types like `for <'a> fn(&'a ())` do
// not *yet* implement `PartialEq`. So for now we leave this here.
let warn_instead_of_hard_error: bool = {
let partial_eq_trait_id = self.tcx().lang_items().eq_trait().unwrap();
let obligation: PredicateObligation<'_> =
self.tcx().predicate_for_trait_def(
self.param_env,
ObligationCause::misc(self.span, self.id),
partial_eq_trait_id,
0,
ty,
&[]);
// FIXME: should this call a `predicate_must_hold` variant instead?
self.infcx.predicate_may_hold(&obligation)
};

debug!("call report_structural_match_violation non_sm_ty: {:?} id: {:?} warn: {:?}",
non_sm_ty, self.id, warn_instead_of_hard_error);
ty::report_structural_match_violation(
self.tcx(), non_sm_ty, self.id, self.span, warn_instead_of_hard_error);
}
}

fn type_marked_structural(&self, ty: Ty<'tcx>) -> bool {
ty::type_marked_structural(self.id, self.span, &self.infcx, ty)
}

fn to_pat(&mut self, cv: &'tcx ty::Const<'tcx>) -> 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.
//
// once indirect_structural_match is a full fledged error, this
// level of indirection can be eliminated

fn to_pat(&mut self,
cv: &'tcx ty::Const<'tcx>,
opt_const_def_id: Option<DefId>)
-> Pat<'tcx>
{
let inlined_const_as_pat = self.recur(cv);

if self.include_lint_checks && !self.saw_const_match_error.get() {
// If we were able to successfully convert the const to some pat,
// double-check that all types in the const implement `Structural`.

let structural = self.search_for_structural_match_violation(cv.ty);
debug!("search_for_structural_match_violation cv.ty: {:?} returned: {:?}",
cv.ty, structural);
if let Some(non_sm_ty) = structural {
let adt_def = match non_sm_ty {
ty::NonStructuralMatchTy::Adt(adt_def) => adt_def,
ty::NonStructuralMatchTy::Param =>
bug!("use of constant whose type is a parameter inside a pattern"),
};
let path = self.tcx().def_path_str(adt_def.did);
let msg = format!(
"to use a constant of type `{}` in a pattern, \
`{}` must be annotated with `#[derive(PartialEq, Eq)]`",
path,
path,
);

// double-check there even *is* a semantic `PartialEq` to dispatch to.
//
// (If there isn't, then we can safely issue a hard
// error, because that's never worked, due to compiler
// using `PartialEq::eq` in this scenario in the past.)
//
// Note: To fix rust-lang/rust#65466, one could lift this check
// *before* any structural-match checking, and unconditionally error
// if `PartialEq` is not implemented. However, that breaks stable
// code at the moment, because types like `for <'a> fn(&'a ())` do
// not *yet* implement `PartialEq`. So for now we leave this here.
let ty_is_partial_eq: bool = {
let partial_eq_trait_id = self.tcx().lang_items().eq_trait().unwrap();
let obligation: PredicateObligation<'_> =
self.tcx().predicate_for_trait_def(
self.param_env,
ObligationCause::misc(self.span, self.id),
partial_eq_trait_id,
0,
cv.ty,
&[]);
// FIXME: should this call a `predicate_must_hold` variant instead?
self.infcx.predicate_may_hold(&obligation)
};

if !ty_is_partial_eq {
// span_fatal avoids ICE from resolution of non-existent method (rare case).
self.tcx().sess.span_fatal(self.span, &msg);
} else {
self.tcx().lint_hir(lint::builtin::INDIRECT_STRUCTURAL_MATCH,
self.id,
self.span,
&msg);
match opt_const_def_id {
Some(const_def_id) if const_def_id.is_local() => {
self.search_const_def_for_structural_match_violation(const_def_id);
}
_ => {
self.search_ty_for_structural_match_violation(cv.ty);
}
}
}

inlined_const_as_pat
}

Expand Down
8 changes: 5 additions & 3 deletions src/librustc_mir/hair/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
mod _match;
mod check_match;
mod const_to_pat;
mod structural_match;

pub(crate) use self::check_match::check_match;

Expand Down Expand Up @@ -867,7 +868,8 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
};
match self.tcx.at(span).const_eval(self.param_env.and(cid)) {
Ok(value) => {
let pattern = self.const_to_pat(value, id, span);
let const_def_id = Some(instance.def_id());
let pattern = self.const_to_pat(value, const_def_id, id, span);
if !is_associated_const {
return pattern;
}
Expand Down Expand Up @@ -934,7 +936,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
let ty = self.tables.expr_ty(expr);
match lit_to_const(&lit.node, self.tcx, ty, false) {
Ok(val) => {
*self.const_to_pat(val, expr.hir_id, lit.span).kind
*self.const_to_pat(val, None, expr.hir_id, lit.span).kind
},
Err(LitToConstError::UnparseableFloat) => {
self.errors.push(PatternError::FloatBug);
Expand All @@ -952,7 +954,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
};
match lit_to_const(&lit.node, self.tcx, ty, true) {
Ok(val) => {
*self.const_to_pat(val, expr.hir_id, lit.span).kind
*self.const_to_pat(val, None, expr.hir_id, lit.span).kind
},
Err(LitToConstError::UnparseableFloat) => {
self.errors.push(PatternError::FloatBug);
Expand Down
Loading