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

Refactor InstKind to move metadata from macros to the type. #4119

Merged
merged 3 commits into from
Jul 11, 2024

Conversation

jonmeow
Copy link
Contributor

@jonmeow jonmeow commented Jul 10, 2024

This adds DefinitionInfo for Define-based configuration so that parameters are optional. It also makes it easier to provide the equivalent functions on both Definition and Define.

A common pattern used here is to change from a switch with in-line cases to instead have cases that call an overloaded function. What's happening here is that the instruction type is used to select an overload, and if an overload is not defined, a compiler error would result. Meanwhile, clusters of overloads are being defined using requires-based templating, so that equivalent implementations are not copied. This addresses a limitation of a vanilla switch approach where it's hard to have redundant cases using conditional logic, while also getting compiler errors when adding new InstKind entries, which had been a significant part of why we used macros previously.

This starts hitting some odd clang-format edge cases causing CARBON_KIND_SWITCH(inst){ (missing space), which I haven't seen before. Adding CARBON_KIND_SWITCH to .clang-format works around it.

@jonmeow

This comment was marked as resolved.

@zygoloid zygoloid added this pull request to the merge queue Jul 11, 2024
Merged via the queue into carbon-language:trunk with commit 469f1c8 Jul 11, 2024
7 checks passed
@jonmeow jonmeow deleted the inst-macros branch July 11, 2024 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants