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

Don't resolve generic impls that may be shadowed by dyn built-in impls #114941

Merged
merged 2 commits into from
Sep 19, 2023
Merged
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
27 changes: 27 additions & 0 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2945,6 +2945,33 @@ impl<'tcx> Ty<'tcx> {
_ => false,
}
}

pub fn is_known_rigid(self) -> bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to have a doc comment explaining what "rigid" means.

For instance, (_, _) would be considered rigid by this check, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I can add this.

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.
Expand Down
25 changes: 24 additions & 1 deletion compiler/rustc_ty_utils/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
20 changes: 20 additions & 0 deletions tests/mir-opt/dont_inline_type_id.call.Inline.diff
Original file line number Diff line number Diff line change
@@ -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;
}
}

15 changes: 15 additions & 0 deletions tests/mir-opt/dont_inline_type_id.rs
Original file line number Diff line number Diff line change
@@ -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()
}
24 changes: 24 additions & 0 deletions tests/mir-opt/inline_generically_if_sized.call.Inline.diff
Original file line number Diff line number Diff line change
@@ -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;
}
}

17 changes: 17 additions & 0 deletions tests/mir-opt/inline_generically_if_sized.rs
Original file line number Diff line number Diff line change
@@ -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()
}
26 changes: 26 additions & 0 deletions tests/ui/const_prop/dont-propagate-generic-instance-2.rs
Original file line number Diff line number Diff line change
@@ -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));
}
24 changes: 24 additions & 0 deletions tests/ui/const_prop/dont-propagate-generic-instance.rs
Original file line number Diff line number Diff line change
@@ -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));
}
Loading