From 55ce976e06e1dc9590a41cdc262688d4db8d93e4 Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Thu, 17 Aug 2023 17:43:38 +0000 Subject: [PATCH 1/2] Failing test --- .../dont_inline_type_id.call.Inline.diff | 37 +++++++++++++++++++ tests/mir-opt/dont_inline_type_id.rs | 15 ++++++++ 2 files changed, 52 insertions(+) create mode 100644 tests/mir-opt/dont_inline_type_id.call.Inline.diff create mode 100644 tests/mir-opt/dont_inline_type_id.rs diff --git a/tests/mir-opt/dont_inline_type_id.call.Inline.diff b/tests/mir-opt/dont_inline_type_id.call.Inline.diff new file mode 100644 index 0000000000000..d67235ddbd2ba --- /dev/null +++ b/tests/mir-opt/dont_inline_type_id.call.Inline.diff @@ -0,0 +1,37 @@ +- // MIR for `call` before Inline ++ // MIR for `call` after Inline + + fn call(_1: &T) -> TypeId { + debug s => _1; + let mut _0: std::any::TypeId; + let mut _2: &T; ++ scope 1 (inlined ::type_id) { ++ debug self => _2; ++ scope 2 (inlined TypeId::of::) { ++ let _3: u128; ++ let mut _4: u128; ++ scope 3 { ++ debug t => _3; ++ } ++ } ++ } + + bb0: { + StorageLive(_2); + _2 = &(*_1); +- _0 = ::type_id(move _2) -> [return: bb1, unwind unreachable]; ++ StorageLive(_3); ++ _3 = std::intrinsics::type_id::() -> [return: bb1, unwind unreachable]; + } + + bb1: { ++ StorageLive(_4); ++ _4 = _3; ++ _0 = TypeId { t: move _4 }; ++ StorageDead(_4); ++ StorageDead(_3); + StorageDead(_2); + return; + } + } + diff --git a/tests/mir-opt/dont_inline_type_id.rs b/tests/mir-opt/dont_inline_type_id.rs new file mode 100644 index 0000000000000..6f4c21a3ab1ce --- /dev/null +++ b/tests/mir-opt/dont_inline_type_id.rs @@ -0,0 +1,15 @@ +// unit-test: Inline +// compile-flags: --crate-type=lib -C panic=abort + +use std::any::Any; +use std::any::TypeId; + +struct A { + a: i32, + b: T, +} + +// EMIT_MIR dont_inline_type_id.call.Inline.diff +fn call(s: &T) -> TypeId { + s.type_id() +} From a30ad3a5a6362c31b16d3bd3f21bbae1315bfaec Mon Sep 17 00:00:00 2001 From: Michael Goulet Date: Mon, 21 Aug 2023 22:39:34 +0000 Subject: [PATCH 2/2] Don't resolve generic instances if they may be shadowed by dyn --- compiler/rustc_middle/src/ty/sty.rs | 27 +++++++++++++++++++ compiler/rustc_ty_utils/src/instance.rs | 25 ++++++++++++++++- .../dont_inline_type_id.call.Inline.diff | 19 +------------ tests/mir-opt/dont_inline_type_id.rs | 2 +- ...line_generically_if_sized.call.Inline.diff | 24 +++++++++++++++++ tests/mir-opt/inline_generically_if_sized.rs | 17 ++++++++++++ .../dont-propagate-generic-instance-2.rs | 26 ++++++++++++++++++ .../dont-propagate-generic-instance.rs | 24 +++++++++++++++++ 8 files changed, 144 insertions(+), 20 deletions(-) create mode 100644 tests/mir-opt/inline_generically_if_sized.call.Inline.diff create mode 100644 tests/mir-opt/inline_generically_if_sized.rs create mode 100644 tests/ui/const_prop/dont-propagate-generic-instance-2.rs create mode 100644 tests/ui/const_prop/dont-propagate-generic-instance.rs diff --git a/compiler/rustc_middle/src/ty/sty.rs b/compiler/rustc_middle/src/ty/sty.rs index edab7fe58baea..a14d09bbb7006 100644 --- a/compiler/rustc_middle/src/ty/sty.rs +++ b/compiler/rustc_middle/src/ty/sty.rs @@ -2945,6 +2945,33 @@ impl<'tcx> Ty<'tcx> { _ => false, } } + + pub fn is_known_rigid(self) -> bool { + match self.kind() { + Bool + | Char + | Int(_) + | Uint(_) + | Float(_) + | Adt(_, _) + | Foreign(_) + | Str + | Array(_, _) + | Slice(_) + | RawPtr(_) + | Ref(_, _, _) + | FnDef(_, _) + | FnPtr(_) + | Dynamic(_, _, _) + | Closure(_, _) + | Generator(_, _, _) + | GeneratorWitness(_) + | GeneratorWitnessMIR(_, _) + | Never + | Tuple(_) => true, + Error(_) | Infer(_) | Alias(_, _) | Param(_) | Bound(_, _) | Placeholder(_) => false, + } + } } /// Extra information about why we ended up with a particular variance. diff --git a/compiler/rustc_ty_utils/src/instance.rs b/compiler/rustc_ty_utils/src/instance.rs index da2958bf56e8c..91f1c21310e30 100644 --- a/compiler/rustc_ty_utils/src/instance.rs +++ b/compiler/rustc_ty_utils/src/instance.rs @@ -141,11 +141,34 @@ fn resolve_associated_item<'tcx>( false } }; - if !eligible { return Ok(None); } + // HACK: We may have overlapping `dyn Trait` built-in impls and + // user-provided blanket impls. Detect that case here, and return + // ambiguity. + // + // This should not affect totally monomorphized contexts, only + // resolve calls that happen polymorphically, such as the mir-inliner + // and const-prop (and also some lints). + let self_ty = rcvr_args.type_at(0); + if !self_ty.is_known_rigid() { + let predicates = tcx + .predicates_of(impl_data.impl_def_id) + .instantiate(tcx, impl_data.args) + .predicates; + let sized_def_id = tcx.lang_items().sized_trait(); + // If we find a `Self: Sized` bound on the item, then we know + // that `dyn Trait` can certainly never apply here. + if !predicates.into_iter().filter_map(ty::Clause::as_trait_clause).any(|clause| { + Some(clause.def_id()) == sized_def_id + && clause.skip_binder().self_ty() == self_ty + }) { + return Ok(None); + } + } + // Any final impl is required to define all associated items. if !leaf_def.item.defaultness(tcx).has_value() { let guard = tcx.sess.delay_span_bug( diff --git a/tests/mir-opt/dont_inline_type_id.call.Inline.diff b/tests/mir-opt/dont_inline_type_id.call.Inline.diff index d67235ddbd2ba..80cfe07b0cbf6 100644 --- a/tests/mir-opt/dont_inline_type_id.call.Inline.diff +++ b/tests/mir-opt/dont_inline_type_id.call.Inline.diff @@ -5,31 +5,14 @@ debug s => _1; let mut _0: std::any::TypeId; let mut _2: &T; -+ scope 1 (inlined ::type_id) { -+ debug self => _2; -+ scope 2 (inlined TypeId::of::) { -+ let _3: u128; -+ let mut _4: u128; -+ scope 3 { -+ debug t => _3; -+ } -+ } -+ } bb0: { StorageLive(_2); _2 = &(*_1); -- _0 = ::type_id(move _2) -> [return: bb1, unwind unreachable]; -+ StorageLive(_3); -+ _3 = std::intrinsics::type_id::() -> [return: bb1, unwind unreachable]; + _0 = ::type_id(move _2) -> [return: bb1, unwind unreachable]; } bb1: { -+ StorageLive(_4); -+ _4 = _3; -+ _0 = TypeId { t: move _4 }; -+ StorageDead(_4); -+ StorageDead(_3); StorageDead(_2); return; } diff --git a/tests/mir-opt/dont_inline_type_id.rs b/tests/mir-opt/dont_inline_type_id.rs index 6f4c21a3ab1ce..d8a5663609443 100644 --- a/tests/mir-opt/dont_inline_type_id.rs +++ b/tests/mir-opt/dont_inline_type_id.rs @@ -10,6 +10,6 @@ struct A { } // EMIT_MIR dont_inline_type_id.call.Inline.diff -fn call(s: &T) -> TypeId { +pub fn call(s: &T) -> TypeId { s.type_id() } diff --git a/tests/mir-opt/inline_generically_if_sized.call.Inline.diff b/tests/mir-opt/inline_generically_if_sized.call.Inline.diff new file mode 100644 index 0000000000000..0cf4565dc994c --- /dev/null +++ b/tests/mir-opt/inline_generically_if_sized.call.Inline.diff @@ -0,0 +1,24 @@ +- // MIR for `call` before Inline ++ // MIR for `call` after Inline + + fn call(_1: &T) -> i32 { + debug s => _1; + let mut _0: i32; + let mut _2: &T; ++ scope 1 (inlined ::bar) { ++ debug self => _2; ++ } + + bb0: { + StorageLive(_2); + _2 = &(*_1); +- _0 = ::bar(move _2) -> [return: bb1, unwind unreachable]; +- } +- +- bb1: { ++ _0 = const 0_i32; + StorageDead(_2); + return; + } + } + diff --git a/tests/mir-opt/inline_generically_if_sized.rs b/tests/mir-opt/inline_generically_if_sized.rs new file mode 100644 index 0000000000000..1acfff7a56b7c --- /dev/null +++ b/tests/mir-opt/inline_generically_if_sized.rs @@ -0,0 +1,17 @@ +// unit-test: Inline +// compile-flags: --crate-type=lib -C panic=abort + +trait Foo { + fn bar(&self) -> i32; +} + +impl Foo for T { + fn bar(&self) -> i32 { + 0 + } +} + +// EMIT_MIR inline_generically_if_sized.call.Inline.diff +pub fn call(s: &T) -> i32 { + s.bar() +} diff --git a/tests/ui/const_prop/dont-propagate-generic-instance-2.rs b/tests/ui/const_prop/dont-propagate-generic-instance-2.rs new file mode 100644 index 0000000000000..e5525af23f0e6 --- /dev/null +++ b/tests/ui/const_prop/dont-propagate-generic-instance-2.rs @@ -0,0 +1,26 @@ +// run-pass + +#![feature(inline_const)] + +// Makes sure we don't propagate generic instances of `Self: ?Sized` blanket impls. +// This is relevant when we have an overlapping impl and builtin dyn instance. +// See for more context. + +trait Trait { + fn foo(&self) -> &'static str; +} + +impl Trait for T { + fn foo(&self) -> &'static str { + std::any::type_name::() + } +} + +fn bar() -> fn(&T) -> &'static str { + const { Trait::foo as fn(&T) -> &'static str } + // If const prop were to propagate the instance +} + +fn main() { + assert_eq!("i32", bar::()(&1i32)); +} diff --git a/tests/ui/const_prop/dont-propagate-generic-instance.rs b/tests/ui/const_prop/dont-propagate-generic-instance.rs new file mode 100644 index 0000000000000..5994961b887b1 --- /dev/null +++ b/tests/ui/const_prop/dont-propagate-generic-instance.rs @@ -0,0 +1,24 @@ +// run-pass + +// Makes sure we don't propagate generic instances of `Self: ?Sized` blanket impls. +// This is relevant when we have an overlapping impl and builtin dyn instance. +// See for more context. + +trait Trait { + fn foo(&self) -> &'static str; +} + +impl Trait for T { + fn foo(&self) -> &'static str { + std::any::type_name::() + } +} + +const fn bar() -> fn(&T) -> &'static str { + Trait::foo + // If const prop were to propagate the instance +} + +fn main() { + assert_eq!("i32", bar::()(&1i32)); +}