Skip to content

Commit

Permalink
Filter and test predicates using normalize_and_test_predicates for …
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
Aaron1011 committed Jan 16, 2020
1 parent d088d8a commit 171fe82
Showing 1 changed file with 22 additions and 22 deletions.
44 changes: 22 additions & 22 deletions src/librustc_mir/transform/const_prop.rs
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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. `<T as Foo>`
// 2. We avoid trying to normalize predicates involving generic
// parameters (e.g. `<T as Foo>::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;
}

Expand Down

0 comments on commit 171fe82

Please sign in to comment.