Fix anon const def-creation when macros are involved
Fixes#128016.
Ever since #125915, some `ast::AnonConst`s turn into `hir::ConstArgKind::Path`s,
which don't have associated `DefId`s. To deal with the fact that we don't have
resolution information in `DefCollector`, we decided to implement a process
where if the anon const *appeared* to be trivial (i.e., `N` or `{ N }`), we
would avoid creating a def for it in `DefCollector`. If later, in AST lowering,
we realized it turned out to be a unit struct literal, or we were lowering it
to something that didn't use `hir::ConstArg`, we'd create its def there.
However, let's say we have a macro `m!()` that expands to a reference to a free
constant `FOO`. If we use `m!()` in the body of an anon const (e.g., `Foo<{ m!() }>`),
then in def collection, it appears to be a nontrivial anon const and we create
a def. But the macro expands to something that looks like a trivial const arg,
but is not, so in AST lowering we "fix" the mistake we assumed def collection
made and create a def for it. This causes a duplicate definition ICE.
The long-term fix for this is to delay the creation of defs for all expression-like
nodes until AST lowering (see #128844 for an incomplete attempt at this). This
would avoid issues like this one that are caused by hacky workarounds. However,
doing this uncovers a pre-existing bug with opaque types that is quite involved
to fix (see #129023).
In the meantime, this PR fixes the bug by delaying def creation for anon consts
whose bodies are macro invocations until after we expand the macro and know
what is inside it. This is accomplished by adding information to create the
anon const's def to the data in `Resolver.invocation_parents`.
r? `@BoxyUwU`
...and remove the `const_arg_path` feature gate as a result. It was only
a stopgap measure to fix the regression that the new lowering introduced
(which should now be fixed by this PR).
Ever since #125915, some `ast::AnonConst`s turn into `hir::ConstArgKind::Path`s,
which don't have associated `DefId`s. To deal with the fact that we don't have
resolution information in `DefCollector`, we decided to implement a process
where if the anon const *appeared* to be trivial (i.e., `N` or `{ N }`), we
would avoid creating a def for it in `DefCollector`. If later, in AST lowering,
we realized it turned out to be a unit struct literal, or we were lowering it
to something that didn't use `hir::ConstArg`, we'd create its def there.
However, let's say we have a macro `m!()` that expands to a reference to a free
constant `FOO`. If we use `m!()` in the body of an anon const (e.g., `Foo<{ m!() }>`),
then in def collection, it appears to be a nontrivial anon const and we create
a def. But the macro expands to something that looks like a trivial const arg,
but is not, so in AST lowering we "fix" the mistake we assumed def collection
made and create a def for it. This causes a duplicate definition ICE.
The ideal long-term fix for this is a bit unclear. One option is to delay def
creation for all expression-like nodes until AST lowering (see #128844 for an
incomplete attempt at this). This would avoid issues like this one that are
caused by hacky workarounds. However, this approach has some downsides as well,
and the best approach is yet to be determined.
In the meantime, this PR fixes the bug by delaying def creation for anon consts
whose bodies are macro invocations until after we expand the macro and know
what is inside it. This is accomplished by adding information to create the
anon const's def to the data in `Resolver.invocation_parents`.
Introduce `'ra` lifetime name.
`rustc_resolve` allocates many things in `ResolverArenas`. The lifetime used for references into the arena is mostly `'a`, and sometimes `'b`.
This commit changes it to `'rslv`, which is much more descriptive. The commit also changes the order of lifetimes on a couple of structs so that '`rslv` is second last, before `'tcx`, and does other minor renamings such as `'r` to `'a`.
r? ``@petrochenkov``
cc ``@oli-obk``
Simplify some nested `if` statements
Applies some but not all instances of `clippy::collapsible_if`. Some ended up looking worse afterwards, though, so I left those out. Also applies instances of `clippy::collapsible_else_if`
Review with whitespace disabled please.
`rustc_resolve` allocates many things in `ResolverArenas`. The lifetime
used for references into the arena is mostly `'a`, and sometimes `'b`.
This commit changes it to `'ra`, which is much more descriptive. The
commit also changes the order of lifetimes on a couple of structs so
that '`ra` is second last, before `'tcx`, and does other minor
renamings such as `'r` to `'a`.
Retroactively feature gate `ConstArgKind::Path`
This puts the lowering introduced by #125915 under a feature gate until we fix the regressions introduced by it. Alternative to whole sale reverting the PR since it didn't seem like a very clean revert and I think this is generally a step in the right direction and don't want to get stuck landing and reverting the PR over and over :)
cc #129137 ``@camelid,`` tests taken from there. beta is branching soon so I think it makes sense to not try and rush that fix through since it wont have much time to bake and if it has issues we can't simply revert it on beta.
Fixes#128016
Use shorthand field initialization syntax more aggressively in the compiler
Caught these when cleaning up #129344 and decided to run clippy to find the rest
Use `bool` in favor of `Option<()>` for diagnostics
We originally only supported `Option<()>` for optional notes/labels, but we now support `bool`. Let's use that, since it usually leads to more readable code.
I'm not removing the support from the derive macro, though I guess we could error on it... 🤔
Don't consider locals to shadow inner items' generics
We don't want to consider the bindings from a `RibKind::Module` itself, because for an inner item that module will contain the local bindings from the function body or wherever else the inner item is being defined.
Fixes#129265
r? petrochenkov
rm `import.used`
By the way, `import_used_map` will only be used during `build_reduced_graph` and `finalize`, so it can be split from `Resolver` in the future.
r? ``@petrochenkov``
Use more slice patterns inside the compiler
Nothing super noteworthy. Just replacing the common 'fragile' pattern of "length check followed by indexing or unwrap" with slice patterns for legibility and 'robustness'.
r? ghost
Only walk ribs to collect possibly shadowed params if we are adding params in our new rib
No need to collect params from parent ribs if we literally have no params to declare in this new rib.
Attempt to win back some of the perf in https://github.com/rust-lang/rust/pull/128357#issuecomment-2262677031.
Please review with whitespace *off*, the diff should be like 2 lines.
r? petrochenkov
Add `REDUNDANT_IMPORTS` lint for new redundant import detection
Defaults to Allow for now. Stacked on #123744 to avoid merge conflict, but much easier to review all as one.
r? petrochenkov
Structured suggestion for `extern crate foo` when `foo` isn't resolved in import
When encountering a name in an import that could have come from a crate that wasn't imported, use a structured suggestion to suggest `extern crate foo;` pointing at the right place in the crate.
When encountering `_` in an import, do not suggest `extern crate _;`.
```
error[E0432]: unresolved import `spam`
--> $DIR/import-from-missing-star-3.rs:2:9
|
LL | use spam::*;
| ^^^^ maybe a missing crate `spam`?
|
help: consider importing the `spam` crate
|
LL + extern crate spam;
|
```
When encountering a name in an import that could have come from a crate that wasn't imported, use a structured suggestion to suggest `extern crate foo;` pointing at the right place in the crate.
When encountering `_` in an import, do not suggest `extern crate _;`.
```
error[E0432]: unresolved import `spam`
--> $DIR/import-from-missing-star-3.rs:2:9
|
LL | use spam::*;
| ^^^^ maybe a missing crate `spam`?
|
help: consider importing the `spam` crate
|
LL + extern crate spam;
|
```
We don't want to have questions in the diagnostic output. Instead, we use wording that communicates uncertainty, like "might":
```
error[E0432]: unresolved import `spam`
--> $DIR/import-from-missing-star-3.rs:2:9
|
LL | use spam::*;
| ^^^^ you might be missing crate `spam`
|
= help: consider adding `extern crate spam` to use the `spam` crate
```
Clean up a few minor refs in `format!` macro, as it has a performance cost. Apparently the compiler is unable to inline `format!("{}", &variable)`, and does a run-time double-reference instead (format macro already does one level referencing). Inlining format args prevents accidental `&` misuse.
When finding item gated behind a `cfg` flag, point at it
Previously we would only mention that the item was gated out, and opportunisitically mention the feature flag name when possible. We now point to the place where the item was gated, which can be behind layers of macro indirection, or in different modules.
```
error[E0433]: failed to resolve: could not find `doesnt_exist` in `inner`
--> $DIR/diagnostics-cross-crate.rs:18:23
|
LL | cfged_out::inner::doesnt_exist::hello();
| ^^^^^^^^^^^^ could not find `doesnt_exist` in `inner`
|
note: found an item that was configured out
--> $DIR/auxiliary/cfged_out.rs:6:13
|
LL | pub mod doesnt_exist {
| ^^^^^^^^^^^^
note: the item is gated here
--> $DIR/auxiliary/cfged_out.rs:5:5
|
LL | #[cfg(FALSE)]
| ^^^^^^^^^^^^^
```
