discard default auto trait impls if explicit ones exist (rebase of #85048)

[Ddystopia]

Rebase of #85048

[rustbot]

r? @b-naber

(rustbot has picked a reviewer for you, use r? to override)

[Ddystopia]

r? @lcnr

[lcnr]

thanks for looking into this ❤️ lets start with crater

@bors try

[bors]

⌛ Trying commit cb2716b0087ddf3b200da9cc6c8cc83511d0e6ed with merge 262225c6fda57a615495e99544a8f50d716aa702...

[bors]

💔 Test failed - checks-actions

[lcnr]

oh, this PR is now breaking tokio-util 😓 that is unexpected and feels wrong 🤔

[lcnr]

error[E0277]: `impl Future<Output = Result<(), tokio::sync::mpsc::error::SendError<_>>>` cannot be sent between threads safely
Error:   --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-util-0.6.9/src/sync/mpsc.rs:43:43
   |
43 |             inner: ReusableBoxFuture::new(make_future(None)),
   |                    ---------------------- ^^^^^^^^^^^^^^^^^ `impl Future<Output = Result<(), tokio::sync::mpsc::error::SendError<_>>>` cannot be sent between threads safely
   |                    |
   |                    required by a bound introduced by this call
   |
   = help: the trait `Send` is not implemented for `impl Future<Output = Result<(), tokio::sync::mpsc::error::SendError<_>>>`
note: required by a bound in `ReusableBoxFuture::<T>::new`
  --> /cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-util-0.6.9/src/sync/reusable_box.rs:21:33
   |
19 |     pub fn new<F>(future: F) -> Self
   |            --- required by a bound in this associated function
20 |     where
21 |         F: Future<Output = T> + Send + 'static,
   |                                 ^^^^ required by this bound in `ReusableBoxFuture::<T>::new`

[lcnr]

ah, this is wrong for opaque types 😅 we need to always emit consider AutoImplCandidates for them if they don't also have a ProjectionCandidate. This is a bit of a mess in the old solver.

The issue is that we consider all impls to potentially apply for opaque types, but we should instead actually leak their underlying type and check whether it implements the auto trait

cc @compiler-errors given that we've recently talked about auto traits and opaques :>

[lcnr]

@bors try

[bors]

⌛ Trying commit 2cc873f3f6dd6bb6330eae720b972d0018317a5d with merge d48499c087dc9ccf49e58dd584b2df4a2c08657c...

[bors]

☀️ Try build successful - checks-actions
Build commit: d48499c087dc9ccf49e58dd584b2df4a2c08657c (d48499c087dc9ccf49e58dd584b2df4a2c08657c)

[bors]

☀️ Try build successful - checks-actions
Build commit: d48499c087dc9ccf49e58dd584b2df4a2c08657c (d48499c087dc9ccf49e58dd584b2df4a2c08657c)

[lcnr]

@craterbot check

[craterbot]

👌 Experiment pr-113312 created and queued.
🤖 Automatically detected try build d48499c087dc9ccf49e58dd584b2df4a2c08657c
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[bors]

✌️ @Ddystopia, you can now approve this pull request!

If @lcnr told you to "r=me" after making some further change, please make that change, then do @bors r=@lcnr

[Ddystopia]

@bors r=@lcnr

[bors]

📌 Commit 3715934 has been approved by lcnr

It is now in the queue for this repository.

[bors]

⌛ Testing commit 3715934 with merge e4c98ca...

[bors]

☀️ Test successful - checks-actions
Approved by: lcnr
Pushing e4c98ca to master...

[RalfJung]

There's an empty file, impl-trait/auto-trait-leak. That looks like an accident?

[RalfJung]

This would be worth a comment explaining what is being tested: that the manual impl Send for Foo should "block" the implicit impl

[lcnr]

Not sure how much explanation makes sense here, though a "Tests that we don't incorrectly allow overlap between a builtin auto trait impl and a user written one. See #83857 for more details." would be good here.

[lcnr]

@Ddystopia are you up to open a PR here? otherwise I can do it next week

[Ddystopia]

@Ddystopia are you up to open a PR here? otherwise I can do it next week

Yes, sure

[rust-timer]

Finished benchmarking commit (e4c98ca): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	0.3%	[0.3%, 0.3%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.8%	[2.2%, 3.9%]	3
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.1%	[-1.4%, -0.9%]	2
All ❌✅ (primary)	-	-	0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 651.183s -> 651.8s (0.09%)

[wangyanci]

Due to this PR, this code cannot be compiled now.

#![feature(negative_impls, auto_traits)]

auto trait NotEqual {}

impl<T> ! NotEqual for (T, T) {} // expected: NotEqual is not implemented only and only if for tuple(a, b) a and b are of the same type

fn call<T>(_: T) where T: NotEqual {} // expected: only tuple(a, b) a and b are of the same type compile err; but a and b are of the different type compile ok

fn main() {
    // compile ok in all nightly;  it's exactly what i expected
    call(1);
    call((1,1.2, 1.3));

    // compile not ok in all nightly;  yes, it's exactly what i expected
    // call((1,1));

    // compile ok in nightly-2023-07-28(cmd: cargo +nightly-2023-07-28 run);  yes, it's exactly what i expected
    // compile not ok after nightly-2023-07-29 version(cmd: cargo +nightly-2023-07-29 run);  it's not exactly what i expected! why? for what reasons did it change?
    call((1,1.2));
}

impl ! NotEqual for (T, T) {} is just means NotEqual is not implemented only when tuple (a, b) a and b are of the same type, it should be reasonable

[RalfJung]

This PR fixed a soundness bug (#84857), so the old behavior was definitely not reasonable enough.

Both auto traits and negative impls are features meant for internal use only, so you can easily run into limitations. A lot of work will be needed before they can be stabilized. (And for auto trait, as far as I know, there's no plans to ever stabilize them, they are just an implementation detail of the standard library.)