[v2] Reintroduce built-ins to the language definition and generate an enum for the AST API#1655
Open
ggiraldez wants to merge 4 commits intoggiraldez/v2-migrate-astfrom
Open
[v2] Reintroduce built-ins to the language definition and generate an enum for the AST API#1655ggiraldez wants to merge 4 commits intoggiraldez/v2-migrate-astfrom
ggiraldez wants to merge 4 commits intoggiraldez/v2-migrate-astfrom
Conversation
|
5dc7adb to
e917ee5
Compare
31cb643 to
e63f118
Compare
e917ee5 to
592548c
Compare
Generate an enum of all the built-ins that can be exposed in a public API.
e63f118 to
1778173
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on top of #1597
This PR reintroduces the built-in definitions to the language model, this time as a simple enumeration. This serves a dual purpose:
NodeIdandTypeIdwhich the existingBuiltInenum has for resolution/typing purposes.The built-ins declarations contain the valid version range, but that's not used for validation since it's hard-coded in the built-ins resolver code. It would be hard to generate that code from the definitions, but maybe we can use that information somehow when implementing semantic validation.
I'm not sure it makes sense to put the generated enum in the
commoncrate, but since it's only constants it maybe fits the bill there. Happy to move it somewhere else. Ideally I'd like to avoid having also an internal enum, but I don't see how we can remove that without losing the type checking benefits for the variants that carry extra information.The PR also removes obsolete built-ins no longer valid in Solidity >= 0.8.0 that slipped through the cracks in the migration.