From 6fd51e74520b87f60be29ac472489a5fbb989e58 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Sun, 27 Mar 2022 22:07:46 -0700 Subject: [PATCH] Check impossible predicates, use it to skip NoopMethodCall and Inline --- compiler/rustc_lint/src/noop_method_call.rs | 8 +++- compiler/rustc_middle/src/mir/mono.rs | 2 +- compiler/rustc_middle/src/query/mod.rs | 34 +++++++++++++-- .../rustc_mir_transform/src/const_prop.rs | 37 +--------------- .../src/const_prop_lint.rs | 11 +---- compiler/rustc_mir_transform/src/inline.rs | 15 +++---- .../rustc_trait_selection/src/traits/mod.rs | 42 +++++++++++-------- src/test/ui/trait-bounds/issue-93008.rs | 17 +++++--- src/test/ui/trait-bounds/issue-93008.stderr | 12 ------ src/test/ui/trait-bounds/issue-94680.rs | 14 +++++++ src/test/ui/trait-bounds/issue-94999.rs | 31 ++++++++++++++ 11 files changed, 126 insertions(+), 97 deletions(-) delete mode 100644 src/test/ui/trait-bounds/issue-93008.stderr create mode 100644 src/test/ui/trait-bounds/issue-94680.rs create mode 100644 src/test/ui/trait-bounds/issue-94999.rs diff --git a/compiler/rustc_lint/src/noop_method_call.rs b/compiler/rustc_lint/src/noop_method_call.rs index 675bee738a672..d3a96f4cff00c 100644 --- a/compiler/rustc_lint/src/noop_method_call.rs +++ b/compiler/rustc_lint/src/noop_method_call.rs @@ -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 }; diff --git a/compiler/rustc_middle/src/mir/mono.rs b/compiler/rustc_middle/src/mir/mono.rs index f76217d921d94..8141dc7e58d05 100644 --- a/compiler/rustc_middle/src/mir/mono.rs +++ b/compiler/rustc_middle/src/mir/mono.rs @@ -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 { diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index 78a3383243306..b3bb2676f1812 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -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. `` + // 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. + query item_has_impossible_predicates(key: DefId) -> bool { + desc { |tcx| + "`{}` has impossible predicates", + tcx.def_path_str(key) } } diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index 691f4fb0e5425..a8ec1b61d5c10 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -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::{ @@ -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. `` - // 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(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; } diff --git a/compiler/rustc_mir_transform/src/const_prop_lint.rs b/compiler/rustc_mir_transform/src/const_prop_lint.rs index 4945c10c9aaa9..7de9b7f7b08bb 100644 --- a/compiler/rustc_mir_transform/src/const_prop_lint.rs +++ b/compiler/rustc_mir_transform/src/const_prop_lint.rs @@ -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; @@ -111,15 +110,7 @@ impl<'tcx> MirLint<'tcx> for ConstProp { // 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(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; } diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs index 5e6dabeba6da2..0a617e9e6ecff 100644 --- a/compiler/rustc_mir_transform/src/inline.rs +++ b/compiler/rustc_mir_transform/src/inline.rs @@ -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}; @@ -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, diff --git a/compiler/rustc_trait_selection/src/traits/mod.rs b/compiler/rustc_trait_selection/src/traits/mod.rs index 8240f5c542a61..7764cbdf4eb44 100644 --- a/compiler/rustc_trait_selection/src/traits/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/mod.rs @@ -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>, + predicates: impl Iterator>, ) -> 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::>(); - 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 { @@ -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 } @@ -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; } @@ -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) { diff --git a/src/test/ui/trait-bounds/issue-93008.rs b/src/test/ui/trait-bounds/issue-93008.rs index 1b010566cbc6e..f4d21a160b695 100644 --- a/src/test/ui/trait-bounds/issue-93008.rs +++ b/src/test/ui/trait-bounds/issue-93008.rs @@ -1,10 +1,15 @@ -// compile-flags: -Zmir-opt-level=4 +// build-pass +// compile-flags: -Zmir-opt-level=3 --crate-type=lib -pub fn bar(s: &'static mut ()) +#![feature(trivial_bounds)] +#![allow(trivial_bounds)] + +trait Foo { + fn test(self); +} +fn baz() where - &'static mut (): Clone, //~ ERROR the trait bound + &'static str: Foo, { - <&'static mut () as Clone>::clone(&s); + "Foo".test() } - -fn main() {} diff --git a/src/test/ui/trait-bounds/issue-93008.stderr b/src/test/ui/trait-bounds/issue-93008.stderr deleted file mode 100644 index 10f80f8de0c9b..0000000000000 --- a/src/test/ui/trait-bounds/issue-93008.stderr +++ /dev/null @@ -1,12 +0,0 @@ -error[E0277]: the trait bound `&'static mut (): Clone` is not satisfied - --> $DIR/issue-93008.rs:5:5 - | -LL | &'static mut (): Clone, - | ^^^^^^^^^^^^^^^^^^^^^^ the trait `Clone` is not implemented for `&'static mut ()` - | - = help: see issue #48214 - = help: add `#![feature(trivial_bounds)]` to the crate attributes to enable - -error: aborting due to previous error - -For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/trait-bounds/issue-94680.rs b/src/test/ui/trait-bounds/issue-94680.rs new file mode 100644 index 0000000000000..58e892079e65f --- /dev/null +++ b/src/test/ui/trait-bounds/issue-94680.rs @@ -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) + } + }); +} diff --git a/src/test/ui/trait-bounds/issue-94999.rs b/src/test/ui/trait-bounds/issue-94999.rs new file mode 100644 index 0000000000000..777c706dfc1d3 --- /dev/null +++ b/src/test/ui/trait-bounds/issue-94999.rs @@ -0,0 +1,31 @@ +// check-pass + +trait Identity { + type T; +} + +impl Identity for T { + type T = T; +} + +trait Holds { + type Q; +} + + +struct S; +struct X(S); + +struct XHelper; + +impl Holds for X { + type Q = XHelper; +} + +impl Clone for X where >::T: Clone, X: Holds { + fn clone(&self) -> Self { + Self(self.0.clone()) + } +} + +fn main() {}