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

Ensure that ItemMeta is registered for root items (fix #521) #522

Merged
merged 2 commits into from
May 25, 2023

Conversation

udoprog
Copy link
Collaborator

@udoprog udoprog commented May 25, 2023

Due to past refactoring, ItemMeta is missing for the root module, meaning any macro which expands into the root will end up erroring as described in #521.

This ensures that root item meta is setup as appropriate and adds a test to make sure item expansion works on all relevant code locations for functions.

There is a bit of "identifier soup" going on right now which I'd like to refactor some time in the future, namely items have two kinds of identifiers:

  • One which is associated with its allocation in the item pool (allowing for deduplicating item allocations).
  • One which associated context metadata to the item, such as its visibility, which module its in, the source location it belongs to. This is what's being referenced by ast items such as ItemFn::id.

The second one is what's being associated with the opaque ast id you can see all over the place, and is the one which was missing. We just had to make sure item meta was present for it and that the Items stack which is used to build items are seeded correctly with identifiers.

Thanks to @ModProg for finding and reporting the issue!

@udoprog udoprog added the bug Something isn't working label May 25, 2023
@udoprog udoprog mentioned this pull request May 25, 2023
@udoprog udoprog force-pushed the missing-root-item-meta branch from 84ed820 to dd1a8b1 Compare May 25, 2023 14:04
@udoprog udoprog merged commit ebfb91e into main May 25, 2023
@udoprog udoprog deleted the missing-root-item-meta branch May 25, 2023 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant