Skip to content

Commit

Permalink
Check impossible predicates, use it to skip NoopMethodCall and Inline
Browse files Browse the repository at this point in the history
  • Loading branch information
compiler-errors committed Apr 24, 2022
1 parent 5cdab3a commit 6fd51e7
Show file tree
Hide file tree
Showing 11 changed files with 126 additions and 97 deletions.
8 changes: 7 additions & 1 deletion compiler/rustc_lint/src/noop_method_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,16 @@ impl<'tcx> LateLintPass<'tcx> for NoopMethodCall {
// we need to perform substitution.
return;
}

if cx.tcx.instantiated_item_has_impossible_predicates((did, substs)) {
tracing::trace!("NoopMethodCall skipped for {:?}: found unsatisfiable predicates", did);
return;
}

let param_env = cx.tcx.param_env(trait_id);
// Resolve the trait method instance.
let Ok(Some(i)) = ty::Instance::resolve(cx.tcx, param_env, did, substs) else {
return
return;
};
// (Re)check that it implements the noop diagnostic.
let Some(name) = cx.tcx.get_diagnostic_name(i.def_id()) else { return };
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/mono.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ impl<'tcx> MonoItem<'tcx> {
MonoItem::GlobalAsm(..) => return true,
};

!tcx.subst_and_check_impossible_predicates((def_id, &substs))
!tcx.instantiated_item_has_impossible_predicates((def_id, &substs))
}

pub fn local_span(&self, tcx: TyCtxt<'tcx>) -> Option<Span> {
Expand Down
34 changes: 31 additions & 3 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1870,10 +1870,38 @@ rustc_queries! {
remap_env_constness
}

query subst_and_check_impossible_predicates(key: (DefId, SubstsRef<'tcx>)) -> bool {
query instantiated_item_has_impossible_predicates(key: (DefId, SubstsRef<'tcx>)) -> bool {
desc { |tcx|
"impossible substituted predicates:`{}`",
tcx.def_path_str(key.0)
"`{}` has impossible predicates after substituting `{:?}`",
tcx.def_path_str(key.0), key.1
}
}

// Check if it's even possible to satisfy the 'where' clauses
// for this item, without substitutions.
//
// 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 certain mir optimizations
// and lints even without any calls to the function, we need to
// make sure that it even makes sense to try to evaluate predicates
// dependent on the where-clause of this function.
//
// 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 substituting generic
// parameters), so this filtering serves two purposes:
//
// 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.
query item_has_impossible_predicates(key: DefId) -> bool {
desc { |tcx|
"`{}` has impossible predicates",
tcx.def_path_str(key)
}
}

Expand Down
37 changes: 1 addition & 36 deletions compiler/rustc_mir_transform/src/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ use rustc_middle::ty::{self, ConstKind, Instance, ParamEnv, Ty, TyCtxt, TypeFold
use rustc_span::{def_id::DefId, Span};
use rustc_target::abi::{HasDataLayout, Size, TargetDataLayout};
use rustc_target::spec::abi::Abi;
use rustc_trait_selection::traits;

use crate::MirPass;
use rustc_const_eval::interpret::{
Expand Down Expand Up @@ -90,41 +89,7 @@ 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.
//
// 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 substituting generic
// parameters, so this filtering serves two purposes:
//
// 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(def_id.to_def_id())
.predicates
.iter()
.filter_map(|(p, _)| if p.is_global() { Some(*p) } else { None });
if traits::impossible_predicates(
tcx,
traits::elaborate_predicates(tcx, predicates).map(|o| o.predicate).collect(),
) {
if tcx.item_has_impossible_predicates(def_id) {
trace!("ConstProp skipped for {:?}: found unsatisfiable predicates", def_id);
return;
}
Expand Down
11 changes: 1 addition & 10 deletions compiler/rustc_mir_transform/src/const_prop_lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use rustc_session::lint;
use rustc_span::{def_id::DefId, Span};
use rustc_target::abi::{HasDataLayout, Size, TargetDataLayout};
use rustc_target::spec::abi::Abi;
use rustc_trait_selection::traits;

use crate::MirLint;
use rustc_const_eval::const_eval::ConstEvalErr;
Expand Down Expand Up @@ -111,15 +110,7 @@ impl<'tcx> MirLint<'tcx> for ConstProp {
// 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(def_id.to_def_id())
.predicates
.iter()
.filter_map(|(p, _)| if p.is_global() { Some(*p) } else { None });
if traits::impossible_predicates(
tcx,
traits::elaborate_predicates(tcx, predicates).map(|o| o.predicate).collect(),
) {
if tcx.item_has_impossible_predicates(def_id) {
trace!("ConstProp skipped for {:?}: found unsatisfiable predicates", def_id);
return;
}
Expand Down
15 changes: 5 additions & 10 deletions compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use rustc_index::vec::Idx;
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
use rustc_middle::mir::visit::*;
use rustc_middle::mir::*;
use rustc_middle::traits::ObligationCause;
use rustc_middle::ty::subst::Subst;
use rustc_middle::ty::{self, ConstKind, Instance, InstanceDef, ParamEnv, Ty, TyCtxt};
use rustc_span::{hygiene::ExpnKind, ExpnData, LocalExpnId, Span};
Expand Down Expand Up @@ -74,18 +73,14 @@ fn inline<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) -> bool {
return false;
}

let param_env = tcx.param_env_reveal_all_normalized(def_id);
let hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
let param_env = rustc_trait_selection::traits::normalize_param_env_or_error(
tcx,
def_id.to_def_id(),
param_env,
ObligationCause::misc(body.span, hir_id),
);
if tcx.item_has_impossible_predicates(def_id) {
trace!("Inline skipped for {:?}: found unsatisfiable predicates", def_id);
return false;
}

let mut this = Inliner {
tcx,
param_env,
param_env: tcx.param_env_reveal_all_normalized(def_id),
codegen_fn_attrs: tcx.codegen_fn_attrs(def_id),
history: Vec::new(),
changed: false,
Expand Down
42 changes: 24 additions & 18 deletions compiler/rustc_trait_selection/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,17 +425,21 @@ where
/// Normalizes the predicates and checks whether they hold in an empty environment. If this
/// returns true, then either normalize encountered an error or one of the predicates did not
/// hold. Used when creating vtables to check for unsatisfiable methods.
pub fn impossible_predicates<'tcx>(
fn has_impossible_predicates<'tcx>(
tcx: TyCtxt<'tcx>,
predicates: Vec<ty::Predicate<'tcx>>,
predicates: impl Iterator<Item = ty::Predicate<'tcx>>,
) -> bool {
debug!("impossible_predicates(predicates={:?})", predicates);
let predicates = elaborate_predicates(tcx, predicates)
.map(|o| tcx.erase_regions(o.predicate))
.filter(|p| p.is_global())
.collect::<Vec<_>>();

let result = tcx.infer_ctxt().enter(|infcx| {
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 cause = ObligationCause::dummy();

let Normalized { value: predicates, obligations } =
normalize(&mut selcx, param_env, cause.clone(), predicates);
for obligation in obligations {
Expand All @@ -447,24 +451,26 @@ pub fn impossible_predicates<'tcx>(
}

let errors = fulfill_cx.select_all_or_error(&infcx);

!errors.is_empty()
});
debug!("impossible_predicates = {:?}", result);
result
})
}

fn subst_and_check_impossible_predicates<'tcx>(
fn instantiated_item_has_impossible_predicates<'tcx>(
tcx: TyCtxt<'tcx>,
key: (DefId, SubstsRef<'tcx>),
) -> bool {
debug!("subst_and_check_impossible_predicates(key={:?})", key);

let mut predicates = tcx.predicates_of(key.0).instantiate(tcx, key.1).predicates;
predicates.retain(|predicate| !predicate.needs_subst());
let result = impossible_predicates(tcx, predicates);
debug!("instantiated_item_has_impossible_predicates(key={:?})", key);
let predicates = tcx.predicates_of(key.0).instantiate(tcx, key.1).predicates;
let result = has_impossible_predicates(tcx, predicates.into_iter());
debug!("instantiated_item_has_impossible_predicates(key={:?}) = {:?}", key, result);
result
}

debug!("subst_and_check_impossible_predicates(key={:?}) = {:?}", key, result);
fn item_has_impossible_predicates<'tcx>(tcx: TyCtxt<'tcx>, key: DefId) -> bool {
debug!("item_has_impossible_predicates(key={:?})", key);
let predicates = tcx.predicates_of(key).instantiate_identity(tcx).predicates;
let result = has_impossible_predicates(tcx, predicates.into_iter());
debug!("item_has_impossible_predicates(key={:?}) = {:?}", key, result);
result
}

Expand Down Expand Up @@ -715,8 +721,7 @@ fn vtable_entries<'tcx>(
// do not hold for this particular set of type parameters.
// 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 impossible_predicates(tcx, predicates.predicates) {
if tcx.instantiated_item_has_impossible_predicates((def_id, substs)) {
debug!("vtable_entries: predicates do not hold");
return VtblEntry::Vacant;
}
Expand Down Expand Up @@ -847,7 +852,8 @@ pub fn provide(providers: &mut ty::query::Providers) {
own_existential_vtable_entries,
vtable_entries,
vtable_trait_upcasting_coercion_new_vptr_slot,
subst_and_check_impossible_predicates,
instantiated_item_has_impossible_predicates,
item_has_impossible_predicates,
thir_abstract_const: |tcx, def_id| {
let def_id = def_id.expect_local();
if let Some(def) = ty::WithOptConstParam::try_lookup(def_id, tcx) {
Expand Down
17 changes: 11 additions & 6 deletions src/test/ui/trait-bounds/issue-93008.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
// compile-flags: -Zmir-opt-level=4
// build-pass
// compile-flags: -Zmir-opt-level=3 --crate-type=lib

pub fn bar<T>(s: &'static mut ())
#![feature(trivial_bounds)]
#![allow(trivial_bounds)]

trait Foo {
fn test(self);
}
fn baz<T>()
where
&'static mut (): Clone, //~ ERROR the trait bound
&'static str: Foo,
{
<&'static mut () as Clone>::clone(&s);
"Foo".test()
}

fn main() {}
12 changes: 0 additions & 12 deletions src/test/ui/trait-bounds/issue-93008.stderr

This file was deleted.

14 changes: 14 additions & 0 deletions src/test/ui/trait-bounds/issue-94680.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// check-pass

fn main() {
println!("{:?}", {
type T = ();

pub fn cloneit(it: &'_ mut T) -> (&'_ mut T, &'_ mut T)
where
for<'any> &'any mut T: Clone,
{
(it.clone(), it)
}
});
}
31 changes: 31 additions & 0 deletions src/test/ui/trait-bounds/issue-94999.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// check-pass

trait Identity<Q> {
type T;
}

impl<Q, T> Identity<Q> for T {
type T = T;
}

trait Holds {
type Q;
}


struct S;
struct X(S);

struct XHelper;

impl Holds for X {
type Q = XHelper;
}

impl<Q> Clone for X where <S as Identity<Q>>::T: Clone, X: Holds<Q = Q> {
fn clone(&self) -> Self {
Self(self.0.clone())
}
}

fn main() {}

0 comments on commit 6fd51e7

Please sign in to comment.