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

Inject extension scopes while running the resolution algorithm #1170

Merged
merged 17 commits into from
Dec 17, 2024

Conversation

ggiraldez
Copy link
Contributor

@ggiraldez ggiraldez commented Nov 27, 2024

This PR builds on top of #1149

This changes the way we handle extension scopes (ie. using directives) in bindings. Instead of using the scope stack from the stack graph, we hook into the resolution algorithm and inject new graph edges from nodes designated as extension hooks (usually the source unit's lexical scope) to extension scopes which are defined at each contract/library and contain the definition nodes from using directives. This simplifies the rules quite a bit and greatly improves performance, particularly in the case of Solidity < 0.7.0 where using directives are inherited from base contracts.

This PR also moves built-ins parsing and ingestion to slang_solidity crate. Since the built-ins file needs to be pre-processed to transform the symbols as to ensure no conflicts can occur with user code, adding the built-ins requires a couple of manual steps that were replicated in every construction of Solidity bindings API. By encapsulating this functionality in the slang_solidity crate we remove a source of user error and make it easier to make changes to the built-ins ingestion code.

Copy link

changeset-bot bot commented Nov 27, 2024

⚠️ No Changeset found

Latest commit: 5a6ee9e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

The ResolverCandidates will insert arbitrary edges to the graph connecting to
the extension scopes determined available at the beginning of resolution (these
are context dependent) when reaching nodes marked as extension hooks (usually
the source unit's lexical scope).
There's an initial dual purpose:
- make resolution reentrant-safe, as some of the resolution tweaks require
resolving references themselves
- enable recursive lookup of extension scopes for Solidity < 0.7.0
Since extensions are now injected during the resolution phases, it's no longer
needed to have a separate extended scope. This simplifies the existing rules
quite a bit.

Also removes all previous commented out rules that dealt with pushing the
extension scope to the scope stack, since that mechanism is no longer used.
Instead of having to manually customize the resolver. Also, simplify
`ResolveOptions` to an enum with two possible values: `Full` and `NonRecursive`.
`NonRecursive` is used internally from `simple_resolve` to disable code paths
that could lead to infinite recursions when attempting to resolve a reference.
Since the built-ins file needs to be pre-processed to transform the symbols as
to ensure no conflicts can occur with user code, adding the built-ins requires a
couple of manual steps that were replicated in every construction of Solidity
bindings API. By encapsulating this functionality in the `slang_solidity` crate
we remove a source of user error and make it easier to make changes to the
built-ins ingestion code.
@ggiraldez ggiraldez marked this pull request as ready for review December 14, 2024 02:24
@ggiraldez ggiraldez requested review from a team as code owners December 14, 2024 02:24
@ggiraldez ggiraldez enabled auto-merge December 17, 2024 19:42
@ggiraldez ggiraldez added this pull request to the merge queue Dec 17, 2024
Merged via the queue into NomicFoundation:main with commit ced4a9a Dec 17, 2024
1 check passed
@ggiraldez ggiraldez deleted the inject-extensions branch December 17, 2024 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants