Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add query to check for impossible predicates, use it to skip NoopMethodCall and Inline #95398

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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() {}
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/missing_const_for_fn.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use clippy_utils::diagnostics::span_lint;
use clippy_utils::qualify_min_const_fn::is_min_const_fn;
use clippy_utils::ty::has_drop;
use clippy_utils::{fn_has_unsatisfiable_preds, is_entrypoint_fn, meets_msrv, msrvs, trait_ref_of_method};
use clippy_utils::{is_entrypoint_fn, meets_msrv, msrvs, trait_ref_of_method};
use rustc_hir as hir;
use rustc_hir::def_id::CRATE_DEF_ID;
use rustc_hir::intravisit::FnKind;
Expand Down Expand Up @@ -104,7 +104,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingConstForFn {
}

// Building MIR for `fn`s with unsatisfiable preds results in ICE.
if fn_has_unsatisfiable_preds(cx, def_id.to_def_id()) {
if cx.tcx.item_has_impossible_predicates(def_id.to_def_id()) {
return;
}

Expand Down
4 changes: 2 additions & 2 deletions src/tools/clippy/clippy_lints/src/redundant_clone.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then};
use clippy_utils::source::snippet_opt;
use clippy_utils::ty::{has_drop, is_copy, is_type_diagnostic_item, walk_ptrs_ty_depth};
use clippy_utils::{fn_has_unsatisfiable_preds, match_def_path, paths};
use clippy_utils::{match_def_path, paths};
use if_chain::if_chain;
use rustc_data_structures::fx::FxHashMap;
use rustc_errors::Applicability;
Expand Down Expand Up @@ -84,7 +84,7 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClone {
let def_id = cx.tcx.hir().body_owner_def_id(body.id());

// Building MIR for `fn`s with unsatisfiable preds results in ICE.
if fn_has_unsatisfiable_preds(cx, def_id.to_def_id()) {
if cx.tcx.item_has_impossible_predicates(def_id.to_def_id()) {
return;
}

Expand Down
Loading