Reject ?Trait bounds in various places where we unconditionally warned since 1.0

[oli-obk]

fixes #135730
fixes #135809

Also a breaking change, so let's see what crater says.

This has been an unconditional warning since before 1.0

[rustbot]

r? @compiler-errors

rustbot has assigned @compiler-errors.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

HIR ty lowering was modified

cc @fmease

[oli-obk]

@bors try

[bors]

⌛ Trying commit 757c75f with merge 173780a...

[bors]

☀️ Try build successful - checks-actions
Build commit: 173780a (173780a87744406b7489b28349c0176f14bb3881)

[oli-obk]

@craterbot check

[craterbot]

👌 Experiment pr-135841 created and queued.
🤖 Automatically detected try build 173780a
🔍 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

[craterbot]

🚧 Experiment pr-135841 is now running

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

[craterbot]

🎉 Experiment pr-135841 is completed!
📊 5 regressed and 4 fixed (571795 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[oli-obk]

Hi @rust-lang/types, it turns out we've accepted ?Trait bounds in various places since 1.0 and just emitted an unconditional warning. I have been unable to find the reason for it, but since it doesn't do anything and emitted a warning, it seems no one actually wrote this (crater shows no regressions when turning it into a hard error). Therefor I propose to just turn it into a hard error and subsequently make our lives easier because we don't need to patch situations everywhere in the compiler.

One common source of problems with such bounds is that they can have generics or associated types/consts and all of these just get dropped on the floor and never evaluated, normalized, or otherwise processed in the type system. This leads to fun bugs like not evaluating constants or just ICEing because opaque types never got defined.

@rfcbot fcp merge

[rfcbot]

Team member @oli-obk has proposed to merge this. The next step is review by the rest of the tagged team members:

• @BoxyUwU
• @compiler-errors
• @jackh726
• @lcnr
• @nikomatsakis
• @oli-obk
• @spastorino

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[lcnr]

cc @rust-lang/lang for info

I think this is fine as a pure types team FCP. Allowing these bounds is causing bugs without any benefit.

[nikomatsakis]

@rfcbot reviewed

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[fmease]

I don't want to block this PR on surface level things like phrasing, so feel free to ignore this and I'll submit this as a follow-up PR. I've never liked how this specific diagnostic is formulated, it's super verbose.

Now with this being a hard error this sentence doesn't quite make sense anymore. I'd rather see a message that's straight to the point like `?` can only be applied to `Sized` (similar to `~const` can only be applied to `#[const_trait]` traits or `async` bound modifier only allowed on `Fn`/`FnMut`/`FnOnce` traits) and defer any further explanations to diagnostic note/help messages if at all (cc error codes).

If we wanted to feel fancy we could provide a structured suggestion for removing the entire bound.

[compiler-errors]

How about we do this as a follow-up

[compiler-errors]

@bors r+ rollup

[bors]

📌 Commit c294da3 has been approved by compiler-errors

It is now in the queue for this repository.

[fmease]

Note that this PR doesn't catch all occurrences of ?Trait, Trait≠Sized because performing the validation in add_sized_bound is super backwards and because we don't reject maybe-bounds in ATB position (#135229).

Meaning the following code still compiles successfully and without warnings:

fn _f(_: impl Iterator<Item: ?Copy>) {}

I'll rewrite the way we validate and handle maybe-bounds because I wanted to do that anyway for the longest time (fixing #136944 in the process).

(Lastly, I'll make it so we don't error at all if the internal feature more_maybe_bounds (#121676) is enabled; while I'm personally not in favor of that internal feature, this PR did partially regress it)

(Re. ATBs / #135229, the crater run didn't show any regressions but GH search revealed users using them (mistakenly), so I'll either make it a FCW or hard error regardless (in yet another PR))