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.

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

Create anon const defs in DefCollector unconditionally #133285

Closed
wants to merge 1 commit into from

Conversation

BoxyUwU
Copy link
Member

@BoxyUwU BoxyUwU commented Nov 21, 2024

Fixes #133064

Macros in statement position can expand to nothing which fundamentally breaks any attempt to not create DefIds that don't need to be created. When a macro expands to nothing we never call visit_expr on its expanded results so never create the DefId for the anon const which leads to ICEs. So... just completely give up on this and make def encoding deal with having defids exist that shouldn't.

Alternatively we could make macro!{} never expand to nothing and always atleast be an empty statement, this would be a breaking change due to:

macro_rules! empty {
    () => {};
}

fn bar() -> usize {
    {
        1
    }
    empty! {}
}

compiling on stable

r? @compiler-errors

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 21, 2024
@BoxyUwU
Copy link
Member Author

BoxyUwU commented Nov 21, 2024

Should also add the following test:

struct Foo<const N: usize>;

macro_rules! expands_to_n {
    () => {
        N
    };
}

macro_rules! empty {
    () => {};
}

const N: usize = 10;

type A = Foo<
    {
        empty! {}
        expands_to_n! {}
    },
>;

which demonstrates why we cant simply special case a fully expanded const argument being {}

@BoxyUwU
Copy link
Member Author

BoxyUwU commented Nov 21, 2024

Actually going to close this until lcnr has had a chance to play around with this since they said they wanted to take a look and see if they could find a clean way to make this work

@BoxyUwU BoxyUwU closed this Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ICE: node HirId .. cannot be placed in TypeckResults with hir_owner DefId ..
3 participants