Use TraitQueryMode::Canonical when testing predicates in const prop

Author: Aaron1011

src/librustc/mir/mono.rs

@@ -1,6 +1,7 @@
 use crate::dep_graph::{DepConstructor, DepNode, WorkProduct, WorkProductId};
 use crate::ich::{Fingerprint, NodeIdHashingMode, StableHashingContext};
 use crate::session::config::OptLevel;
+use crate::traits::TraitQueryMode;
 use crate::ty::print::obsolete::DefPathBasedNames;
 use crate::ty::{subst::InternalSubsts, Instance, InstanceDef, SymbolName, TyCtxt};
 use rustc_data_structures::base_n;
@@ -167,7 +168,9 @@ impl<'tcx> MonoItem<'tcx> {
             MonoItem::GlobalAsm(..) => return true,
         };
 
-        tcx.substitute_normalize_and_test_predicates((def_id, &substs))
+        // We shouldn't encounter any overflow here, so we use TraitQueryMode::Standard\
+        // to report an error if overflow somehow occurs.
+        tcx.substitute_normalize_and_test_predicates((def_id, &substs, TraitQueryMode::Standard))
     }
 
     pub fn to_string(&self, tcx: TyCtxt<'tcx>, debug: bool) -> String {

src/librustc/query/mod.rs

@@ -1141,11 +1141,11 @@ rustc_queries! {
             desc { "normalizing `{:?}`", goal }
         }
 
-        query substitute_normalize_and_test_predicates(key: (DefId, SubstsRef<'tcx>)) -> bool {
+        query substitute_normalize_and_test_predicates(key: (DefId, SubstsRef<'tcx>, traits::TraitQueryMode)) -> bool {
             no_force
             desc { |tcx|
-                "testing substituted normalized predicates:`{}`",
-                tcx.def_path_str(key.0)
+                "testing substituted normalized predicates in mode {:?}:`{}`",
+                key.2, tcx.def_path_str(key.0)
             }
         }
 

src/librustc/traits/fulfill.rs

@@ -16,6 +16,7 @@ use super::CodeSelectionError;
 use super::{ConstEvalFailure, Unimplemented};
 use super::{FulfillmentError, FulfillmentErrorCode};
 use super::{ObligationCause, PredicateObligation};
+use crate::traits::TraitQueryMode;
 
 impl<'tcx> ForestObligation for PendingPredicateObligation<'tcx> {
     type Predicate = ty::Predicate<'tcx>;
@@ -62,6 +63,9 @@ pub struct FulfillmentContext<'tcx> {
     // a snapshot (they don't *straddle* a snapshot, so there
     // is no trouble there).
     usable_in_snapshot: bool,
+
+    // The `TraitQueryMode` used when constructing a `SelectionContext`
+    query_mode: TraitQueryMode,
 }
 
 #[derive(Clone, Debug)]
@@ -75,12 +79,26 @@ pub struct PendingPredicateObligation<'tcx> {
 static_assert_size!(PendingPredicateObligation<'_>, 136);
 
 impl<'a, 'tcx> FulfillmentContext<'tcx> {
-    /// Creates a new fulfillment context.
+    /// Creates a new fulfillment context with `TraitQueryMode::Standard`
+    /// You almost always want to use this instead of `with_query_mode`
     pub fn new() -> FulfillmentContext<'tcx> {
         FulfillmentContext {
             predicates: ObligationForest::new(),
             register_region_obligations: true,
             usable_in_snapshot: false,
+            query_mode: TraitQueryMode::Standard,
+        }
+    }
+
+    /// Creates a new fulfillment context with the specified query mode.
+    /// This should only be used when you want to ignore overflow,
+    /// rather than reporting it as an error.
+    pub fn with_query_mode(query_mode: TraitQueryMode) -> FulfillmentContext<'tcx> {
+        FulfillmentContext {
+            predicates: ObligationForest::new(),
+            register_region_obligations: true,
+            usable_in_snapshot: false,
+            query_mode,
         }
     }
 
@@ -89,6 +107,7 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> {
             predicates: ObligationForest::new(),
             register_region_obligations: true,
             usable_in_snapshot: true,
+            query_mode: TraitQueryMode::Standard,
         }
     }
 
@@ -97,6 +116,7 @@ impl<'a, 'tcx> FulfillmentContext<'tcx> {
             predicates: ObligationForest::new(),
             register_region_obligations: false,
             usable_in_snapshot: false,
+            query_mode: TraitQueryMode::Standard,
         }
     }
 
@@ -217,7 +237,7 @@ impl<'tcx> TraitEngine<'tcx> for FulfillmentContext<'tcx> {
         &mut self,
         infcx: &InferCtxt<'_, 'tcx>,
     ) -> Result<(), Vec<FulfillmentError<'tcx>>> {
-        let mut selcx = SelectionContext::new(infcx);
+        let mut selcx = SelectionContext::with_query_mode(infcx, self.query_mode);
         self.select(&mut selcx)
     }
 

src/librustc/traits/mod.rs

@@ -95,7 +95,7 @@ pub enum IntercrateMode {
 }
 
 /// The mode that trait queries run in.
-#[derive(Copy, Clone, PartialEq, Eq, Debug)]
+#[derive(Copy, Clone, PartialEq, Eq, Debug, Hash, HashStable)]
 pub enum TraitQueryMode {
     // Standard/un-canonicalized queries get accurate
     // spans etc. passed in and hence can do reasonable
@@ -1017,13 +1017,14 @@ where
 fn normalize_and_test_predicates<'tcx>(
     tcx: TyCtxt<'tcx>,
     predicates: Vec<ty::Predicate<'tcx>>,
+    mode: TraitQueryMode,
 ) -> bool {
-    debug!("normalize_and_test_predicates(predicates={:?})", predicates);
+    debug!("normalize_and_test_predicates(predicates={:?}, mode={:?})", predicates, mode);
 
     let result = tcx.infer_ctxt().enter(|infcx| {
         let param_env = ty::ParamEnv::reveal_all();
-        let mut selcx = SelectionContext::new(&infcx);
-        let mut fulfill_cx = FulfillmentContext::new();
+        let mut selcx = SelectionContext::with_query_mode(&infcx, mode);
+        let mut fulfill_cx = FulfillmentContext::with_query_mode(mode);
         let cause = ObligationCause::dummy();
         let Normalized { value: predicates, obligations } =
             normalize(&mut selcx, param_env, cause.clone(), &predicates);
@@ -1043,12 +1044,12 @@ fn normalize_and_test_predicates<'tcx>(
 
 fn substitute_normalize_and_test_predicates<'tcx>(
     tcx: TyCtxt<'tcx>,
-    key: (DefId, SubstsRef<'tcx>),
+    key: (DefId, SubstsRef<'tcx>, TraitQueryMode),
 ) -> bool {
     debug!("substitute_normalize_and_test_predicates(key={:?})", key);
 
     let predicates = tcx.predicates_of(key.0).instantiate(tcx, key.1).predicates;
-    let result = normalize_and_test_predicates(tcx, predicates);
+    let result = normalize_and_test_predicates(tcx, predicates, key.2);
 
     debug!("substitute_normalize_and_test_predicates(key={:?}) = {:?}", key, result);
     result
@@ -1101,7 +1102,10 @@ fn vtable_methods<'tcx>(
             // Note that this method could then never be called, so we
             // do not want to try and codegen it, in that case (see #23435).
             let predicates = tcx.predicates_of(def_id).instantiate_own(tcx, substs);
-            if !normalize_and_test_predicates(tcx, predicates.predicates) {
+            // We don't expect overflow here, so report an error if it somehow ends
+            // up happening.
+            if !normalize_and_test_predicates(tcx, predicates.predicates, TraitQueryMode::Standard)
+            {
                 debug!("vtable_methods: predicates do not hold");
                 return None;
             }

src/librustc/ty/query/keys.rs

@@ -115,6 +115,15 @@ impl<'tcx> Key for (DefId, SubstsRef<'tcx>) {
     }
 }
 
+impl<'tcx> Key for (DefId, SubstsRef<'tcx>, traits::TraitQueryMode) {
+    fn query_crate(&self) -> CrateNum {
+        self.0.krate
+    }
+    fn default_span(&self, tcx: TyCtxt<'_>) -> Span {
+        self.0.default_span(tcx)
+    }
+}
+
 impl<'tcx> Key for (ty::ParamEnv<'tcx>, ty::PolyTraitRef<'tcx>) {
     fn query_crate(&self) -> CrateNum {
         self.1.def_id().krate

src/librustc_mir/transform/const_prop.rs

@@ -14,6 +14,7 @@ use rustc::mir::{
     SourceInfo, SourceScope, SourceScopeData, Statement, StatementKind, Terminator, TerminatorKind,
     UnOp, RETURN_PLACE,
 };
+use rustc::traits::TraitQueryMode;
 use rustc::ty::layout::{
     HasDataLayout, HasTyCtxt, LayoutError, LayoutOf, Size, TargetDataLayout, TyLayout,
 };
@@ -88,9 +89,24 @@ impl<'tcx> MirPass<'tcx> for ConstProp {
         // sure that it even makes sense to try to evaluate the body.
         // If there are unsatisfiable where clauses, then all bets are
         // off, and we just give up.
+        //
+        // Note that we use TraitQueryMode::Canonical here, which causes
+        // us to treat overflow like any other error. This is because we
+        // are "speculatively" evaluating this item with the default substs.
+        // While this usually succeeds, it may fail with tricky impls
+        // (e.g. the typenum crate). Const-propagation is fundamentally
+        // "best-effort", and does not affect correctness in any way.
+        // Therefore, it's perfectly fine to just "give up" if we're
+        // unable to check the bounds with the default substs.
+        //
+        // False negatives (failing to run const-prop on something when we actually
+        // could) are fine. However, false positives (running const-prop on
+        // an item with unsatisfiable bounds) can lead to us generating invalid
+        // MIR.
         if !tcx.substitute_normalize_and_test_predicates((
             source.def_id(),
             InternalSubsts::identity_for_item(tcx, source.def_id()),
+            TraitQueryMode::Canonical,
         )) {
             trace!(
                 "ConstProp skipped for item with unsatisfiable predicates: {:?}",