Fixed ICE for EII with multiple defaults due to duplicate definition in nameres
r? ``@jieyouxu`` (since you looked at the other one)
Fixes https://github.com/rust-lang/rust/issues/149982
Previously a [fix was proposed](https://github.com/rust-lang/rust/pull/149985) by ``@SATVIKsynopsis`` which I marked as co-author on the first commit for the test they contributed. I'm closing this previous PR.
Duplicate definitions of EII defaults shouldn't be possible. I want to still panic on them, since I want to know when other bugs exist. However, in this case the duplicate was caused by something more subtle: both eiis have the same name, and as such a "duplicate definition" error is given. However, the compiler gracefully continues compiling despite that, assuming only one of the two EIIs is actually defined.
Both defaults then name resolve, and find the same single remaining EII, and both register themselves to be its default, breaking the single-default assumption.
The solution: I added a span-delayed-bug, to make sure we only panic if we hadn't previously had this duplicate definition name resolution error.
Thanks to ``@SATVIKsynopsis`` for their attempt. Adding a diagnostic here could make some sense, but nonetheless I think this is the better solution here <3
Also thanks to ``@yaahc`` for debugging help, she made me understand the name resolution of the situation so much better and is just lovely in general :3
The last commit is something I tried during debugging, which felt like a relevant test to add (one where both eiis also have the same function name)
Support recursive delegation
This PR adds support for recursive delegations and is a part of the delegation feature rust-lang/rust#118212.
r? ``@petrochenkov``
Suggest struct pattern when destructuring Range with .. syntax
implemented a new diagnostic in rustc_resolve to detect invalid range destructuring attempts (e.g., let start..end = range). The fix identifies when resolution fails for identifiers acting as range bounds specifically handling cases where bounds are parsed as expressions and suggests the correct struct pattern syntax (std::ops::Range { start, end }). This replaces confusing "cannot find value" errors with actionable help, verified by a new UI test covering various identifier names.
Fixesrust-lang/rust#149777
Make closure capturing have consistent and correct behaviour around patterns
Reference PR:
- https://github.com/rust-lang/reference/pull/1837
This PR has two goals:
- firstly, it fixesrust-lang/rust#137467. In order to do so, it needs to introduce a small breaking change surrounding the interaction of closure captures with matching against enums with uninhabited variants. Yes – to fix an ICE!
- this also fixesrust-lang/rust#138973, a slightly different case with the same root cause.
- likewise, fixesrust-lang/rust#140011.
- secondly, it fixesrust-lang/rust#137553, making the closure capturing rules consistent between `let` patterns and `match` patterns. This is new insta-stable behavior.
## Background
This change concerns how precise closure captures interact with patterns. As a little known feature, patterns that require inspecting only part of a value will only cause that part of the value to get captured:
```rust
fn main() {
let mut a = (21, 37);
// only captures a.0, writing to a.1 does not invalidate the closure
let mut f = || {
let (ref mut x, _) = a;
*x = 42;
};
a.1 = 69;
f();
}
```
I was not able to find any discussion of this behavior being introduced, or discussion of its edge-cases, but it is [documented in the Rust reference](https://doc.rust-lang.org/reference/types/closure.html#r-type.closure.capture.precision.wildcard).
The currently stable behavior is as follows:
- if any pattern contains a binding, the place it binds gets captured (implemented in current `walk_pat`)
- patterns in refutable positions (`match`, `if let`, `let ... else`, but not destructuring `let` or destructuring function parameters) get processed as follows (`maybe_read_scrutinee`):
- if matching against the pattern will at any point require inspecting a discriminant, or it includes a variable binding not followed by an ``@`-pattern,` capture *the entire scrutinee* by reference
You will note that this behavior is quite weird and it's hard to imagine a sensible rationale for at least some of its aspects. It has the following issues:
- firstly, it assumes that matching against an irrefutable pattern cannot possibly require inspecting any discriminants. With or-patterns, this isn't true, and it is the cause of the rust-lang/rust#137467 ICE.
- secondly, the presence of an ``@`-pattern` doesn't really have any semantics by itself. This is the weird behavior tracked as rust-lang/rust#137553.
- thirdly, the behavior is different between pattern-matching done through `let` and pattern-matching done through `match` – which is a superficial syntactic difference
This PR aims to address all of the above issues. The new behavior is as follows:
- like before, if a pattern contains a binding, the place it binds gets captured as required by the binding mode
- if matching against the pattern requires inspecting a disciminant, the place whose discriminant needs to be inspected gets captured by reference
"requires inspecting a discriminant" is also used here to mean "compare something with a constant" and other such decisions. For types other than ADTs, the details are not interesting and aren't changing.
## The breaking change
During closure capture analysis, matching an `enum` against a constructor is considered to require inspecting a discriminant if the `enum` has more than one variant. Notably, this is the case even if all the other variants happen to be uninhabited. This is motivated by implementation difficulties involved in querying whether types are inhabited before we're done with type inference – without moving mountains to make it happen, you hit this assert: https://github.com/rust-lang/rust/blob/43f0014ef0f242418674f49052ed39b70f73bc1c/compiler/rustc_middle/src/ty/inhabitedness/mod.rs#L121
Now, because the previous implementation did not concern itself with capturing the discriminants for irrefutable patterns at all, this is a breaking change – the following example, adapted from the testsuite, compiles on current stable, but will not compile with this PR:
```rust
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
enum Void {}
pub fn main() {
let mut r = Result::<Void, (u32, u32)>::Err((0, 0));
let mut f = || {
let Err((ref mut a, _)) = r;
*a = 1;
};
let mut g = || {
//~^ ERROR: cannot borrow `r` as mutable more than once at a time
let Err((_, ref mut b)) = r;
*b = 2;
};
f();
g();
assert_eq!(r, Err((1, 2)));
}
```
## Is the breaking change necessary?
One other option would be to double down, and introduce a set of syntactic rules for determining whether a sub-pattern is in an irrefutable position, instead of querying the types and checking how many variants there are.
**This would not eliminate the breaking change,** but it would limit it to more contrived examples, such as
```rust
let ((true, Err((ref mut a, _, _))) | (false, Err((_, ref mut a, _)))) = x;
```
In this example, the `Err`s would not be considered in an irrefutable position, because they are part of an or-pattern. However, current stable would treat this just like a tuple `(bool, (T, U, _))`.
While introducing such a distinction would limit the impact, I would say that the added complexity would not be commensurate with the benefit it introduces.
## The new insta-stable behavior
If a pattern in a `match` expression or similar has parts it will never read, this part will not be captured anymore:
```rust
fn main() {
let mut a = (21, 37);
// now only captures a.0, instead of the whole a
let mut f = || {
match a {
(ref mut x, _) => *x = 42,
}
};
a.1 = 69;
f();
}
```
Note that this behavior was pretty much already present, but only accessible with this One Weird Trick™:
```rust
fn main() {
let mut a = (21, 37);
// both stable and this PR only capture a.0, because of the no-op `@-pattern`
let mut f = || {
match a {
(ref mut x @ _, _) => *x = 42,
}
};
a.1 = 69;
f();
}
```
## The second, more practically-relevant breaking change
After running crater, we have discovered that the aforementioned insta-stable behavior, where sometimes closures will now capture less, can also manifest as a breaking change. This is because it is possible that previously a closure would capture an entire struct by-move, and now it'll start capturing only part of it – some by move, and some by reference. This then causes the closure to have a more restrictive lifetime than it did previously.
See:
- https://github.com/rust-lang/rust/pull/138961#issuecomment-2761888557
- https://github.com/EC-labs/cec-assignment/pull/1
- https://github.com/tryandromeda/andromeda/pull/43
## Implementation notes
The PR has two main commits:
- "ExprUseVisitor: properly report discriminant reads" makes `walk_pat` perform all necessary capturing. This is the part that fixesrust-lang/rust#137467.
- "ExprUseVisitor: remove maybe_read_scrutinee" removes the unnecessary "capture the entire scrutinee" behavior, fixing rust-lang/rust#137553.
The new logic stops making the distinction between one particular example that used to work, and another ICE, tracked as rust-lang/rust#119786. As this requires an unstable feature, I am leaving this as future work.
Port `#[rustc_no_implicit_autorefs]`, `#[rustc_lint_opt_ty]`, and `#[rustc_lint_query_instability]` attributes to be parsed
This PR ports three simple internal rustc attributes to be parsed in a single PR to avoid having to resolve multiple rebase confilicts
r? ``@JonathanBrouwer``
Tidying up tests/ui/issues 14 tests [5/N]
> [!NOTE]
> Intermediate commits are intended to help review, but will be squashed add comment commit prior to merge.
part of rust-lang/rust#133895
move `tests/ui/inherent-impls-overlap-check` to `tests/ui/duplicate/inherent-impls-overlap-check`.
r? Kivooeo
The split between walk_pat and maybe_read_scrutinee has now become
redundant.
Due to this change, one testcase within the testsuite has become similar
enough to a known ICE to also break. I am leaving this as future work,
as it requires feature(type_alias_impl_trait)
Avoid unhelpful suggestion when crate name is invalid
Pointing out they can set the crate's name is non-actionable: their problem is they found out how and set it incorrectly. Remove extraneous information that can only confuse the matter.
Pointing out they can set the crate's name is non-actionable:
their problem is they found out how and set it incorrectly.
Remove extraneous information that can only confuse the matter.
Port #[no_link] to use attribute parser
Adds `#[no_link]` to the attribute parser, as well as adds tests making sure to FCW warn on `field`, `arm`, and `macrodef `
Port `#[rustc_legacy_const_generics]` to use attribute parser
Small PR that ports the `#[rustc_legacy_const_generics]` to use the new attribute parser!
r? JonathanBrouwer
Use annotate-snippet as default emitter on stable
This is implementation of https://github.com/rust-lang/rust/issues/149932
Now, after MCP was accepted, we can use annotate-snippet as default emitter for errors, that means that we not longer need of previous emitter, so this PR removed previous emitter and makes annotate-snippet new default one both on stable and nightly
(this PR does not remove a code of previous emitter it just removes a `Default` option of `HumanReadableErrorType` enum, and keeping only `HumanReadableErrorType::AnnotateSnippet` as it now uses by default)
dont create unnecessary `DefId`s under mgca
Fixesrust-lang/rust#149977Fixesrust-lang/rust#148838
Accidentally left this out of rust-lang/rust#149136 even though being able to do this was a large part of the point of the PR :3
First ICE was caused by the fact that we create a defid but never lower the nodeid associated with it to a hirid which later parts of the compiler can't handle.
See test for second ICE
r? oli-obk
Tidying up tests/ui/issues 33 tests [4/N]
> [!NOTE]
> Intermediate commits are intended to help review, but will be squashed add comment commit prior to merge.
part of rust-lang/rust#133895
`tests/ui/compile-flags` split it into `tests/ui/compile-flags/invalid/` and `tests/ui/compile-flags/run-pass/`
r? Kivooeo
Fix span note for question mark expression
Fixesrust-lang/rust#144304
Seems it's better to fix the note instead of modifying the span to cover the whole expression.
r? `@estebank`
Introduces `BackendRepr::ScalableVector` corresponding to scalable
vector types annotated with `repr(scalable)` which lowers to a scalable
vector type in LLVM.
Co-authored-by: Jamie Cunliffe <Jamie.Cunliffe@arm.com>
Extend well-formedness checking and HIR analysis to prohibit the use of
scalable vectors in structs, enums, unions, tuples and arrays. LLVM does
not support scalable vectors being members of other types, so these
restrictions are necessary.
Co-authored-by: Jamie Cunliffe <Jamie.Cunliffe@arm.com>
Extend parsing of `ReprOptions` with `rustc_scalable_vector(N)` which
optionally accepts a single literal integral value - the base multiple of
lanes that are in a scalable vector. Can only be applied to structs.
Co-authored-by: Jamie Cunliffe <Jamie.Cunliffe@arm.com>
Warn on codegen attributes on required trait methods
This PR turns applying the following attributes on required trait methods (that is, trait methods **without** a default implementation) into a FCW:
- `#[cold]`
- `#[link_section]`
- `#[linkage]` (unstable)
- `#[rustc_allow_const_fn_unstable]` (internal attribute)
These attributes already had no effect when applied to a required trait method, this PR only adds a warning.
Furthermore, it adds a comment in the code that the following codegen attributes are *inherited* when applied to a required trait method:
- `#[track_caller]`
- `#[align]` (unstable)
````@rustbot```` labels +I-lang-nominated
````@rust-lang/lang````
Two questions for the lang team:
- Is adding this warning ok?
- Does the current behaviour of these attributes align with that you would expect them to be?
Fixes https://github.com/rust-lang/rust/issues/147432
Jonathan Brouwer