Rollup merge of #114941 - compiler-errors:inline-shadowed-by-dyn, r=lcnr

Don't resolve generic impls that may be shadowed by dyn built-in impls **NOTE:** This is a hack. This is not trying to be a general fix for the issue that we've allowed overlapping built-in trait object impls and user-written impls for quite a long time, and traits like `Any` rely on this (#57893) -- this PR specifically aims to mitigate a new unsoundness that is uncovered by the MIR inliner (#114928) that interacts with this pre-existing issue. Builtin `dyn Trait` impls may overlap with user-provided blanket impls (`impl<T: ?Sized> Trait for T`) in generic contexts. This leads to bugs when instances are resolved in polymorphic contexts, since we typically prefer object candidates over impl candidates. This PR implements a (hacky) heuristic to `resolve_associated_item` to account for that unfortunate hole in the type system -- we now bail with ambiguity if we try to resolve a non-rigid instance whose self type is not known to be sized. This makes sure we can still inline instances like `impl<T: Sized> Trait for T`, which can never overlap with `dyn Trait`'s built-in impl, but we avoid inlining an impl that may be shadowed by a `dyn Trait`. Fixes #114928
rust-lang · Sep 19, 2023 · 66b7bdf · 66b7bdf
2 parents 3f68468 a30ad3a
commit 66b7bdf
Show file tree

Hide file tree

Showing 8 changed files with 177 additions and 1 deletion.
diff --git 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
@@ -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
@@ -0,0 1,20 @@
 - // 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;
 
  bb0: {
  StorageLive(_2);
  _2 = &(*_1);
  _0 = <T as Any>::type_id(move _2) -> [return: bb1, unwind unreachable];
  }
 
  bb1: {
  StorageDead(_2);
  return;
  }
  }
 
diff --git a/tests/mir-opt/dont_inline_type_id.rs 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<T: ?Sized 'static> {
  a: i32,
  b: T,
 }
 
 // EMIT_MIR dont_inline_type_id.call.Inline.diff
 pub fn call<T: ?Sized 'static>(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
@@ -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 <T as Foo>::bar) {
   debug self => _2;
   }
 
  bb0: {
  StorageLive(_2);
  _2 = &(*_1);
 - _0 = <T as Foo>::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
@@ -0,0 1,17 @@
 // unit-test: Inline
 // compile-flags: --crate-type=lib -C panic=abort
 
 trait Foo {
  fn bar(&self) -> i32;
 }
 
 impl<T> Foo for T {
  fn bar(&self) -> i32 {
  0
  }
 }
 
 // EMIT_MIR inline_generically_if_sized.call.Inline.diff
 pub fn call<T>(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
@@ -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 <https://github.com/rust-lang/rust/pull/114941> for more context.
 
 trait Trait {
  fn foo(&self) -> &'static str;
 }
 
 impl<T: ?Sized> Trait for T {
  fn foo(&self) -> &'static str {
  std::any::type_name::<T>()
  }
 }
 
 fn bar<T: ?Sized>() -> 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::<dyn Trait>()(&1i32));
 }
diff --git a/tests/ui/const_prop/dont-propagate-generic-instance.rs 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 <https://github.com/rust-lang/rust/pull/114941> for more context.
 
 trait Trait {
  fn foo(&self) -> &'static str;
 }
 
 impl<T: ?Sized> Trait for T {
  fn foo(&self) -> &'static str {
  std::any::type_name::<T>()
  }
 }
 
 const fn bar<T: ?Sized>() -> fn(&T) -> &'static str {
  Trait::foo
  // If const prop were to propagate the instance
 }
 
 fn main() {
  assert_eq!("i32", bar::<dyn Trait>()(&1i32));
 }