safely `transmute<&List<Ty<'tcx>>, &List<GenericArg<'tcx>>>`
This PR has 3 relevant steps which are is split in distinct commits.
The first commit now interns `List<Ty<'tcx>>` and `List<GenericArg<'tcx>>` together, potentially reusing memory while allowing free conversions between these two using `List<Ty<'tcx>>::as_substs()` and `SubstsRef<'tcx>::try_as_type_list()`.
Using this, we then use `&'tcx List<Ty<'tcx>>` instead of a `SubstsRef<'tcx>` for tuple fields, simplifying a bunch of code.
Finally, as tuple fields and other generic arguments now use a different `TypeFoldable<'tcx>` impl, we optimize the impl for `List<Ty<'tcx>>` improving perf by slightly less than 1% in tuple heavy benchmarks.
Normalize obligation and expected trait_refs in confirm_poly_trait_refs
Consolidate normalization the obligation and expected trait refs in `confirm_poly_trait_refs`. Also, _always_ normalize these trait refs -- we were already normalizing the obligation trait ref when confirming closure and generator candidates, but this does it for fn pointer confirmation as well.
This presumably does more work in the case that the obligation's trait ref is already normalized, but we can see from the perf runs in #94070, it actually (paradoxically, perhaps) improves performance when paired with logic that normalizes projections in fulfillment loop.
Move ty::print methods to Drop-based scope guards
Primary goal is reducing codegen of the TLS access for each closure, which shaves ~3 seconds of bootstrap time over rustc as a whole.
Only mark projection as ambiguous if GAT substs are constrained
A slightly more targeted version of #92917, where we only give up with ambiguity if we infer something about the GATs substs when probing for a projection candidate.
fixes#93874
also note (but like the previous PR, does not fix) #91762
r? `@jackh726`
cc `@nikomatsakis` who reviewed #92917
Do not ICE when inlining a function with un-satisfiable bounds
Fixes#93008
This is kinda a hack... but it's the fix I thought had the least blast-radius.
We use `normalize_param_env_or_error` to verify that the predicates in the param env are self-consistent, since with RevealAll, a bad predicate like `<&'static () as Clone>` will be evaluated with an empty ParamEnv (since it references no generics), and we'll raise an error for it.
Suggest copying trait associated type bounds on lifetime error
Closes#92033
Kind of the most simple suggestion to make - we don't try to be fancy. Turns out, it's still pretty useful (the couple existing tests that trigger this error end up fixed - for this error - upon applying the fix).
r? ``@estebank``
cc ``@nikomatsakis``
Correctly mark the span of captured arguments in `format_args!()`
It should not include the braces, or misspelling suggestions will be wrong.
Fixes#94010.
Specifically, rename the `Const` struct as `ConstS` and re-introduce `Const` as
this:
```
pub struct Const<'tcx>(&'tcx Interned<ConstS>);
```
This now matches `Ty` and `Predicate` more closely, including using
pointer-based `eq` and `hash`.
Notable changes:
- `mk_const` now takes a `ConstS`.
- `Const` was copy, despite being 48 bytes. Now `ConstS` is not, so need a
we need separate arena for it, because we can't use the `Dropless` one any
more.
- Many `&'tcx Const<'tcx>`/`&Const<'tcx>` to `Const<'tcx>` changes
- Many `ct.ty` to `ct.ty()` and `ct.val` to `ct.val()` changes.
- Lots of tedious sigil fiddling.
The variant names are exported, so we can use them directly (possibly
with a `ty::` qualifier). Lots of places already do this, this commit
just increases consistency.
Specifically, change `Region` from this:
```
pub type Region<'tcx> = &'tcx RegionKind;
```
to this:
```
pub struct Region<'tcx>(&'tcx Interned<RegionKind>);
```
This now matches `Ty` and `Predicate` more closely.
Things to note
- Regions have always been interned, but we haven't been using pointer-based
`Eq` and `Hash`. This is now happening.
- I chose to impl `Deref` for `Region` because it makes pattern matching a lot
nicer, and `Region` can be viewed as just a smart wrapper for `RegionKind`.
- Various methods are moved from `RegionKind` to `Region`.
- There is a lot of tedious sigil changes.
- A couple of types like `HighlightBuilder`, `RegionHighlightMode` now have a
`'tcx` lifetime because they hold a `Ty<'tcx>`, so they can call `mk_region`.
- A couple of test outputs change slightly, I'm not sure why, but the new
outputs are a little better.
Specifically, change `Ty` from this:
```
pub struct Predicate<'tcx> { inner: &'tcx PredicateInner<'tcx> }
```
to this:
```
pub struct Predicate<'tcx>(&'tcx Interned<PredicateS<'tcx>>)
```
where `PredicateInner` is renamed as `PredicateS`.
This (plus a few other minor changes) makes the parallels with `Ty` and
`TyS` much clearer, and makes the uniqueness more explicit.
Specifically, change `Ty` from this:
```
pub type Ty<'tcx> = &'tcx TyS<'tcx>;
```
to this
```
pub struct Ty<'tcx>(Interned<'tcx, TyS<'tcx>>);
```
There are two benefits to this.
- It's now a first class type, so we can define methods on it. This
means we can move a lot of methods away from `TyS`, leaving `TyS` as a
barely-used type, which is appropriate given that it's not meant to
be used directly.
- The uniqueness requirement is now explicit, via the `Interned` type.
E.g. the pointer-based `Eq` and `Hash` comes from `Interned`, rather
than via `TyS`, which wasn't obvious at all.
Much of this commit is boring churn. The interesting changes are in
these files:
- compiler/rustc_middle/src/arena.rs
- compiler/rustc_middle/src/mir/visit.rs
- compiler/rustc_middle/src/ty/context.rs
- compiler/rustc_middle/src/ty/mod.rs
Specifically:
- Most mentions of `TyS` are removed. It's very much a dumb struct now;
`Ty` has all the smarts.
- `TyS` now has `crate` visibility instead of `pub`.
- `TyS::make_for_test` is removed in favour of the static `BOOL_TY`,
which just works better with the new structure.
- The `Eq`/`Ord`/`Hash` impls are removed from `TyS`. `Interned`s impls
of `Eq`/`Hash` now suffice. `Ord` is now partly on `Interned`
(pointer-based, for the `Equal` case) and partly on `TyS`
(contents-based, for the other cases).
- There are many tedious sigil adjustments, i.e. adding or removing `*`
or `&`. They seem to be unavoidable.
This is required to avoid creating large numbers of universes from each
Chalk query, while still having enough universe information for lifetime
errors.
Make all `hir::Map` methods consistently by-value
`hir::Map` only consists of a single reference (as part of the contained `TyCtxt`) anyways, so copying is literally zero overhead compared to passing a reference
Ensure that queries only return Copy types.
This should pervent the perf footgun of returning a result with an expensive `Clone` impl (like a `Vec` of a hash map).
I went for the stupid solution of allocating on an arena everything that was not `Copy`. Some query results could be made Copy easily, but I did not really investigate.
Improve opaque type higher-ranked region error message under NLL
Currently, any higher-ranked region errors involving opaque types
fall back to a generic "higher-ranked subtype error" message when
run under NLL. This PR adds better error message handling for this
case, giving us the same kinds of error messages that we currently
get without NLL:
```
error: implementation of `MyTrait` is not general enough
--> $DIR/opaque-hrtb.rs:12:13
|
LL | fn foo() -> impl for<'a> MyTrait<&'a str> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `MyTrait` is not general enough
|
= note: `impl MyTrait<&'2 str>` must implement `MyTrait<&'1 str>`, for any lifetime `'1`...
= note: ...but it actually implements `MyTrait<&'2 str>`, for some specific lifetime `'2`
error: aborting due to previous error
```
To accomplish this, several different refactoring needed to be made:
* We now have a dedicated `InstantiateOpaqueType` struct which
implements `TypeOp`. This is used to invoke `instantiate_opaque_types`
during MIR type checking.
* `TypeOp` is refactored to pass around a `MirBorrowckCtxt`, which is
needed to report opaque type region errors.
* We no longer assume that all `TypeOp`s correspond to canonicalized
queries. This allows us to properly handle opaque type instantiation
(which does not occur in a query) as a `TypeOp`.
A new `ErrorInfo` associated type is used to determine what
additional information is used during higher-ranked region error
handling.
* The body of `try_extract_error_from_fulfill_cx`
has been moved out to a new function `try_extract_error_from_region_constraints`.
This allows us to re-use the same error reporting code between
canonicalized queries (which can extract region constraints directly
from a fresh `InferCtxt`) and opaque type handling (which needs to take
region constraints from the pre-existing `InferCtxt` that we use
throughout MIR borrow checking).
Currently, any higher-ranked region errors involving opaque types
fall back to a generic "higher-ranked subtype error" message when
run under NLL. This PR adds better error message handling for this
case, giving us the same kinds of error messages that we currently
get without NLL:
```
error: implementation of `MyTrait` is not general enough
--> $DIR/opaque-hrtb.rs:12:13
|
LL | fn foo() -> impl for<'a> MyTrait<&'a str> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `MyTrait` is not general enough
|
= note: `impl MyTrait<&'2 str>` must implement `MyTrait<&'1 str>`, for any lifetime `'1`...
= note: ...but it actually implements `MyTrait<&'2 str>`, for some specific lifetime `'2`
error: aborting due to previous error
```
To accomplish this, several different refactoring needed to be made:
* We now have a dedicated `InstantiateOpaqueType` struct which
implements `TypeOp`. This is used to invoke `instantiate_opaque_types`
during MIR type checking.
* `TypeOp` is refactored to pass around a `MirBorrowckCtxt`, which is
needed to report opaque type region errors.
* We no longer assume that all `TypeOp`s correspond to canonicalized
queries. This allows us to properly handle opaque type instantiation
(which does not occur in a query) as a `TypeOp`.
A new `ErrorInfo` associated type is used to determine what
additional information is used during higher-ranked region error
handling.
* The body of `try_extract_error_from_fulfill_cx`
has been moved out to a new function `try_extract_error_from_region_constraints`.
This allows us to re-use the same error reporting code between
canonicalized queries (which can extract region constraints directly
from a fresh `InferCtxt`) and opaque type handling (which needs to take
region constraints from the pre-existing `InferCtxt` that we use
throughout MIR borrow checking).
Don't constrain projection predicates with inference vars in GAT substs
cc #91762
Not a fix, but a mitigation to prevent a backwards-compatible hazard where we normalize using a predicate only because it's the only one available, but shouldn't. This would constrain an inference variable which didn't really want. We already do this when selecting a projection candidate, which isn't always correct. But changing that is a problem for a different day.
Also found out that a suggestion for `await`ing a future was using the wrong substs.
r? ``@nikomatsakis``
Add in ValuePair::Term
This adds in an enum when matching on positions which can either be types or consts.
It will default to emitting old special cased error messages for types.
r? `@oli-obk`
cc `@matthiaskrgr`
Fixes#93578
by using an opaque type obligation to bubble up comparisons between opaque types and other types
Also uses proper obligation causes so that the body id works, because out of some reason nll uses body ids for logic instead of just diagnostics.
Continue work on associated const equality
This actually implements some more complex logic for assigning associated consts to values.
Inside of projection candidates, it now defers to a separate function for either consts or
types. To reduce amount of code, projections are now generic over T, where T is either a Type or
a Const. I can add some comments back later, but this was the fastest way to implement it.
It also now finds the correct type of consts in type_of.
---
The current main TODO is finding the const of the def id for the LeafDef.
Right now it works if the function isn't called, but once you use the trait impl with the bound it fails inside projection.
I was hoping to get some help in getting the `&'tcx ty::Const<'tcx>`, in addition to a bunch of other `todo!()`s which I think may not be hit.
r? `@oli-obk`
Updates #92827
Add note suggesting that predicate may be satisfied, but is not `const`
Not sure if we should be printing this in addition to, or perhaps _instead_ of the help message:
```
help: the trait `~const Add` is not implemented for `NonConstAdd`
```
Also added `ParamEnv::is_const` and `PolyTraitPredicate::is_const_if_const` and, in a separate commit, used those in other places instead of `== hir::Constness::Const`, etc.
r? ````@fee1-dead````
Only traverse attrs once while checking for coherence override attributes
In coherence, while checking for negative impls override attributes: only traverse the `DefId`s' attributes once.
This PR is an easy way to get back some of the small perf loss in #93175
Store a `Symbol` instead of an `Ident` in `AssocItem`
This is the same idea as #92533, but for `AssocItem` instead
of `VariantDef`/`FieldDef`.
With this change, we no longer have any uses of
`#[stable_hasher(project(...))]`
Properly track `DepNode`s in trait evaluation provisional cache
Fixes#92987
During evaluation of an auto trait predicate, we may encounter a cycle.
This causes us to store the evaluation result in a special 'provisional
cache;. If we later end up determining that the type can legitimately
implement the auto trait despite the cycle, we remove the entry from
the provisional cache, and insert it into the evaluation cache.
Additionally, trait evaluation creates a special anonymous `DepNode`.
All queries invoked during the predicate evaluation are added as
outoging dependency edges from the `DepNode`. This `DepNode` is then
store in the evaluation cache - if a different query ends up reading
from the cache entry, it will also perform a read of the stored
`DepNode`. As a result, the cached evaluation will still end up
(transitively) incurring all of the same dependencies that it would
if it actually performed the uncached evaluation (e.g. a call to
`type_of` to determine constituent types).
Previously, we did not correctly handle the interaction between the
provisional cache and the created `DepNode`. Storing an evaluation
result in the provisional cache would cause us to lose the `DepNode`
created during the evaluation. If we later moved the entry from the
provisional cache to the evaluation cache, we would use the `DepNode`
associated with the evaluation that caused us to 'complete' the cycle,
not the evaluatoon where we first discovered the cycle. As a result,
future reads from the evaluation cache would miss some incremental
compilation dependencies that would have otherwise been added if the
evaluation was *not* cached.
Under the right circumstances, this could lead to us trying to force
a query with a no-longer-existing `DefPathHash`, since we were missing
the (red) dependency edge that would have caused us to bail out before
attempting forcing.
This commit makes the provisional cache store the `DepNode` create
during the provisional evaluation. When we move an entry from the
provisional cache to the evaluation cache, we create a *new* `DepNode`
that has dependencies going to *both* of the evaluation `DepNodes` we
have available. This ensures that cached reads will incur all of
the necessary dependency edges.
Check `const Drop` impls considering `~const` Bounds
This PR adds logic to trait selection to account for `~const` bounds in custom `impl const Drop` for types, elaborates the `const Drop` check in `rustc_const_eval` to check those bounds, and steals some drop linting fixes from #92922, thanks `@DrMeepster.`
r? `@fee1-dead` `@oli-obk` <sup>(edit: guess I can't request review from two people, lol)</sup>
since each of you wrote and reviewed #88558, respectively.
Since the logic here is more complicated than what existed, it's possible that this is a perf regression. But it works correctly with tests, and that makes me happy.
Fixes#92881
