From e390b91e5717a338db20360a1ddffa23ba66701d Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 13 Jan 2020 06:06:42 -0500 Subject: [PATCH] Use TraitQueryMode::Canonical when testing predicates in const prop --- src/librustc/mir/mono.rs | 5 ++++- src/librustc/query/mod.rs | 6 +++--- src/librustc/traits/fulfill.rs | 24 ++++++++++++++++++++++-- src/librustc/traits/mod.rs | 18 +++++++++++------- src/librustc/ty/query/keys.rs | 9 +++++++++ src/librustc_mir/transform/const_prop.rs | 16 ++++++++++++++++ 6 files changed, 65 insertions(+), 13 deletions(-) diff --git a/src/librustc/mir/mono.rs b/src/librustc/mir/mono.rs index 51ce575e51f3b..e7a4c5b592105 100644 --- a/src/librustc/mir/mono.rs +++ b/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 { diff --git a/src/librustc/query/mod.rs b/src/librustc/query/mod.rs index 50ef115da2c42..669ba4abfadd8 100644 --- a/src/librustc/query/mod.rs +++ b/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) } } diff --git a/src/librustc/traits/fulfill.rs b/src/librustc/traits/fulfill.rs index 46ece6fc40593..9e5abc80822c7 100644 --- a/src/librustc/traits/fulfill.rs +++ b/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>> { - let mut selcx = SelectionContext::new(infcx); + let mut selcx = SelectionContext::with_query_mode(infcx, self.query_mode); self.select(&mut selcx) } diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index 2d3160dc3e51a..31de5409fc8be 100644 --- a/src/librustc/traits/mod.rs +++ b/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>, + 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; } diff --git a/src/librustc/ty/query/keys.rs b/src/librustc/ty/query/keys.rs index d64f27d9cc26c..20f8e7f564abd 100644 --- a/src/librustc/ty/query/keys.rs +++ b/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 diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 35dbf2f7eaa42..90c97480c7562 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/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: {:?}",