HIR-based (rather than type-based) structural-match checking of consts occurring in patterns.

Incorporated review feedback: moved the HIR traversal to
`rustc_mir::hair::structural_match`.

Author: pnkfelix

src/librustc/ty/mod.rs

@@ -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;

src/librustc/ty/structural_match.rs

@@ -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};
 
@@ -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
@@ -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
@@ -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:
@@ -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,
 
@@ -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
     }
@@ -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),
@@ -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!
         }

src/librustc_mir/hair/pattern/const_to_pat.rs

@@ -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};
@@ -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)
         })
     }
 }
@@ -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
     }
 

src/librustc_mir/hair/pattern/mod.rs

@@ -3,6 +3,7 @@
 mod _match;
 mod check_match;
 mod const_to_pat;
+mod structural_match;
 
 pub(crate) use self::check_match::check_match;
 
@@ -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;
                                 }
@@ -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);
@@ -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);