| Age | Commit message (Collapse) | Author | Lines |
|---|
|
Only disable cache if predicate has opaques within it
This is an alternative to https://github.com/rust-lang/rust/pull/132075.
This refines the check implemented in https://github.com/rust-lang/rust/pull/126024 to only disable the global cache if the predicate being considered has opaques in it. This is still theoretically unsound, since goals can indirectly rely on opaques in the defining scope, but we're much less likely to hit it.
It doesn't totally fix https://github.com/rust-lang/rust/issues/132064: for example, `lemmy` goes from 1:29 (on rust 1.81) to 9:53 (on nightly) to 4:07 (after this PR). But I think it's at least *more* sound than a total revert :/
r? lcnr
|
|
|
|
Remove unnecessary pub enum glob-imports from `rustc_middle::ty`
We used to have an idiom in the compiler where we'd prefix or suffix all the variants of an enum, for example `BoundRegionKind`, with something like `Br`, and then *glob-import* that enum variant directly.
`@noratrieb` brought this up, and I think that it's easier to read when we just use the normal style `EnumName::Variant`.
This PR is a bit large, but it's just naming.
The only somewhat opinionated change that this PR does is rename `BorrowKind::Imm` to `BorrowKind::Immutable` and same for the other variants. I think these enums are used sparingly enough that the extra length is fine.
r? `@noratrieb` or reassign
|
|
document `type_implements_trait`
Rendered:
r? `@compiler-errors`
|
|
|
|
Suggest creating unary tuples when types don't match a trait
When you want to have a variadic function, a common workaround to implement this is to create a trait and then implement that trait for various tuples. For example in `pyo3` there exists
```rust
/// Calls the object with only positional arguments.
pub fn call1(&self, args: impl IntoPy<Py<PyTuple>>) -> PyResult<&PyAny> {
...
}
```
with various impls like
```rust
impl<A: IntoPy<PyObject> IntoPy<Py<PyAny>> for (A,)
impl<A: IntoPy<PyObject, B: IntoPy<PyObject> IntoPy<Py<PyAny>> for (A, B)
... etc
```
This means that if you want to call the method with a single item you have to create a unary tuple, like `(x,)`, rather than just `x`.
This PR implements a suggestion to do that, if applicable.
|
|
No need to instantiate binder in `confirm_async_closure_candidate`
Removes a FIXME that is redundant. No longer needed since #122267.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
Do not suggest `#[derive(Copy)]` when we wanted a `!Copy` type.
Do not say "`Copy` is not implemented for `T` but `Copy` is".
Do not talk about `Trait` having no implementations when `!Trait` was desired.
|
|
Long span labels don't read well.
|
|
```
error[E0277]: the trait bound `{gen block@$DIR/gen_block_is_coro.rs:7:5: 7:8}: Coroutine` is not satisfied
--> $DIR/gen_block_is_coro.rs:6:13
|
LL | fn foo() -> impl Coroutine<Yield = u32, Return = ()> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Coroutine` is not implemented for `{gen block@$DIR/gen_block_is_coro.rs:7:5: 7:8}`
LL | gen { yield 42 }
| ---------------- return type was inferred to be `{gen block@$DIR/gen_block_is_coro.rs:7:5: 7:8}` here
```
The secondary span label is new.
|
|
|
|
Remove default note for "trait is not implemented" in favor of the
more colorful diff output from the previous commit. Removes
duplicated output.
|
|
When a trait is not implemented for a type, but there *is* an `impl`
for another type or different trait params, we format the output to
use highlighting in the same way that E0308 does for types.
The logic accounts for 3 cases:
- When both the type and trait in the expected predicate and the candidate are different
- When only the types are different
- When only the trait generic params are different
For each case, we use slightly different formatting and wording.
|
|
continue `TypingMode` refactor
There are still quite a few places which (indirectly) rely on the `Reveal` of a `ParamEnv`, but we're slowly getting there
r? `@compiler-errors`
|
|
|
|
|
|
Remove region from adjustments
It's not necessary to store this region, because it's only used in THIR and MemCat/ExprUse, both of which already basically only deal with erased regions anyways.
|
|
Try to point out when edition 2024 lifetime capture rules cause borrowck issues
Lifetime capture rules in 2024 are modified to capture more lifetimes, which sometimes lead to some non-local borrowck errors. This PR attempts to link these back together with a useful note pointing out the capture rule changes.
This is not a blocking concern, but I'd appreciate feedback (though, again, I'd like to stress that I don't want to block this PR on this): I'm worried about this note drowning in the sea of other diagnostics that borrowck emits. I was tempted to change the level of the note to `.span_warn` just so it would show up in a different color. Thoughts?
Fixes #130545
Opening as a draft first since it's stacked on #131183.
r? `@ghost`
|
|
|
|
Rollup of 3 pull requests
Successful merges:
- #132368 (Remove `do_not_const_check` from `Iterator` methods)
- #132373 (Make sure `type_param_predicates` resolves correctly for RPITIT)
- #132374 (Remove dead code stemming from the old effects desugaring)
r? `@ghost`
`@rustbot` modify labels: rollup
|
|
|
|
Merge `HostPolarity` and `BoundConstness`
They're basically the same thing, and I think `BoundConstness` is easier to use.
r? fee1-dead or reassign
|
|
Rename `rustc_abi::Abi` to `BackendRepr`
Remove the confabulation of `rustc_abi::Abi` with what "ABI" actually means by renaming it to `BackendRepr`, and rename `Abi::Aggregate` to `BackendRepr::Memory`. The type never actually represented how things are passed, as that has to have `PassMode` considered, at minimum, but rather it just is how we represented some things to the backend. This conflation arose because LLVM, the primary backend at the time, would lower certain IR forms using certain ABIs. Even that only somewhat was true, as it broke down when one ventured significantly afield of what is described by the System V AMD64 ABI either by using different architectures, ABI-modifying IR annotations, the same architecture **with different ISA extensions enabled**, or other... unexpected delights.
Unfortunately both names are still somewhat of a misnomer right now, as people have written code for years based on this misunderstanding. Still, their original names are even moreso, and for better or worse, this backend code hasn't received as much maintenance as the rest of the compiler, lately. Actually arriving at a correct end-state will simply require us to disentangle a lot of code in order to fix, much of it pointlessly repeated in several places. Thus this is not an "actual fix", just a way to deflect further misunderstandings.
|
|
|
|
|
|
TypingMode: merge intercrate, reveal, and defining_opaque_types
This adds `TypingMode` and uses it in most places. We do not yet remove `Reveal` from `param_env`s. This and other future work as tracked in #132279 and via `FIXME`s.
Fetching the `TypingMode` of the `InferCtxt` asserts that the `TypingMode` agrees with `ParamEnv::reveal` to make sure we don't introduce any subtle bugs here. This will be unnecessary once `ParamEnv::reveal` no longer exists.
As the `TypingMode` is now a part of the query input, I've merged the coherence and non-coherence caches for the new solver. I've also enabled the local `infcx` cache during coherence by clearing the cache when forking it with a different `TypingMode`.
#### `TypingMode::from_param_env`
I am using this even in cases where I know that the `param_env` will always be `Reveal::UserFacing`. This is to make it easier to correctly refactor this code in the future, any time we use `Reveal::UserFacing` in a body while not defining its opaque types is incorrect and should use a `TypingMode` which only reveals opaques defined by that body instead, cc #124598
r? ``@compiler-errors``
|
|
The initial naming of "Abi" was an awful mistake, conveying wrong ideas
about how psABIs worked and even more about what the enum meant.
It was only meant to represent the way the value would be described to
a codegen backend as it was lowered to that intermediate representation.
It was never meant to mean anything about the actual psABI handling!
The conflation is because LLVM typically will associate a certain form
with a certain ABI, but even that does not hold when the special cases
that actually exist arise, plus the IR annotations that modify the ABI.
Reframe `rustc_abi::Abi` as the `BackendRepr` of the type, and rename
`BackendRepr::Aggregate` as `BackendRepr::Memory`. Unfortunately, due to
the persistent misunderstandings, this too is now incorrect:
- Scattered ABI-relevant code is entangled with BackendRepr
- We do not always pre-compute a correct BackendRepr that reflects how
we "actually" want this value to be handled, so we leave the backend
interface to also inject various special-cases here
- In some cases `BackendRepr::Memory` is a "real" aggregate, but in
others it is in fact using memory, and in some cases it is a scalar!
Our rustc-to-backend lowering code handles this sort of thing right now.
That will eventually be addressed by lifting duplicated lowering code
to either rustc_codegen_ssa or rustc_target as appropriate.
|
|
Remove the "which is required by `{root_obligation}`" post-script in
"the trait `X` is not implemented for `Y`" explanation in E0277. This
information is already conveyed in the notes explaining requirements,
making it redundant while making the text (particularly in labels)
harder to read.
```
error[E0277]: the trait bound `NotCopy: Copy` is not satisfied
--> $DIR/wf-static-type.rs:10:13
|
LL | static FOO: IsCopy<Option<NotCopy>> = IsCopy { t: None };
| ^^^^^^^^^^^^^^^^^^^^^^^ the trait `Copy` is not implemented for `NotCopy`
|
= note: required for `Option<NotCopy>` to implement `Copy`
note: required by a bound in `IsCopy`
--> $DIR/wf-static-type.rs:7:17
|
LL | struct IsCopy<T:Copy> { t: T }
| ^^^^ required by this bound in `IsCopy`
```
vs the prior
```
error[E0277]: the trait bound `NotCopy: Copy` is not satisfied
--> $DIR/wf-static-type.rs:10:13
|
LL | static FOO: IsCopy<Option<NotCopy>> = IsCopy { t: None };
| ^^^^^^^^^^^^^^^^^^^^^^^ the trait `Copy` is not implemented for `NotCopy`, which is required by `Option<NotCopy>: Copy`
|
= note: required for `Option<NotCopy>` to implement `Copy`
note: required by a bound in `IsCopy`
--> $DIR/wf-static-type.rs:7:17
|
LL | struct IsCopy<T:Copy> { t: T }
| ^^^^ required by this bound in `IsCopy`
```
|
|
|
|
|
|
|
|
|
|
Remove `ObligationCause::span()` method
I think it's an incredibly confusing footgun to expose both `obligation_cause.span` and `obligation_cause.span()`. Especially because `ObligationCause::span()` (the method) seems to just be hacking around a single quirk in the way we set up obligation causes for match arms.
First commit removes the need for that hack, with only one diagnostic span changing (but IMO not really getting worse -- I'd argue that it was already confusing).
|
|
Tweak E0277 highlighting and "long type" path printing
Partially address #132013.
|
|
|
|
|
|
Then we can rename the _raw functions to drop their suffix, and instead
explicitly use is_stable_const_fn for the few cases where that is really what
you want.
|
|
path multiple times
Because `note_obligation_cause_code` is recursive, if multiple types are too
long to print to the terminal, a `long_ty_file` will be created. Before, one was
created *per recursion*. Now, it is passed in so it gets printed only once.
Part of #132013.
|
|
When printing
```
= help: the trait `chumsky::private::ParserSealed<'_, &'a str, ((), ()), chumsky::extra::Full<EmptyErr, (), ()>>` is implemented for `Then<Ignored<chumsky::combinator::Filter<chumsky::primitive::Any<&str, chumsky::extra::Full<EmptyErr, (), ()>>, {closure@src/main.rs:9:17: 9:27}>, char>, chumsky::combinator::Map<impl CSTParser<'a, O>, O, {closure@src/main.rs:11:24: 11:27}>, (), (), chumsky::extra::Full<EmptyErr, (), ()>>`
= help: for that trait implementation, expected `((), ())`, found `()`
```
Highlight only the `expected` and `found` types, instead of the full type in the first `help`.
|
|
Represent trait constness as a distinct predicate
cc `@rust-lang/project-const-traits`
r? `@ghost` for now
Also mirrored everything that is written below on this hackmd here: https://hackmd.io/`@compiler-errors/r12zoixg1l`
# Tl;dr:
* This PR removes the bulk of the old effect desugaring.
* This PR reimplements most of the effect desugaring as a new predicate and set of a couple queries. I believe it majorly simplifies the implementation and allows us to move forward more easily on its implementation.
I'm putting this up both as a request for comments and a vibe-check, but also as a legitimate implementation that I'd like to see land (though no rush of course on that last part).
## Background
### Early days
Once upon a time, we represented trait constness in the param-env and in `TraitPredicate`. This was very difficult to implement correctly; it had bugs and was also incomplete; I don't think this was anyone's fault though, it was just the limit of experimental knowledge we had at that point.
Dealing with `~const` within predicates themselves meant dealing with constness all throughout the trait solver. This was difficult to keep track of, and afaict was not handled well with all the corners of candidate assembly.
Specifically, we had to (in various places) remap constness according to the param-env constness:
https://github.com/rust-lang/rust/blob/574b64a97f52162f965bc201e47f0af8279ca65d/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1498
This was annoying and manual and also error prone.
### Beginning of the effects desugaring
Later on, #113210 reimplemented a new desugaring for const traits via a `<const HOST: bool>` predicate. This essentially "reified" the const checking and separated it from any of the remapping or separate tracking in param-envs. For example, if I was in a const-if-const environment, but I wanted to call a trait that was non-const, this reification would turn the constness mismatch into a simple *type* mismatch of the effect parameter.
While this was a monumental step towards straightening out const trait checking in the trait system, it had its own issues, since that meant that the constness of a trait (or any item within it, like an associated type) was *early-bound*. This essentially meant that `<T as Trait>::Assoc` was *distinct* from `<T as ~const Trait>::Assoc`, which was bad.
### Associated-type bound based effects desugaring
After this, #120639 implemented a new effects desugaring. This used an associated type to more clearly represent the fact that the constness is not an input parameter of a trait, but a property that could be computed of a impl. The write-up linked in that PR explains it better than I could.
However, I feel like it really reached the limits of what can comfortably be expressed in terms of associated type and trait calculus. Also, `<const HOST: bool>` remains a synthetic const parameter, which is observable in nested items like RPITs and closures, and comes with tons of its own hacks in the astconv and middle layer.
For example, there are pieces of unintuitive code that are needed to represent semantics like elaboration, and eventually will be needed to make error reporting intuitive, and hopefully in the future assist us in implementing built-in traits (eventually we'll want something like `~const Fn` trait bounds!).
elaboration hack: https://github.com/rust-lang/rust/blob/8069f8d17a6c86a8fd881939fcce359a90c57ff2/compiler/rustc_type_ir/src/elaborate.rs#L133-L195
trait bound remapping hack for diagnostics: https://github.com/rust-lang/rust/blob/8069f8d17a6c86a8fd881939fcce359a90c57ff2/compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs#L2370-L2413
I want to be clear that I don't think this is a issue of implementation quality or anything like that; I think it's simply a very clear sign that we're using types and traits in a way that they're not fundamentally supposed to be used, especially given that constness deserves to be represented as a first-class concept.
### What now?
This PR implements a new desugaring for const traits. Specifically, it introduces a `HostEffect` predicate to represent the obligation an impl is const, rather than using associated type bounds and the compat trait that exists for effects today.
### `HostEffect` predicate
A `HostEffect` clause has two parts -- the `TraitRef` we're trying to prove, and a `HostPolarity::{Maybe, Const}`.
`HostPolarity::Const` corresponds to `T: const Trait` bounds, which must *always* be proven as const, and which can be written in any context. These are lowered directly into the predicates of an item, since they're not "context-specific".
On the other hand, `HostPolarity::Maybe` corresponds to `T: ~const Trait` bounds which must only exist in a conditionally-const context like a method in a `#[const_trait]`, or a `const fn` free function. We do not lower these immediately into the predicates of an item; instead, we collect them into a new query called the **`const_conditions`**. These are the set of trait refs that we need to prove have const implementations for an item to be const.
Notably, they're represented as bare (poly) trait refs because they are meant to be paired back together with a `HostPolarity` when they're being registered in typeck (see next section).
For example, given:
```rust
const fn foo<T: ~const A + const B>() {}
```
`foo`'s const conditions would contain `T: A`, but not `T: B`. On the flip side, foo's predicates (`predicates_of`) query would contain `HostEffect(T: B, HostPolarity::Const)` but not `HostEffect(T: A, HostPolarity::Maybe)` since we don't need to prove that predicate in a non-const environment (and it's not even the right predicate to prove in an unconditionally const environment).
### Type checking const bodies
When type checking bodies in HIR, when we encounter a call expression, we additionally register the callee item's const conditions with the `HostPolarity` from the body we're typechecking (`Const` for unconditionally const things like `const`/`static` items, and `Maybe` for conditionally const things like const fns; and we don't register `HostPolarity` predicates for non-const bodies).
When type-checking a conditionally const body, we augment its param-env with `HostEffect(..., Maybe)` predicates.
### Checking that const impls are WF
We extend the logic in `compare_method_predicate_entailment` to also check the const-conditions of the impl method, to make sure that we error for:
```rust
#[const_trait] Bar {}
#[const_trait] trait Foo {
fn method<T: Bar>();
}
impl Foo for () {
fn method<T: ~const Bar>() {} // stronger assumption!
}
```
We also extend the WF check for impls to register the const conditions of the trait that is being implemented. This is to make sure we error for:
```rust
#[const_trait] trait Bar {}
#[const_trait] trait Foo<T> where T: ~const Bar {}
impl<T> const Foo<T> for () {}
//~^ `T: ~const Bar` is missing!
```
### Proving a `HostEffect` predicate
We have several ways of proving a `HostEffect` predicate:
1. Matching a `HostEffect` predicate from the param-env
2. From an impl - we do impl selection very similar to confirming a trait goal, except we filter for only const impls, and we additionally register the impl's const conditions (i.e. the impl's `~const` where clauses).
Later I expect that we will add more built-in implementations for things like `Fn`.
## What next?
After this PR, I'd like to split out the work more so it can proceed in parallel and probably amongst others that are not me.
* Register `HostEffect` goal for places in HIR typeck that correspond to call terminators, like autoderef.
* Make traits in libstd const again.
* Probably need to impl host effect preds in old solver.
* Implement built-in `HostEffect` rules for traits like `Fn`.
* Rip out const checking from MIR altogether.
## So what?
This ends up being super convenient basically everywhere in the compiler. Due to the design of the new trait solver, we end up having an almost parallel structure to the existing trait and projection predicates for assembling `HostEffect` predicates; adding new candidates and especially new built-in implementations is now basically trivial, and it's quite straightforward to understand the confirmation logic for these predicates.
Same with diagnostics reporting; since we have predicates which represent the obligation to prove an impl is const, we can simplify and make these diagnostics richer without having to write a ton of logic to intercept and rewrite the existing `Compat` trait errors.
Finally, it gives us a much more straightforward path for supporting the const effect on the old trait solver. I'm personally quite passionate about getting const trait support into the hands of users without having to wait until the new solver lands[^1], so I think after this PR lands we can begin to gauge how difficult it would be to implement constness in the old trait solver too. This PR will not do this yet.
[^1]: Though this is not a prerequisite or by any means the only justification for this PR.
|
