AST Rust API #1477

ggiraldez · 2025-12-03T01:56:15Z

This PR defines an AST API as a façade over the last IR language currently available (ir2_flat_contracts) that allows opaque access to the IR nodes and the semantic information computed in the frontend passes.

From the SemanticAnalysis object that is obtainable from the CompilationUnit it's possible to retrieve the root of the AST of each file in the unit get_file_ast_root(), which returns ast::SourceUnit.
Navigation from the SourceUnit down through the intermediate AST nodes, down to the terminal nodes.
Non-terminal nodes are wrapped in objects which contain a reference to the IR node and the SemanticAnalysis.
Terminal nodes are not wrapped by default except for Identifier nodes, which allow attempting to resolve to a Definition object.
A Visitor trait can be implemented and used via accept_source_unit (and related functions) to traverse the wrapped AST.
Non-terminal nodes define methods to access the child nodes.
Certain non-terminal nodes also define other methods to access other semantic information (eg. SourceUnit defines contracts() to retrieve the list of contracts defined in the file)

changeset-bot · 2025-12-03T01:56:20Z

⚠️ No Changeset found

Latest commit: b039fbe

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

…isitor

crates/solidity/outputs/cargo/crate/src/backend/semantic/mod.rs

crates/solidity/outputs/cargo/crate/src/backend/ir/mod.rs

OmarTawfik · 2025-12-19T05:04:22Z

crates/solidity/outputs/cargo/crate/src/backend/ir/ast/nodes.rs

+    define_variant!(YulVariable, Identifier);
+}
+
+include!("ast_nodes.generated.rs");


I was confused by the structure/imports until I found this include!() here.
I suggest using the same pattern in other IRs, by renaming ast_nodes to nodes, and the new nodes here to node_extensions. In that case, ast/mod.rs can simply pub use nodes::* and pub use node_extensions::* to achieve the same result.

I also suggest splitting the new types/extensions into type-specific modules under ast/node_extensions/*, as I expect them to only grow in size, and it would make reviewing them much easier.

Sounds good. I thought it was not possible to extend the implementations (defining new impl blocks) for types in a different module, but I was mistaken. I'll change the structure of this code.

OmarTawfik · 2025-12-19T05:12:18Z

crates/solidity/outputs/cargo/crate/src/backend/ir/ast/nodes.rs

+}
+
+impl IdentifierStruct {
+    fn create(ir_node: &Rc<TerminalNode>, semantic: &Rc<SemanticAnalysis>) -> Self {


should this be pub(crate)?

Yes, this is an oversight.

OmarTawfik · 2025-12-19T05:16:54Z

crates/solidity/outputs/cargo/crate/src/backend/ir/ast/nodes.rs

+            .unwrap()
+    }
+
+    define_variant!(Constant);


In addition to the extra SemanticAnalysis field for every def in the compilation, this struct/define_variant!() pattern makes it harder to match on the type, resorting to the added is_/as_ extensions.

I wonder if we should make it a proper enum instead? Each *Definition type contains a SemanticAnalysis object already.

enum Definition { Contract(ContractDefinition), Enum(EnumDefinition), // etc... }

I also suggest adding a sync marker between the variants of this Definition and binder::Definition.

I'll try changing this to an enum. Seeing the code in perspective it makes perfect sense.

OmarTawfik · 2025-12-19T05:19:31Z

crates/solidity/outputs/cargo/crate/src/backend/ir/ast/nodes.rs

+        }
+    }
+
+    fn internal_definition(&self) -> &binder::Definition {


I see this PR adds definitions, but not references.
Is this planned for a future PR?

References would need to be added to the the Definition (or each *Definition AST node). It's not added yet because we don't have the inverse mapping computed yet on the SemanticAnalysis object, but that's easy to do. I think that can go into this PR. I'll add that.

crates/solidity/outputs/cargo/crate/src/backend/ir/ast/visitor.rs.jinja2

OmarTawfik · 2025-12-19T05:33:39Z

crates/solidity/outputs/cargo/crate/src/backend/semantic/mod.rs

+                let binder::Definition::Contract(contract) = definition else {
+                    return None;
+                };
+                if definition.identifier().unparse() == name {


Not blocking this PR:

I notice in many places in the backend we are using unparse() (which clones the string value) just to get a reference to check/compare. I wonder if we should use/add a fn TerminalNode.value() -> &str instead.

Just to add on this, we should intern all of these strings so these comparisons are just over ids (numbers).

I like the idea of interning, which will also further improve performance of the binder, as there's a big chunk of time spent in hashing identifiers for lookup. This should be done in the parser though.

OmarTawfik · 2025-12-19T05:35:24Z

crates/solidity/outputs/cargo/crate/src/backend/ir/ast/nodes.generated.rs

@@ -0,0 +1,6549 @@
+// This file is generated automatically by infrastructure scripts. Please don't edit by hand.


I wonder if we measured the impact of adding this additional Rc<SemanticAnalysis> to every single node in the tree?
I like the new API, and that people don't have to see/interact with the SemanticAnalysis object at all, but I want to make sure that the increase in memory/allocation time is acceptable.

I think an issue for me right now is that we're cloning everything by default, so if a user wants to traverse the AST they will create a huge amount of (probably short lived) Rcs to it. The API looks almost like a typed cursor, so I wonder if maybe the methods should consume self rather than borrow it:

pub fn members(self) -> SourceUnitMembersStruct { SourceUnitMembersStruct::create( &self.ir_node.members, self.semantic, )) }

This way, if a user actually needs to keep the reference they can cheaply clone it (we should #[derive(Clone)])

Also, whenever we start looking into allocators we'll probably bring in lifetimes in, so a lot of this Rc<SemanticAnalysis> should become &'arena SemanticAnalysis

~~Would adding a Rc<SemanticAnalysis> to each IR node be even possible? I think there is a circular dependency there, and the borrow checker will not like that.~~ Nevermind this, I read the opening comment wrong.

Re: the use of Rc, maybe we could be using value structs directly for Rust code, but once we cross into Wasm territory we need to keep track of the memory somehow, and our wrappers will use Rcs anyway. The AST is a façade/cursor API so maybe it's not that impactful?

OmarTawfik · 2025-12-19T05:39:02Z

crates/solidity/outputs/cargo/tests/src/backend/passes/pipeline.rs

    let unit = build_compilation_unit()?;
    let semantic_analysis = unit.semantic_analysis();
-    assert_eq!(2, semantic_analysis.files().len());
+    assert_eq!(3, semantic_analysis.files().len());


should we move this test to sample.rs as well, eliminating this module?

Good point!

OmarTawfik

Left a few questions/suggestions. Thanks!

teofr

Left a few comments and questions

crates/solidity/outputs/cargo/crate/src/backend/ir/ast/nodes.generated.rs

teofr · 2025-12-19T14:17:57Z

crates/solidity/outputs/cargo/crate/src/backend/ir/ast/nodes.generated.rs

@@ -0,0 +1,6549 @@
+// This file is generated automatically by infrastructure scripts. Please don't edit by hand.


I think an issue for me right now is that we're cloning everything by default, so if a user wants to traverse the AST they will create a huge amount of (probably short lived) Rcs to it. The API looks almost like a typed cursor, so I wonder if maybe the methods should consume self rather than borrow it:

pub fn members(self) -> SourceUnitMembersStruct { SourceUnitMembersStruct::create( &self.ir_node.members, self.semantic, )) }

This way, if a user actually needs to keep the reference they can cheaply clone it (we should #[derive(Clone)])

teofr · 2025-12-19T14:18:51Z

crates/solidity/outputs/cargo/crate/src/backend/ir/ast/nodes.generated.rs

@@ -0,0 +1,6549 @@
+// This file is generated automatically by infrastructure scripts. Please don't edit by hand.


Also, whenever we start looking into allocators we'll probably bring in lifetimes in, so a lot of this Rc<SemanticAnalysis> should become &'arena SemanticAnalysis

crates/solidity/outputs/cargo/crate/src/backend/ir/ast/nodes.rs.jinja2

crates/solidity/outputs/cargo/crate/src/backend/ir/ast/mod.rs

teofr · 2025-12-19T14:57:53Z

crates/solidity/outputs/cargo/crate/src/backend/semantic/mod.rs

+                let binder::Definition::Contract(contract) = definition else {
+                    return None;
+                };
+                if definition.identifier().unparse() == name {


Just to add on this, we should intern all of these strings so these comparisons are just over ids (numbers).

Also constructs the this (reverse) mapping after resolving all references.

crates/solidity/outputs/cargo/crate/src/backend/binder/mod.rs

teofr

Left a small comment, but LGTM

teofr · 2025-12-31T11:13:15Z

crates/solidity/outputs/cargo/crate/src/backend/binder/mod.rs

+                        definitions.insert(*node_id, vec![*reference_id]);
+                    }
+                }
+                Resolution::Ambiguous(_) | Resolution::BuiltIn(_) | Resolution::Unresolved => {}


Small note, we may be interested in computing references for BuiltIns as well.

ggiraldez force-pushed the ast-api branch from 94da337 to dcfc4da Compare December 13, 2025 00:23

ggiraldez added 11 commits December 16, 2025 11:33

Beginnings of a public facing AST API

af056ab

Wrap Identifier and IdentifierPath and resolve to definitions

14275bb

Allow converting from Definition into the appropriate AST node types

49d4b44

Retrieve AST trees from the SemanticAnalysis object

48d3e9d

Add unit test for SemanticAnalysis and some methods of the AST

7f1f42e

Add all_definitions and find_contract_by_name to SemanticAnalysis

0ce7990

Extend test sample

1886306

Implement and test contract methods to compute bases

b39e0fe

Generalize support for custom AST types

91ee0f6

Add more field accessors to AST types and generate a visitor for them

826f3fc

Visit custom types (ie. Identifier) and add unit test for the AST v…

19cffba

…isitor

ggiraldez force-pushed the ast-api branch from dcfc4da to 19cffba Compare December 16, 2025 16:47

ggiraldez added 4 commits December 16, 2025 13:10

Fix visiting collection types inside choices

0200e6e

Eliminate most custom types in AST templates

f32e02c

Improve templates for AST nodes and visitor generation

a9c3567

Relax expectation on linearised bases being available for all contracts

b7807dc

ggiraldez marked this pull request as ready for review December 17, 2025 23:27

ggiraldez requested review from a team as code owners December 17, 2025 23:27

ggiraldez requested review from OmarTawfik and teofr December 18, 2025 14:59

OmarTawfik requested changes Dec 19, 2025

View reviewed changes

teofr requested changes Dec 19, 2025

View reviewed changes

ggiraldez added 2 commits December 19, 2025 18:35

Move AST custom code for nodes to node_extensions module

0ba9202

Split node_extensions module into several files

441840e

OmarTawfik self-requested a review December 20, 2025 07:27

Provide an API to access the mapping from definitions to references

d59b4c0

Also constructs the this (reverse) mapping after resolving all references.

ggiraldez added 4 commits December 23, 2025 12:00

Refactor and improve mapping from definition to references AST methods

2cbcc71

Change Definition to be an enum instead of a struct

e5f34a3

Move backend pipeline test into the sample module

e7ccf9e

Add TODO comment to unify File with SemanticFile

938e54d

teofr reviewed Dec 29, 2025

View reviewed changes

crates/solidity/outputs/cargo/crate/src/backend/binder/mod.rs Show resolved Hide resolved

ggiraldez added 2 commits December 30, 2025 12:56

Change AST choice types to enums

2789ffd

Add marker comment for definition types (AST and binder)

b039fbe

ggiraldez requested a review from teofr December 30, 2025 18:49

teofr approved these changes Dec 31, 2025

View reviewed changes

This was referenced Jan 5, 2026

Add getters to AST Contracts API manastech/slang#6

Closed

Compute ABI, storage layout and add getters to Contracts AST API #1499

Draft

		@@ -0,0 +1,6549 @@
		// This file is generated automatically by infrastructure scripts. Please don't edit by hand.

AST Rust API #1477

Are you sure you want to change the base?

AST Rust API #1477

Uh oh!

Conversation

ggiraldez commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OmarTawfik Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OmarTawfik Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggiraldez Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggiraldez Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ggiraldez Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

OmarTawfik left a comment

Choose a reason for hiding this comment

Uh oh!

teofr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ggiraldez commented Dec 3, 2025 •

edited

Loading

changeset-bot bot commented Dec 3, 2025 •

edited

Loading

OmarTawfik Dec 19, 2025 •

edited

Loading

OmarTawfik Dec 19, 2025 •

edited

Loading

ggiraldez Dec 19, 2025 •

edited

Loading

ggiraldez Dec 19, 2025 •

edited

Loading

ggiraldez Dec 19, 2025 •

edited

Loading