From 171fe82efc2ab420e3fe2e9ad5a44db093a751f1 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Thu, 16 Jan 2020 18:53:51 -0500 Subject: [PATCH] Filter and test predicates using `normalize_and_test_predicates` for const-prop Fixes #68264 Previously, I attempted to use `substitute_normalize_and_test_predicates` to detect unsatisfiable bounds. Unfortunately, since const-prop runs in a generic environment (we don't have any of the function's generic parameters substituted), this could lead to cycle errors when attempting to normalize predicates. This check is replaced with a more precise check. We now only call `normalize_and_test_predicates` on predicates that have the possibility of being proved unsatisfiable - that is, predicates that don't depend on anything local to the function (e.g. generic parameters). This ensures that we don't hit cycle errors when we normalize said predicates, while still ensuring that we detect unsatisfiable predicates. --- src/librustc_mir/transform/const_prop.rs | 44 ++++++++++++------------ 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 9d5dbe3564ee0..badca10569404 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -14,7 +14,7 @@ use rustc::mir::{ SourceInfo, SourceScope, SourceScopeData, Statement, StatementKind, Terminator, TerminatorKind, UnOp, RETURN_PLACE, }; -use rustc::traits::TraitQueryMode; +use rustc::traits; use rustc::ty::layout::{ HasDataLayout, HasTyCtxt, LayoutError, LayoutOf, Size, TargetDataLayout, TyLayout, }; @@ -90,28 +90,28 @@ impl<'tcx> MirPass<'tcx> for ConstProp { // 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. + // We manually filter the predicates, skipping anything that's not + // "global". We are in a potentially generic context + // (e.g. we are evaluating a function without substituging generic + // parameters, so this filtering serves two purposes: // - // 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() - ); + // 1. We skip evaluating any predicates that we would + // never be able prove are unsatisfiable (e.g. `` + // 2. We avoid trying to normalize predicates involving generic + // parameters (e.g. `::MyItem`). This can confuse + // the normalization code (leading to cycle errors), since + // it's usually never invoked in this way. + let predicates = tcx + .predicates_of(source.def_id()) + .predicates + .iter() + .filter_map(|(p, _)| if p.is_global() { Some(*p) } else { None }) + .collect(); + if !traits::normalize_and_test_predicates( + tcx, + traits::elaborate_predicates(tcx, predicates).collect(), + ) { + trace!("ConstProp skipped for {:?}: found unsatisfiable predicates", source.def_id()); return; }