Deconstruct values in the THIR visitor
I continue to add deconstruction for task rust-lang/rust#141849
The changes concern a more complex part of the task `compiler/rustc_hir/src/intravisit.rs`
r? `@nnethercote`
Don't declare variables in `ExprKind::Let` in invalid positions
Handle `let` expressions in invalid positions specially during resolve in order to avoid making destructuring-assignment expressions that reference (invalid) variables that have not yet been delcared yet.
See further explanation in test and comment in the source.
Fixesrust-lang/rust#141844
`UsePath` contains a `SmallVec<[Res; 3]>`. This holds up to three `Res`
results, one per namespace (type, value, or macro). `lower_import_res`
takes a `PerNS<Option<Res<NodeId>>>` result and lowers it into the
`SmallVec`. This is pretty weird. The input `PerNS` makes it clear which
`Res` belongs to which namespace, but the `SmallVec` throws that
information away.
And code that operates on the `SmallVec` tends to use iteration (or even
just grabbing the first entry!) without knowing which namespace the
`Res` belongs to. Even weirder! Also, `SmallVec` is an overly flexible
type to use here, because it can contain any number of elements (even
though it's optimized for 3 in this case).
This commit changes `UsePath` so it also contains a
`PerNS<Option<Res<HirId>>>`. This type preserves more information and is
more self-documenting. The commit also changes a lot of the use sites to
access the result for a particular namespace. E.g. if you're looking up
a trait, it will be in the `Res` for the type namespace if it's present;
it's silly to look in the `Res` for the value namespace or macro
namespace. Overall I find the new code much easier to understand.
However, some use sites still iterate. These now use `present_items`
because that filters out the `None` results.
Also, `redundant_pub_crate.rs` gets a bigger change. A
`UseKind:ListStem` item gets no `Res` results, which means the old `all`
call in `is_not_macro_export` would succeed (because `all` succeeds on
an empty iterator) and the `ListStem` would be ignored. This is what we
want, but was more by luck than design. The new code detects `ListStem`
explicitly. The commit generalizes the name of that function
accordingly.
Finally, the commit also removes the `use_path` arena, because
`PerNS<Option<Res>>` impls `Copy` (unlike `SmallVec`) and it can be
allocated in the arena shared by all `Copy` types.
Specifically `TyAlias`, `Enum`, `Struct`, `Union`. So the fields match
the textual order in the source code.
The interesting part of the change is in
`compiler/rustc_hir/src/hir.rs`. The rest is extremely mechanical
refactoring.
All uses have been removed. And it's nonsensical: an identifier by
definition has at least one char.
The commits adds an is-non-empty assertion to `Ident::new` to enforce
this, and converts some `Ident` constructions to use `Ident::new`.
Adding the assertion requires making `Ident::new` and
`Ident::with_dummy_span` non-const, which is no great loss.
The commit amends a couple of places that do path splitting to ensure no
empty identifiers are created.
Remove global `next_disambiguator` state and handle it with a `DisambiguatorState` type
This removes `Definitions.next_disambiguator` as it doesn't guarantee deterministic def paths when `create_def` is called in parallel. Instead a new `DisambiguatorState` type is passed as a mutable reference to `create_def` to help create unique def paths. `create_def` calls with distinct `DisambiguatorState` instances must ensure that that the def paths are unique without its help.
Anon associated types did rely on this global state for uniqueness and are changed to use (method they're defined in + their position in the method return type) as the `DefPathData` to ensure uniqueness. This also means that the method they're defined in appears in error messages, which is nicer.
`DefPathData::NestedStatic` is added to use for nested data inside statics instead of reusing `DefPathData::AnonConst` to avoid conflicts with those.
cc `@oli-obk`
Simplify `LazyAttrTokenStream`
`LazyAttrTokenStream` is an unpleasant type: `Lrc<Box<dyn ToAttrTokenStream>>`. Why does it look like that?
- There are two `ToAttrTokenStream` impls, one for the lazy case, and one for the case where we already have an `AttrTokenStream`.
- The lazy case (`LazyAttrTokenStreamImpl`) is implemented in `rustc_parse`, but `LazyAttrTokenStream` is defined in `rustc_ast`, which does not depend on `rustc_parse`. The use of the trait lets `rustc_ast` implicitly depend on `rustc_parse`. This explains the `dyn`.
- `LazyAttrTokenStream` must have a `size_of` as small as possible, because it's used in many AST nodes. This explains the `Lrc<Box<_>>`, which keeps it to one word. (It's required `Lrc<dyn _>` would be a fat pointer.)
This PR moves `LazyAttrTokenStreamImpl` (and a few other token stream things) from `rustc_parse` to `rustc_ast`. This lets us replace the `ToAttrTokenStream` trait with a two-variant enum and also remove the `Box`, changing `LazyAttrTokenStream` to `Lrc<LazyAttrTokenStreamInner>`. Plus it does a few cleanups.
r? `@petrochenkov`
This commit does the following.
- Changes it from `Lrc<Box<dyn ToAttrTokenStream>>` to
`Lrc<LazyAttrTokenStreamInner>`.
- Reworks `LazyAttrTokenStreamImpl` as `LazyAttrTokenStreamInner`, which
is a two-variant enum.
- Removes the `ToAttrTokenStream` trait and the two impls of it.
The recursion limit must be increased in some crates otherwise rustdoc
aborts.
Make #![feature(let_chains)] bootstrap conditional in compiler/
Let chains have been stabilized recently in #132833, so we can remove the gating from our uses in the compiler (as the compiler uses edition 2024).
An upcoming lint will want to be able to know if a lifetime is
hidden (e.g. `&u8`, `ContainsLifetime`) or anonymous: (e.g. `&'_ u8`,
`ContainsLifetime<'_>`). It will also want to know if the lifetime is
related to a reference (`&u8`) or a path (`ContainsLifetime`).
I'm removing empty identifiers everywhere, because in practice they
always mean "no identifier" rather than "empty identifier". (An empty
identifier is impossible.) It's better to use `Option` to mean "no
identifier" because you then can't forget about the "no identifier"
possibility.
Some specifics:
- When testing an attribute for a single name, the commit uses the
`has_name` method.
- When testing an attribute for multiple names, the commit uses the new
`has_any_name` method.
- When using `match` on an attribute, the match arms now have `Some` on
them.
In the tests, we now avoid printing empty identifiers by not printing
the identifier in the `error:` line at all, instead letting the carets
point out the problem.
It's a much better name, more consistent with how we name such things.
Also rename `Lifetime::res` as `Lifetime::kind` to match. I suspect this
field used to have the type `LifetimeRes` and then the type was changed
but the field name remained the same.
Enable contracts for const functions
Use `const_eval_select!()` macro to enable contract checking only at runtime. The existing contract logic relies on closures, which are not supported in constant functions.
This commit also removes one level of indirection for ensures clauses since we no longer build a closure around the ensures predicate.
Resolves#136925
**Call-out:** This is still a draft PR since CI is broken due to a new warning message for unreachable code when the bottom of the function is indeed unreachable. It's not clear to me why the warning wasn't triggered before.
r? ```@compiler-errors```
Initial `UnsafePinned` implementation [Part 1: Libs]
Initial libs changes necessary to unblock further work on [RFC 3467](https://rust-lang.github.io/rfcs/3467-unsafe-pinned.html).
Tracking issue: #125735
This PR is split off from #136964, and includes just the libs changes:
- `UnsafePinned` struct
- private `UnsafeUnpin` structural auto trait
- Lang items for both
- Compiler changes necessary to block niches on `UnsafePinned`
This PR does not change codegen, miri, the existing `!Unpin` hack, or anything else. That work is to be split into later PRs.
---
cc ``@RalfJung`` ``@Noratrieb``
``@rustbot`` label F-unsafe_pinned T-libs-api
PR #137977 changed `DefPathData::TypeNs` to contain `Option<Symbol>` to
account for RPITIT assoc types being anonymous. This commit changes it
back to `Symbol` and gives anonymous assoc types their own variant. It
makes things a bit nicer overall.
Allow drivers to supply a list of extra symbols to intern
Allows adding new symbols as `const`s in external drivers, desirable in Clippy so we can use them in patterns to replace code like 75530e9f72/src/tools/clippy/clippy_lints/src/casts/cast_ptr_alignment.rs (L66)
The Clippy change adds a couple symbols as a demo, the exact `clippy_utils` API and replacing other usages can be done on the Clippy side to minimise sync conflicts
---
try-job: aarch64-gnu
It bugs me when variables of type `Ident` are called `name`. It leads to
silly things like `name.name`. `Ident` variables should be called
`ident`, and `name` should be used for variables of type `Symbol`.
This commit improves things by by doing `s/name/ident/` on a bunch of
`Ident` variables. Not all of them, but a decent chunk.
Use `const_eval_select!()` macro to enable contract checking only at
runtime. The existing contract logic relies on closures,
which are not supported in constant functions.
This commit also removes one level of indirection for ensures clauses,
however, it currently has a spurious warning message when the bottom
of the function is unreachable.
Implement `super let`
Tracking issue: https://github.com/rust-lang/rust/issues/139076
This implements `super let` as proposed in #139080, based on the following two equivalence rules.
1. For all expressions `$expr` in any context, these are equivalent:
- `& $expr`
- `{ super let a = & $expr; a }`
2. And, additionally, these are equivalent in any context when `$expr` is a temporary (aka rvalue):
- `& $expr`
- `{ super let a = $expr; & a }`
So far, this experiment has a few interesting results:
## Interesting result 1
In this snippet:
```rust
super let a = f(&temp());
```
I originally expected temporary `temp()` would be dropped at the end of the statement (`;`), just like in a regular `let`, because `temp()` is not subject to temporary lifetime extension.
However, it turns out that that would break the fundamental equivalence rules.
For example, in
```rust
g(&f(&temp()));
```
the temporary `temp()` will be dropped at the `;`.
The first equivalence rule tells us this must be equivalent:
```rust
g({ super let a = &f(&temp()); a });
```
But that means that `temp()` must live until the last `;` (after `g()`), not just the first `;` (after `f()`).
While this was somewhat surprising to me at first, it does match the exact behavior we need for `pin!()`: The following _should work_. (See also https://github.com/rust-lang/rust/issues/138718)
```rust
g(pin!(f(&mut temp())));
```
Here, `temp()` lives until the end of the statement. This makes sense from the perspective of the user, as no other `;` or `{}` are visible. Whether `pin!()` uses a `{}` block internally or not should be irrelevant.
This means that _nothing_ in a `super let` statement will be dropped at the end of that super let statement. It does not even need its own scope.
This raises questions that are useful for later on:
- Will this make temporaries live _too long_ in cases where `super let` is used not in a hidden block in a macro, but as a visible statement in code like the following?
```rust
let writer = {
super let file = File::create(&format!("/home/{user}/test"));
Writer::new(&file)
};
```
- Is a `let` statement in a block still the right syntax for this? Considering it has _no_ scope of its own, maybe neither a block nor a statement should be involved
This leads me to think that instead of `{ super let $pat = $init; $expr }`, we might want to consider something like `let $pat = $init in $expr` or `$expr where $pat = $init`. Although there are also issues with these, as it isn't obvious anymore if `$init` should be subject to temporary lifetime extension. (Do we want both `let _ = _ in ..` and `super let _ = _ in ..`?)
## Interesting result 2
What about `super let x;` without initializer?
```rust
let a = {
super let x;
x = temp();
&x
};
```
This works fine with the implementation in this PR: `x` is extended to live as long as `a`.
While it matches my expectations, a somewhat interesting thing to realize is that these are _not_ equivalent:
- `super let x = $expr;`
- `super let x; x = $expr;`
In the first case, all temporaries in $expr will live at least as long as (the result of) the surrounding block.
In the second case, temporaries will be dropped at the end of the assignment statement. (Because the assignment statement itself "is not `super`".)
This difference in behavior might be confusing, but it _might_ be useful.
One might want to extend the lifetime of a variable without extending all the temporaries in the initializer expression.
On the other hand, that can also be expressed as:
- `let x = $expr; super let x = x;` (w/o temporary lifetime extension), or
- `super let x = { $expr };` (w/ temporary lifetime extension)
So, this raises these questions:
- Do we want to accept `super let x;` without initializer at all?
- Does it make sense for statements other than let statements to be "super"? An expression statement also drops temporaries at its `;`, so now that we discovered that `super let` basically disables that `;` (see interesting result 1), is there a use to having other statements without their own scope? (I don't think that's ever useful?)
## Interesting result 3
This works now:
```rust
super let Some(x) = a.get(i) else { return };
```
I didn't put in any special cases for `super let else`. This is just the behavior that 'naturally' falls out when implementing `super let` without thinking of the `let else` case.
- Should `super let else` work?
## Interesting result 4
This 'works':
```rust
fn main() {
super let a = 123;
}
```
I didn't put in any special cases for `super let` at function scope. I had expected the code to cause an ICE or other weird failure when used at function body scope, because there's no way to let the variable live as long as the result of the function.
This raises the question:
- Does this mean that this behavior is the natural/expected behavior when `super let` is used at function scope? Or is this just a quirk and should we explicitly disallow `super let` in a function body? (Probably the latter.)
---
The questions above do not need an answer to land this PR. These questions should be considered when redesigning/rfc'ing/stabilizing the feature.
Add new `PatKind::Missing` variants
To avoid some ugly uses of `kw::Empty` when handling "missing" patterns, e.g. in bare fn tys. Helps with #137978. Details in the individual commits.
r? ``@oli-obk``
