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

discard default auto trait impls if explicit ones exist #85048

Closed
wants to merge 2 commits into from
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
Original file line number Diff line number Diff line change
Expand Up @@ -332,11 +332,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {

self.assemble_candidates_from_projected_tys(obligation, &mut candidates);
self.assemble_candidates_from_caller_bounds(stack, &mut candidates)?;
// Auto implementations have lower priority, so we only
// consider triggering a default if there is no other impl that can apply.
if candidates.vec.is_empty() {
self.assemble_candidates_from_auto_impls(obligation, &mut candidates);
}
self.assemble_candidates_from_auto_impls(obligation, &mut candidates);
}
debug!("candidate list size: {}", candidates.vec.len());
Ok(candidates)
Expand Down Expand Up @@ -600,7 +596,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// for an example of a test case that exercises
// this path.
}
ty::Infer(ty::TyVar(_)) => {
ty::Infer(ty::TyVar(_) | ty::IntVar(_) | ty::FloatVar(_)) => {
// The auto impl might apply; we don't know.
candidates.ambiguous = true;
}
Expand All @@ -620,7 +616,62 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}
}

_ => candidates.vec.push(AutoImplCandidate(def_id)),
ty::Placeholder(..)
| ty::Bound(..)
| ty::Infer(ty::FreshTy(_) | ty::FreshIntTy(_) | ty::FreshFloatTy(_)) => {
bug!(
"asked to assemble auto trait candidates of unexpected type: {:?}",
self_ty
);
}

ty::Opaque(_, _)
if candidates.vec.iter().any(|c| matches!(c, ProjectionCandidate(_))) =>
{
// We do not generate an auto impl candidate for `impl Trait`s which already
// reference our auto trait.
//
// For example during candidate assembly for `impl Send: Send`, we don't have
// to look at the constituent types for this opaque types to figure out that this
// trivially holds.
//
// Note that this is only sound as projection candidates of opaque types
// are always applicable for auto traits.
}

ty::Bool
| ty::Char
| ty::Int(_)
| ty::Uint(_)
| ty::Float(_)
| ty::Str
| ty::Array(_, _)
| ty::Slice(_)
| ty::Adt(..)
| ty::RawPtr(_)
| ty::Ref(..)
| ty::FnDef(..)
| ty::FnPtr(_)
| ty::Closure(_, _)
| ty::Generator(..)
| ty::Never
| ty::Tuple(_)
| ty::Opaque(_, _)
| ty::GeneratorWitness(_) => {
// Only consider auto impls if there are no manual impls for the root of `self_ty`.
//
// For example, we only consider auto candidates for `&i32: Auto` if no explicit impl
// for `&SomeType: Auto` exists. Due to E0321 the only crate where impls
// for `&SomeType: Auto` can be defined is the crate where `Auto` has been defined.
//
// Generally, we have to guarantee that for all `SimplifiedType`s the only crate
// which may define impls for that type is either the crate defining the type
// or the trait. This should be guaranteed by the orphan check.
if self.tcx().find_map_relevant_impl(def_id, self_ty, |_| Some(())).is_none() {
candidates.vec.push(AutoImplCandidate(def_id))
}
}
ty::Error(_) => {} // do not add an auto trait impl for `ty::Error` for now.
}
}
}
Expand Down
25 changes: 18 additions & 7 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1518,6 +1518,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
/// candidates and prefer where-clause candidates.
///
/// See the comment for "SelectionCandidate" for more details.
#[instrument(level = "debug", skip(self, needs_infer))]
fn candidate_should_be_dropped_in_favor_of(
&mut self,
sized_predicate: bool,
Expand All @@ -1542,13 +1543,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// This is a fix for #53123 and prevents winnowing from accidentally extending the
// lifetime of a variable.
match (&other.candidate, &victim.candidate) {
(_, AutoImplCandidate(..)) | (AutoImplCandidate(..), _) => {
bug!(
"default implementations shouldn't be recorded \
when there are other valid candidates"
);
}

// (*)
(
BuiltinCandidate { has_nested: false }
Expand Down Expand Up @@ -1610,6 +1604,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
(
ParamCandidate(ref cand),
ImplCandidate(..)
| AutoImplCandidate(_)
| ClosureCandidate
| GeneratorCandidate
| FnPointerCandidate { .. }
Expand All @@ -1621,6 +1616,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
) => !is_global(cand),
(
ImplCandidate(_)
| AutoImplCandidate(_)
| ClosureCandidate
| GeneratorCandidate
| FnPointerCandidate { .. }
Expand Down Expand Up @@ -1651,6 +1647,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
(
ObjectCandidate(_) | ProjectionCandidate(_),
ImplCandidate(..)
| AutoImplCandidate(_)
| ClosureCandidate
| GeneratorCandidate
| FnPointerCandidate { .. }
Expand All @@ -1663,6 +1660,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {

(
ImplCandidate(..)
| AutoImplCandidate(_)
| ClosureCandidate
| GeneratorCandidate
| FnPointerCandidate { .. }
Expand Down Expand Up @@ -1741,6 +1739,19 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}
}

(AutoImplCandidate(_), ImplCandidate(_)) | (ImplCandidate(_), AutoImplCandidate(_)) => {
false
}

(AutoImplCandidate(_), _) | (_, AutoImplCandidate(_)) => {
bug!(
"default implementations shouldn't be recorded \
when there are other global candidates: {:?} {:?}",
other,
victim
);
}

// Everything else is ambiguous
(
ImplCandidate(_)
Expand Down
29 changes: 29 additions & 0 deletions src/test/ui/auto-traits/issue-83857-ub.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
struct Always<T, U>(T, U);
unsafe impl<T, U> Send for Always<T, U> {}
struct Foo<T, U>(Always<T, U>);

trait False {}
unsafe impl<U: False> Send for Foo<u32, U> {}

trait WithAssoc {
type Output;
}
impl<T: Send> WithAssoc for T {
type Output = Self;
}
impl WithAssoc for Foo<u32, ()> {
type Output = Box<i32>;
}

fn generic<T, U>(v: Foo<T, U>, f: fn(<Foo<T, U> as WithAssoc>::Output) -> i32) {
//~^ ERROR `Foo<T, U>` cannot be sent between threads safely
f(foo(v));
}

fn foo<T: Send>(x: T) -> <T as WithAssoc>::Output {
x
}

fn main() {
generic(Foo(Always(0, ())), |b| *b);
}
16 changes: 16 additions & 0 deletions src/test/ui/auto-traits/issue-83857-ub.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
error[E0277]: `Foo<T, U>` cannot be sent between threads safely
--> $DIR/issue-83857-ub.rs:18:38
|
LL | fn generic<T, U>(v: Foo<T, U>, f: fn(<Foo<T, U> as WithAssoc>::Output) -> i32) {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Foo<T, U>` cannot be sent between threads safely
|
= help: the trait `Send` is not implemented for `Foo<T, U>`
= note: required because of the requirements on the impl of `WithAssoc` for `Foo<T, U>`
help: consider introducing a `where` bound, but there might be an alternative better way to express this requirement
|
LL | fn generic<T, U>(v: Foo<T, U>, f: fn(<Foo<T, U> as WithAssoc>::Output) -> i32) where Foo<T, U>: Send {
| +++++++++++++++++++++

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.
6 changes: 3 additions & 3 deletions src/test/ui/generator/auto-trait-regions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ fn assert_foo<T: Foo>(f: T) {}
fn main() {
// Make sure 'static is erased for generator interiors so we can't match it in trait selection
let x: &'static _ = &OnlyFooIfStaticRef(No);
let gen = || {
let gen = move || {
let x = x;
yield;
assert_foo(x);
Expand All @@ -34,15 +34,15 @@ fn main() {

// Allow impls which matches any lifetime
let x = &OnlyFooIfRef(No);
let gen = || {
let gen = move || {
let x = x;
yield;
assert_foo(x);
};
assert_foo(gen); // ok

// Disallow impls which relates lifetimes in the generator interior
let gen = || {
let gen = move || {
let a = A(&mut true, &mut true, No);
yield;
assert_foo(a);
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/impl-trait/auto-trait-leak.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ fn main() {
// return type, which can't depend on the obligation.
fn cycle1() -> impl Clone {
//~^ ERROR cycle detected
//~| ERROR cycle detected
//~| ERROR cycle detected
send(cycle2().clone());

Rc::new(Cell::new(5))
Expand Down
Loading