Remove dead code and extend test coverage and diagnostics around it
I was staring a bit at the `dont_niche_optimize_enum` variable and figured out that part of it is dead code (at least today it is). I changed the diagnostic and test around the code that makes that part dead code, so everything that makes removing that code sound is visible in this PR
We lost the following comment during refactorings:
The current code for niche-filling relies on variant indices instead of actual discriminants, so enums with explicit discriminants (RFC 2363) would misbehave.
Having multiple relaxed bounds like `?Sized + ?Iterator` is actually *fine*.
We actually want to reject *duplicate* relaxed bounds like `?Sized + ?Sized`
because these most certainly represent a user error.
Note that this doesn't mean that we accept more code because a bound like
`?Iterator` is still invalid as it's not relaxing a *default* trait and
the only way to define / use more default bounds is under the experimental
and internal feature `more_maybe_bounds` plus `lang_items` plus unstable
flag `-Zexperimental-default-bounds` (historical context: for the longest
time, bounds like `?Iterator` were actually allowed and lead to a hard
warning).
Ultimately, this simply *reframes* the diagnostic. The scope of
`more_maybe_bounds` / `-Zexperimental-default-bounds` remains unchanged
as well.
* The phrasing "only does something for" made sense back when this
diagnostic was a (hard) warning. Now however, it's simply a hard
error and thus completely rules out "doing something".
* The primary message was way too long
* The new wording more closely mirrors the wording we use for applying
other bound modifiers (like `const` and `async`) to incompatible
traits.
* "all other traits are not bound by default" is no longer accurate
under Sized Hierarchy. E.g., traits and assoc tys are (currently)
bounded by `MetaSized` by default but can't be relaxed using
`?MetaSized` (instead, you relax it by adding `PointeeSized`).
* I've decided against adding any diagnositic notes or suggestions
for now like "trait `Trait` can't be relaxed as it's not bound by
default" which would be incorrect for `MetaSized` and assoc tys
as mentioned above) or "consider changing `?MetaSized` to
`PointeeSized`" as the Sized Hierarchy impl is still WIP)
`-Zhigher-ranked-assumptions`: Consider WF of coroutine witness when proving outlives assumptions
### TL;DR
This PR introduces an unstable flag `-Zhigher-ranked-assumptions` which tests out a new algorithm for dealing with some of the higher-ranked outlives problems that come from auto trait bounds on coroutines. See:
* rust-lang/rust#110338
While it doesn't fix all of the issues, it certainly fixed many of them, so I'd like to get this landed so people can test the flag on their own code.
### Background
Consider, for example:
```rust
use std::future::Future;
trait Client {
type Connecting<'a>: Future + Send
where
Self: 'a;
fn connect(&self) -> Self::Connecting<'_>;
}
fn call_connect<C>(c: C) -> impl Future + Send
where
C: Client + Send + Sync,
{
async move { c.connect().await }
}
```
Due to the fact that we erase the lifetimes in a coroutine, we can think of the interior type of the async block as something like: `exists<'r, 's> { C, &'r C, C::Connecting<'s> }`. The first field is the `c` we capture, the second is the auto-ref that we perform on the call to `.connect()`, and the third is the resulting future we're awaiting at the first and only await point. Note that every region is uniquified differently in the interior types.
For the async block to be `Send`, we must prove that both of the interior types are `Send`. First, we have an `exists<'r, 's>` binder, which needs to be instantiated universally since we treat the regions in this binder as *unknown*[^exist]. This gives us two types: `{ &'!r C, C::Connecting<'!s> }`. Proving `&'!r C: Send` is easy due to a [`Send`](https://doc.rust-lang.org/nightly/std/marker/trait.Send.html#impl-Send-for-%26T) impl for references.
Proving `C::Connecting<'!s>: Send` can only be done via the item bound, which then requires `C: '!s` to hold (due to the `where Self: 'a` on the associated type definition). Unfortunately, we don't know that `C: '!s` since we stripped away any relationship between the interior type and the param `C`. This leads to a bogus borrow checker error today!
### Approach
Coroutine interiors are well-formed by virtue of them being borrow-checked, as long as their callers are invoking their parent functions in a well-formed way, then substitutions should also be well-formed. Therefore, in our example above, we should be able to deduce the assumption that `C: '!s` holds from the well-formedness of the interior type `C::Connecting<'!s>`.
This PR introduces the notion of *coroutine assumptions*, which are the outlives assumptions that we can assume hold due to the well-formedness of a coroutine's interior types. These are computed alongside the coroutine types in the `CoroutineWitnessTypes` struct. When we instantiate the binder when proving an auto trait for a coroutine, we instantiate the `CoroutineWitnessTypes` and stash these newly instantiated assumptions in the region storage in the `InferCtxt`. Later on in lexical region resolution or MIR borrowck, we use these registered assumptions to discharge any placeholder outlives obligations that we would otherwise not be able to prove.
### How well does it work?
I've added a ton of tests of different reported situations that users have shared on issues like rust-lang/rust#110338, and an (anecdotally) large number of those examples end up working straight out of the box! Some limitations are described below.
### How badly does it not work?
The behavior today is quite rudimentary, since we currently discharge the placeholder assumptions pretty early in region resolution. This manifests itself as some limitations on the code that we accept.
For example, `tests/ui/async-await/higher-ranked-auto-trait-11.rs` continues to fail. In that test, we must prove that a placeholder is equal to a universal for a param-env candidate to hold when proving an auto trait, e.g. `'!1 = 'a` is required to prove `T: Trait<'!1>` in a param-env that has `T: Trait<'a>`. Unfortunately, at that point in the MIR body, we only know that the placeholder is equal to some body-local existential NLL var `'?2`, which only gets equated to the universal `'a` when being stored into the return local later on in MIR borrowck.
This could be fixed by integrating these assumptions into the type outlives machinery in a more first-class way, and delaying things to the end of MIR typeck when we know the full relationship between existential and universal NLL vars. Doing this integration today is quite difficult today.
`tests/ui/async-await/higher-ranked-auto-trait-11.rs` fails because we don't compute the full transitive outlives relations between placeholders. In that test, we have in our region assumptions that some `'!1 = '!2` and `'!2 = '!3`, but we must prove `'!1 = '!3`.
This can be fixed by computing the set of coroutine outlives assumptions in a more transitive way, or as I mentioned above, integrating these assumptions into the type outlives machinery in a more first-class way, since it's already responsible for the transitive outlives assumptions of universals.
### Moving forward
I'm still quite happy with this implementation, and I'd like to land it for testing. I may work on overhauling both the way we compute these coroutine assumptions and also how we deal with the assumptions during (lexical/nll) region checking. But for now, I'd like to give users a chance to try out this new `-Zhigher-ranked-assumptions` flag to uncover more shortcomings.
[^exist]: Instantiating this binder with infer regions would be incomplete, since we'd be asking for *some* instantiation of the interior types, not proving something for *all* instantiations of the interior types.
Dont collect assoc ty item bounds from trait where clause for host effect predicates
For background, we uplift `where Self::Assoc: Trait` bounds in a trait's where clauses into *item bounds* on `type Assoc;`. This is because before we *had* syntactical item bounds, users would express their item bounds like so.
Let's opt out of doing this same behavior for `HostEffect` predicates like `where Self::Assoc: [const] Trait`. I left a comment in the code:
```rust
// FIXME(const_trait_impl): We *could* uplift the
// `where Self::Assoc: [const] Trait` bounds from the parent trait
// here too, but we'd need to split `const_conditions` into two
// queries (like we do for `trait_explicit_predicates_and_bounds`)
// since we need to also filter the predicates *out* of the const
// conditions or they lead to cycles in the trait solver when
// utilizing these bounds. For now, let's do nothing.
```
As an aside, this was an ICE that was only triggerable when building libraries and not binaries because we never were calling `tcx.ensure_ok().explicit_implied_const_bounds(def_id);` on associated types like we should have been. I adjusted the calls to `ensure_ok` to make sure this happens, so we catch bugs like this in the future more easily.
As another aside, I fixed the bound uplifting logic for *always const* predicates, since those act like normal clauses and have no notion of conditional constness.
r? ```@oli-obk``` ```@fee1-dead``` or anyone really
Fixesrust-lang/rust#133275
Implement unstable trait impl
This PR allows marking impls of stable trait with stable type as unstable.
## Approach
In std/core, an impl can be marked as unstable by annotating it with ``#[unstable_feature_bound(feat_name)]``. This will add a ``ClauseKind::Unstable_Feature(feat_name)`` to the list of predicates in ``predicates_of`` .
When an unstable impl's function is called, we will first iterate through all the goals in ``param_env`` to check if there is any ``ClauseKind::UnstableFeature(feat_name)`` in ``param_env``.
The existence of ``ClauseKind::Unstable_Feature(feat_name)`` in ``param_env`` means an``#[unstable_feature_bound(feat_name)]`` is present at the call site of the function, so we allow the check to succeed in this case.
If ``ClauseKind::UnstableFeature(feat_name)`` does not exist in ``param_env``, we will still allow the check to succeed for either of the cases below:
1. The feature is enabled through ``#[feature(feat_name)]`` outside of std / core.
2. We are in codegen because we may be monomorphizing a body from an upstream crate which had an unstable feature enabled that the downstream crate do not.
For the rest of the case, it will fail with ambiguity.
## Limitation
In this PR, we do not support:
1. using items that need ``#[unstable_feature_bound]`` within stable APIs
2. annotate main function with ``#[unstable_feature_bound]``
3. annotate ``#[unstable_feature_bound]`` on items other than free function and impl
## Acknowledgement
The design and mentoring are done by `@BoxyUwU`
de-duplicate condition scoping logic between AST→HIR lowering and `ScopeTree` construction
There was some overlap between `rustc_ast_lowering::LoweringContext::lower_cond` and `rustc_hir_analysis::check::region::resolve_expr`, so I've removed the former and migrated its logic to the latter, with some simplifications.
Consequences:
- For `while` and `if` expressions' `let`-chains, this changes the `HirId`s for the `&&`s to properly correspond to their AST nodes. This is how guards were handled already.
- This makes match guards share previously-duplicated logic with `if`/`while` expressions. This will also be used by guard pattern[^1] guards.
- Aside from legacy syntax extensions (e.g. some builtin macros) that directly feed AST to the compiler, it's currently impossible to put attributes directly on `&&` operators in `let` chains[^2]. Nonetheless, attributes on `&&` operators in `let` chains in `if`/`while` expression conditions are no longer silently ignored and will be lowered.
- This no longer wraps conditions in `DropTemps`, so the HIR and THIR will be slightly smaller.
- `DesugaringKind::CondTemporary` is now gone. It's no longer applied to any spans, and all uses of it were dead since they were made to account for `if` and `while` being desugared to `match` on a boolean scrutinee.
- Should be a marginal perf improvement beyond that due to leveraging [`ScopeTree` construction](5e749eb66f/compiler/rustc_hir_analysis/src/check/region.rs (L312-L355))'s clever handling of `&&` and `||`:
- This removes some unnecessary terminating scopes that were placed around top-level `&&` and `||` operators in conditions. When lowered to MIR, logical operator chains don't create intermediate boolean temporaries, so there's no temporary to drop. The linked snippet handles wrapping the operands in terminating scopes as necessary, in case they create temporaries.
- The linked snippet takes care of letting `let` temporaries live and terminating other operands, so we don't need separate traversals of `&&` chains for that.
[^1]: rust-lang/rust#129967
[^2]: Case-by-case, here's my justification: `#[attr] e1 && e2` applies the attribute to `e1`. In `#[attr] (e1 && e2)` , the attribute is on the parentheses in the AST, plus it'd fail to parse if `e1` or `e2` contains a `let`. In `#[attr] expands_to_let_chain!()`, the attribute would already be ignored (rust-lang/rust#63221) and it'd fail to parse anyway; even if the expansion site is a condition, the expansion wouldn't be parsed with `Restrictions::ALLOW_LET`. If it *was* allowed, the notion of a "reparse context" from https://github.com/rust-lang/rust/issues/61733#issuecomment-509626449 would be necessary in order to make `let`-chains left-associative; multiple places in the compiler assume they are.
Port several trait/coherence-related attributes the new attribute system
Part of rust-lang/rust#131229
This ports:
- `#[const_trait]`
- `#[rustc_deny_explicit_impl]`
- `#[rustc_do_not_implement_via_object]`
- `#[rustc_coinductive]`
- `#[type_const]`
- `#[rustc_specialization_trait]`
- `#[rustc_unsafe_specialization_marker]`
- `#[marker]`
- `#[fundamental]`
- `#[rustc_paren_sugar]`
- `#[rustc_allow_incoherent_impl]`
- `#[rustc_coherence_is_core]`
This also changes `#[marker]` to error on duplicates instead of warning.
cc rust-lang/rust#142838, but I don't think it matters too much, since it's unstable.
r? ``@oli-obk``
