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.
Rename `LifetimeName` as `LifetimeKind`.
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.
r? ``@BoxyUwU``
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.
These were low value even before #137978 resulted in empty names being
used much less. (Why check for non-emptiness in these three places?
There are thousands of places in the compiler you could check.)
Overhaul `AssocItem`
`AssocItem` has multiple fields that only make sense some of the time. E.g. the `name` can be empty if it's an RPITIT associated type. It's clearer and less error prone if these fields are moved to the relevant `kind` variants.
r? ``@fee1-dead``
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```
`hir::AssocItem` currently has a boolean `fn_has_self_parameter` field,
which is misplaced, because it's only relevant for associated fns, not
for associated consts or types. This commit moves it (and renames it) to
the `AssocKind::Fn` variant, where it belongs.
This requires introducing a new C-style enum, `AssocTag`, which is like
`AssocKind` but without the fields. This is because `AssocKind` values
are passed to various functions like `find_by_ident_and_kind` to
indicate what kind of associated item should be searched for, and having
to specify `has_self` isn't relevant there.
New methods:
- Predicates `AssocItem::is_fn` and `AssocItem::is_method`.
- `AssocItem::as_tag` which converts `AssocItem::kind` to `AssocTag`.
Removed `find_by_name_and_kinds`, which is unused.
`AssocItem::descr` can now distinguish between methods and associated
functions, which slightly improves some error messages.
Allow parenthesis around inferred array lengths
In #135272 it was noticed that we weren't handling `Vec<(((((_)))))>` correctly under the new desugaring for `generic_arg_infer`, this had to be fixed in order to not regress stable code for types that should continue working. This has the side effect of *also* allowing the following to work:
```rust
#![feature(generic_arg_infer)]
struct Bar<const N: usize>;
fn main() {
let a: Bar<((_))> = Bar::<10>;
}
```
However I did not make the same change for array lengths resulting in the following not compiling:
```rust
#![feature(generic_arg_infer)]
fn main() {
let a: [u8; (((_)))] = [2; 2];
let a: [u8; 2] = [2; (((((_)))))];
}
```
This is rather inconsistent as parenthesis around `_` *are* supported for const args to non-arrays, and type args. This PR fixes this allowing the above example to compile. No stable impact.
r? compiler-errors
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.
Invert the order that we pass the arguments to the
`contract_check_ensures` function to avoid the warning when the tail
of the function is unreachable.
Note that the call itself is also unreachable, but we have already
handled that case by ignoring unreachable call for contract calls.
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.
Remove support for `extern "rust-intrinsic"` blocks
Part of rust-lang/rust#132735
Looked manageable and there didn't appear to have been progress in the last two weeks,
so decided to give it a try.
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``
Apply `Recovery::Forbidden` when reparsing pasted macro fragments.
Fixes#137874.
The changes to the output of `tests/ui/associated-consts/issue-93835.rs`
partly undo the changes seen when `NtTy` was removed in #133436, which
is good.
r? ``@petrochenkov``
In the AST, currently we use `BinOpKind` within `ExprKind::AssignOp` and
`AssocOp::AssignOp`, even though this allows some nonsensical
combinations. E.g. there is no `&&=` operator. Likewise for HIR and
THIR.
This commit introduces `AssignOpKind` which only includes the ten
assignable operators, and uses it in `ExprKind::AssignOp` and
`AssocOp::AssignOp`. (And does similar things for `hir::ExprKind` and
`thir::ExprKind`.) This avoids the possibility of nonsensical
combinations, as seen by the removal of the `bug!` case in
`lang_item_for_binop`.
The commit is mostly plumbing, including:
- Adds an `impl From<AssignOpKind> for BinOpKind` (AST) and `impl
From<AssignOp> for BinOp` (MIR/THIR).
- `BinOpCategory` can now be created from both `BinOpKind` and
`AssignOpKind`.
- Replaces the `IsAssign` type with `Op`, which has more information and
a few methods.
- `suggest_swapping_lhs_and_rhs`: moves the condition to the call site,
it's easier that way.
- `check_expr_inner`: had to factor out some code into a separate
method.
I'm on the fence about whether avoiding the nonsensical combinations is
worth the extra code.
They are no longer needed.
This does slightly worsen the error message for a single test, but that
test contains code that is so badly broken that I'm not worried about
it.
`ast::Item` has an `ident` field.
- It's always non-empty for these item kinds: `ExternCrate`, `Static`,
`Const`, `Fn`, `Mod`, `TyAlias`, `Enum`, `Struct`, `Union`,
`Trait`, `TraitAlias`, `MacroDef`, `Delegation`.
- It's always empty for these item kinds: `Use`, `ForeignMod`,
`GlobalAsm`, `Impl`, `MacCall`, `DelegationMac`.
There is a similar story for `AssocItemKind` and `ForeignItemKind`.
Some sites that handle items check for an empty ident, some don't. This
is a very C-like way of doing things, but this is Rust, we have sum
types, we can do this properly and never forget to check for the
exceptional case and never YOLO possibly empty identifiers (or possibly
dummy spans) around and hope that things will work out.
The commit is large but it's mostly obvious plumbing work. Some notable
things.
- `ast::Item` got 8 bytes bigger. This could be avoided by boxing the
fields within some of the `ast::ItemKind` variants (specifically:
`Struct`, `Union`, `Enum`). I might do that in a follow-up; this
commit is big enough already.
- For the visitors: `FnKind` no longer needs an `ident` field because
the `Fn` within how has one.
- In the parser, the `ItemInfo` typedef is no longer needed. It was used
in various places to return an `Ident` alongside an `ItemKind`, but
now the `Ident` (if present) is within the `ItemKind`.
- In a few places I renamed identifier variables called `name` (or
`foo_name`) as `ident` (or `foo_ident`), to better match the type, and
because `name` is normally used for `Symbol`s. It's confusing to see
something like `foo_name.name`.
Instead of calling new(), we can just use a struct expression directly.
Before:
Placeholder::new(…, …, …, …)
After:
Placeholder {
position: …,
flags: …,
width: …,
precision: …,
}
`hir::Lifetime::ident` currently sometimes uses `kw::Empty` for elided
lifetimes and sometimes uses `kw::UnderscoreLifetime`, and the
distinction is used when creating some error suggestions, e.g. in
`Lifetime::suggestion` and `ImplicitLifetimeFinder::visit_ty`. I found
this *really* confusing, and it took me a while to understand what was
going on.
This commit replaces all uses of `kw::Empty` in `hir::Lifetime::ident`
with `kw::UnderscoreLifetime`. It adds a new field
`hir::Lifetime::is_path_anon` that mostly replaces the old
empty/underscore distinction and makes things much clearer.
Some other notable changes:
- Adds a big comment to `Lifetime` talking about permissable field
values.
- Adds some assertions in `new_named_lifetime` about what ident values
are permissible for the different `LifetimeRes` values.
- Adds a `Lifetime::new` constructor that does some checking to make
sure the `is_elided` and `is_anonymous` states are valid.
- `add_static_impl_trait_suggestion` now looks at `Lifetime::res`
instead of the ident when creating the suggestion. This is the one
case where `is_path_anon` doesn't replace the old empty/underscore
distinction.
- A couple of minor pretty-printing improvements.
"Missing" patterns are possible in bare fn types (`fn f(u32)`) and
similar places. Currently these are represented in the AST with
`ast::PatKind::Ident` with no `by_ref`, no `mut`, an empty ident, and no
sub-pattern. This flows through to `{hir,thir}::PatKind::Binding` for
HIR and THIR.
This is a bit nasty. It's very non-obvious, and easy to forget to check
for the exceptional empty identifier case.
This commit adds a new variant, `PatKind::Missing`, to do it properly.
The process I followed:
- Add a `Missing` variant to `{ast,hir,thir}::PatKind`.
- Chang `parse_param_general` to produce `ast::PatKind::Missing`
instead of `ast::PatKind::Missing`.
- Look through `kw::Empty` occurrences to find functions where an
existing empty ident check needs replacing with a `PatKind::Missing`
check: `print_param`, `check_trait_item`, `is_named_param`.
- Add a `PatKind::Missing => unreachable!(),` arm to every exhaustive
match identified by the compiler.
- Find which arms are actually reachable by running the test suite,
changing them to something appropriate, usually by looking at what
would happen to a `PatKind::Ident`/`PatKind::Binding` with no ref, no
`mut`, an empty ident, and no subpattern.
Quite a few of the `unreachable!()` arms were never reached. This makes
sense because `PatKind::Missing` can't happen in every pattern, only
in places like bare fn tys and trait fn decls.
I also tried an alternative approach: modifying `ast::Param::pat` to
hold an `Option<P<Pat>>` instead of a `P<Pat>`, but that quickly turned
into a very large and painful change. Adding `PatKind::Missing` is much
easier.
Stabilize `#![feature(precise_capturing_in_traits)]`
# Precise capturing (`+ use<>` bounds) in traits - Stabilization Report
Fixes https://github.com/rust-lang/rust/issues/130044.
## Stabilization summary
This report proposes the stabilization of `use<>` precise capturing bounds in return-position impl traits in traits (RPITITs). This completes a missing part of [RFC 3617 "Precise capturing"].
Precise capturing in traits was not ready for stabilization when the first subset was proposed for stabilization (namely, RPITs on free and inherent functions - https://github.com/rust-lang/rust/pull/127672) since this feature has a slightly different implementation, and it hadn't yet been implemented or tested at the time. It is now complete, and the type system implications of this stabilization are detailed below.
## Motivation
Currently, RPITITs capture all in-scope lifetimes, according to the decision made in the ["lifetime capture rules 2024" RFC](https://rust-lang.github.io/rfcs/3498-lifetime-capture-rules-2024.html#return-position-impl-trait-in-trait-rpitit). However, traits can be designed such that some lifetimes in arguments may not want to be captured. There is currently no way to express this.
## Major design decisions since the RFC
No major decisions were made. This is simply an extension to the RFC that was understood as a follow-up from the original stabilization.
## What is stabilized?
Users may write `+ use<'a, T>` bounds on their RPITITs. This conceptually modifies the desugaring of the RPITIT to omit the lifetimes that we would copy over from the method. For example,
```rust
trait Foo {
fn method<'a>(&'a self) -> impl Sized;
// ... desugars to something like:
type RPITIT_1<'a>: Sized;
fn method_desugared<'a>(&'a self) -> Self::RPITIT_1<'a>;
// ... whereas with precise capturing ...
fn precise<'a>(&'a self) -> impl Sized + use<Self>;
// ... desugars to something like:
type RPITIT_2: Sized;
fn precise_desugared<'a>(&'a self) -> Self::RPITIT_2;
}
```
And thus the GAT doesn't name `'a`. In the compiler internals, it's not implemented exactly like this, but not in a way that users should expect to be able to observe.
#### Limitations on what generics must be captured
Currently, we require that all generics from the trait (including the `Self`) type are captured. This is because the generics from the trait are required to be *invariant* in order to do associated type normalization.
And like regular precise capturing bounds, all type and const generics in scope must be captured.
Thus, only the in-scope method lifetimes may be relaxed with this syntax today.
## What isn't stabilized? (a.k.a. potential future work)
See section above. Relaxing the requirement to capture all type and const generics in scope may be relaxed when https://github.com/rust-lang/rust/issues/130043 is implemented, however it currently interacts with some underexplored corners of the type system (e.g. unconstrained type bivariance) so I don't expect it to come soon after.
## Implementation summary
This functionality is implemented analogously to the way that *opaque type* precise capturing works.
Namely, we currently use *variance* to model the capturedness of lifetimes. However, since RPITITs are anonymous GATs instead of opaque types, we instead modify the type relation of GATs to consider variances for RPITITs (along with opaque types which it has done since https://github.com/rust-lang/rust/pull/103491).
30f168ef81/compiler/rustc_middle/src/ty/util.rs (L954-L976)30f168ef81/compiler/rustc_type_ir/src/relate.rs (L240-L244)
Using variance to model capturedness is an implementation detail, and in the future it would be desirable if opaques and RPITITs simply did not include the uncaptured lifetimes in their generics. This can be changed in a forwards-compatible way, and almost certainly would not be observable by users (at least not negatively, since it may indeed fix some bugs along the way).
## Tests
* Test that the lifetime isn't actually captured: `tests/ui/impl-trait/precise-capturing/rpitit.rs` and `tests/ui/impl-trait/precise-capturing/rpitit-outlives.rs` and `tests/ui/impl-trait/precise-capturing/rpitit-outlives-2.rs`.
* Technical test for variance computation: `tests/ui/impl-trait/in-trait/variance.rs`.
* Test that you must capture all trait generics: `tests/ui/impl-trait/precise-capturing/forgot-to-capture-type.rs`.
* Test that you cannot capture more than what the trait specifies: `tests/ui/impl-trait/precise-capturing/rpitit-captures-more-method-lifetimes.rs` and `tests/ui/impl-trait/precise-capturing/rpitit-impl-captures-too-much.rs`.
* Undercapturing (refinement) lint: `tests/ui/impl-trait/in-trait/refine-captures.rs`.
### What other unstable features may be exposed by this feature?
I don't believe that this exposes any new unstable features indirectly.
## Remaining bugs and open issues
Not aware of any open issues or bugs.
## Tooling support
Rustfmt: ✅ Supports formatting `+ use<>` everywhere.
Clippy: ✅ No support needed, unless specific clippy lints are impl'd to care for precise capturing itself.
Rustdoc: ✅ Rendering `+ use<>` precise capturing bounds is supported.
Rust-analyzer: ✅ Parser support, and then lifetime support isn't needed https://github.com/rust-lang/rust/pull/138128#issuecomment-2705292494 (previous: ~~❓ There is parser support, but I am unsure of rust-analyzer's level of support for RPITITs in general.~~)
## History
Tracking issue: https://github.com/rust-lang/rust/issues/130044
* https://github.com/rust-lang/rust/pull/131033
* https://github.com/rust-lang/rust/pull/132795
* https://github.com/rust-lang/rust/pull/136554
Visitors track whether an assoc item is in a trait impl or an inherent impl
`AssocCtxt::Impl` now contains an `of_trait` field. This allows ast lowering and nameres to not have to track whether we're in a trait impl or an inherent impl.
Add a helper for building an owner id in ast lowering
Just some deduplication of owner-id creations. Will also help me later split up ast lowering into per-owner queries, as it won't be possible anymore to go from a NodeId to a DefId of an owner without doing extra work to check whether we have an owner id. So I'd just do that in the new `owner_id` function and keep the `local_def_id` function free of that logic
Reduce FormattingOptions to 64 bits
This is part of https://github.com/rust-lang/rust/issues/99012
This reduces FormattingOptions from 6-7 machine words (384 bits on 64-bit platforms, 224 bits on 32-bit platforms) to just 64 bits (a single register on 64-bit platforms).
Before:
```rust
pub struct FormattingOptions {
flags: u32, // only 6 bits used
fill: char,
align: Option<Alignment>,
width: Option<usize>,
precision: Option<usize>,
}
```
After:
```rust
pub struct FormattingOptions {
/// Bits:
/// - 0-20: fill character (21 bits, a full `char`)
/// - 21: `+` flag
/// - 22: `-` flag
/// - 23: `#` flag
/// - 24: `0` flag
/// - 25: `x?` flag
/// - 26: `X?` flag
/// - 27: Width flag (if set, the width field below is used)
/// - 28: Precision flag (if set, the precision field below is used)
/// - 29-30: Alignment (0: Left, 1: Right, 2: Center, 3: Unknown)
/// - 31: Always set to 1
flags: u32,
/// Width if width flag above is set. Otherwise, always 0.
width: u16,
/// Precision if precision flag above is set. Otherwise, always 0.
precision: u16,
}
```
Use `Option<Ident>` for lowered param names.
Parameter patterns are lowered to an `Ident` by `lower_fn_params_to_names`, which is used when lowering bare function types, trait methods, and foreign functions. Currently, there are two exceptional cases where the lowered param can become an empty `Ident`.
- If the incoming pattern is an empty `Ident`. This occurs if the parameter is anonymous, e.g. in a bare function type.
- If the incoming pattern is neither an ident nor an underscore. Any such parameter will have triggered a compile error (hence the `span_delayed_bug`), but lowering still occurs.
This commit replaces these empty `Ident` results with `None`, which eliminates a number of `kw::Empty` uses, and makes it impossible to fail to check for these exceptional cases.
Note: the `FIXME` comment in `is_unwrap_or_empty_symbol` is removed. It actually should have been removed in #138482, the precursor to this PR. That PR changed the lowering of wild patterns to `_` symbols instead of empty symbols, which made the mentioned underscore check load-bearing.
r? ``@compiler-errors``
