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 1d5a643484a73..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, }; @@ -74,6 +75,46 @@ impl<'tcx> MirPass<'tcx> for ConstProp { return; } + // Check if it's even possible to satisfy the 'where' clauses + // for this item. + // This branch will never be taken for any normal function. + // However, it's possible to `#!feature(trivial_bounds)]` to write + // a function with impossible to satisfy clauses, e.g.: + // `fn foo() where String: Copy {}` + // + // We don't usually need to worry about this kind of case, + // since we would get a compilation error if the user tried + // to call it. However, since we can do const propagation + // even without any calls to the function, we need to make + // 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: {:?}", + source.def_id() + ); + return; + } + trace!("ConstProp starting for {:?}", source.def_id()); let dummy_body = &Body::new( diff --git a/src/test/ui/consts/issue-67696-const-prop-ice.rs b/src/test/ui/consts/issue-67696-const-prop-ice.rs new file mode 100644 index 0000000000000..ad52608b3f46d --- /dev/null +++ b/src/test/ui/consts/issue-67696-const-prop-ice.rs @@ -0,0 +1,20 @@ +// check-pass +// compile-flags: --emit=mir,link +// Checks that we don't ICE due to attempting to run const prop +// on a function with unsatisifable 'where' clauses + +#![allow(unused)] + +trait A { + fn foo(&self) -> Self where Self: Copy; +} + +impl A for [fn(&())] { + fn foo(&self) -> Self where Self: Copy { *(&[] as &[_]) } +} + +impl A for i32 { + fn foo(&self) -> Self { 3 } +} + +fn main() {} diff --git a/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.rs b/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.rs index 6450ddd1b67fc..69eee66e64d89 100644 --- a/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.rs +++ b/src/test/ui/trivial-bounds/trivial-bounds-inconsistent-associated-functions.rs @@ -1,4 +1,8 @@ -// run-pass +// check-pass +// compile-flags: --emit=mir,link +// Force mir to be emitted, to ensure that const +// propagation doesn't ICE on a function +// with an 'impossible' body. See issue #67696 // Inconsistent bounds with trait implementations #![feature(trivial_bounds)]