Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix anon const def-creation when macros are involved #129137

Merged
merged 2 commits into from
Sep 13, 2024
Merged

Fix anon const def-creation when macros are involved #129137

merged 2 commits into from
Sep 13, 2024

Commits on Sep 12, 2024

  1. Fix anon const def-creation when macros are involved 

    Ever since rust-lang#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 rust-lang#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`.
    @camelid
    camelid committed Sep 12, 2024
    Configuration menu
    Copy the full SHA
    8b75004 View commit details
    Browse the repository at this point in the history

  2. Re-enable ConstArgKind::Path lowering by default 

    ...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).
    @camelid
    camelid committed Sep 12, 2024
    Configuration menu
    Copy the full SHA
    e0bd011 View commit details
    Browse the repository at this point in the history