feat(slang): inheritance by hedgar2017 · Pull Request #697 · NomicFoundation/solx

hedgar2017 · 2026-09-07T23:44:56Z

Lowers contracts with contract bases in the Slang frontend: inherited state and getters, virtual dispatch to the most-derived override, super and contract-qualified calls, and the base-constructor chain with each argument list evaluated at the call to its target. The tester's Slang view follows the compilation API of the revision this pins.

cargo run-tester-slang: 20182 passed, 85 failed, 35 invalid.

github-actions · 2026-09-08T07:35:29Z

Coverage Summary

Crate	Line Coverage	Function Coverage
solx	🟢 83.6%	🔴 20.0%
solx-benchmark-converter	🔴 0.0%	🔴 0.0%
solx-codegen-evm	🔴 24.7%	🔴 11.5%
solx-compiler-downloader	🔴 0.0%	🔴 0.0%
solx-core	🔴 39.2%	🔴 46.9%
solx-dev	🔴 2.4%	🔴 3.0%
solx-evm-assembly	🔴 0.0%	🔴 0.0%
solx-mlir	🟡 50.3%	🔴 46.3%
solx-slang	🔴 22.6%	🔴 31.0%
solx-solc-test-adapter	🔴 1.7%	🔴 2.1%
solx-standard-json	🔴 42.6%	🔴 47.7%
solx-tester	🔴 36.1%	🔴 34.3%
solx-utils	🔴 32.3%	🔴 35.1%
solx-yul	🔴 0.0%	🔴 0.0%
Total	🔴 10.9%	🔴 13.1%

Codecov Report | HTML Report | Workflow Run

Copilot

Copilot review overview

🟡 Changes recommended

The fallback fixture contains an unobserved base implementation, so its CHECKs do not fully pin the input.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

Make the unused base fallback declaration abstract · New

What changed in this PR

Adds Solidity inheritance lowering to the Slang frontend, covering inherited members, virtual dispatch, and constructor chaining.

Changes:

Lowers inherited state, getters, functions, super, and qualified calls.
Builds base-constructor chains with correctly scoped arguments.
Updates MLIR dispatch APIs, Slang revision, and inheritance fixtures.

File	Description
`solx-slang/src/source_unit.rs`	Enables contracts with bases.
`solx-slang/src/scope/source_unit.rs`	Passes constructor-chain context.
`solx-slang/src/scope/function.rs`	Tracks constructor frames.
`solx-slang/src/scope/contract.rs`	Adds hierarchy-aware resolution.
`solx-slang/src/contract/state_variable.rs`	Initializes inherited state.
`solx-slang/src/contract/object.rs`	Exposes linearized members and layout.
`solx-slang/src/contract/mod.rs`	Emits inherited contract members.
`solx-slang/src/contract/getter/mod.rs`	Updates getter dispatch.
`solx-slang/src/contract/function/statement/revert.rs`	Adapts argument lowering.
`solx-slang/src/contract/function/statement/event.rs`	Adapts event arguments.
`solx-slang/src/contract/function/mod.rs`	Emits inherited functions and constructors.
`solx-slang/src/contract/function/expression/mod.rs`	Handles parenthesized inheritance expressions.
`solx-slang/src/contract/function/expression/member.rs`	Resolves `super` and qualified members.
`solx-slang/src/contract/function/expression/keyword.rs`	Updates keyword documentation.
`solx-slang/src/contract/function/expression/identifier.rs`	Applies virtual function resolution.
`solx-slang/src/contract/function/expression/call/mod.rs`	Classifies inheritance-related calls.
`solx-slang/src/contract/function/expression/call/arguments.rs`	Simplifies converted arguments.
`solx-slang/src/contract/constructor_chain/mod.rs`	Implements constructor chaining.
`solx-slang/src/contract/constructor_chain/arguments.rs`	Models base-constructor arguments.
`solx-mlir/tests/lit/inheritance_virtual.sol`	Covers virtual and qualified calls.
`solx-mlir/tests/lit/inheritance_synthesized.sol`	Covers synthesized constructors.
`solx-mlir/tests/lit/inheritance_super.sol`	Covers `super` resolution.
`solx-mlir/tests/lit/inheritance_state.sol`	Covers inherited state.
`solx-mlir/tests/lit/inheritance_getter.sol`	Covers getter overrides.
`solx-mlir/tests/lit/inheritance_fallback.sol`	Covers fallback inheritance.
`solx-mlir/tests/lit/inheritance_constructor.sol`	Covers constructor arguments.
`solx-mlir/src/lib.rs`	Removes the obsolete function-entry export.
`solx-mlir/src/context/function/mod.rs`	Returns entry blocks directly.
`solx-mlir/src/context/function/entry.rs`	Removes the entry wrapper.
`solx-mlir/src/context/function/dispatch.rs`	Adds symbol-only dispatch.
`Cargo.toml`	Pins the required Slang revision.
`Cargo.lock`	Records the updated Slang sources.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

Copilot review overview

🟡 Changes recommended

Constructor-chain construction introduces a prohibited hierarchy pre-pass and threaded cache.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 · 1

Open (2)

Avoid precomputing constructor hierarchy; resolve lazily during emission · New
Make the unused base fallback declaration abstract

Copilot

Copilot review overview

🟡 Changes recommended

Selector dispatch is still precomputed before emission, contrary to the frontend’s required on-demand model.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 · 1

Open (2)

Avoid eager selector pre-pass; derive eligibility during emission · New
Rename identifier binding to node_id · New

Resolved since last review (2)

Avoid precomputing constructor hierarchy; resolve lazily during emission
Make the unused base fallback declaration abstract

abinavpp · 2026-09-21T14:11:26Z

+    /// The functions the object dispatches and defines: a contract's resolved hierarchy, where an
+    /// overridden or getter-shadowed function has given way to its override, listed in declaration
+    /// order per contract of its linearisation, as print-init emits them; a library's own.
+    pub fn functions(&self) -> Vec<FunctionDefinition> {


"dispatches and defines"? you mean all functions?

don't say "print-init". nothing in the codegen should depend on mlir textual dumps

yes, the comment is outdated

yes, also leftover slop, everything verified against legacy although semanticTests don't really cover everything

Thanks for noticing!

abinavpp · 2026-09-21T15:08:22Z

thank you for working on this!

This is pre-existing, but what does the Object enum in solx-slang/src/contract/object.rs buy us over slang's own definitions? Object::functions(), state_variables() and storage_layout() are now thin wrappers over linearised_functions(), linearised_state_variables() and compute_abi(), and the library cases are the plain node.functions() / node.state_variables(). If I read functions() right, the extra walk over contracts() is only there to emit in linearisation/declaration order rather than slang's name order

abinavpp · 2026-09-21T14:38:12Z

-            Some(enclosing) if enclosing.node_id() == self.object_id => function.compute_selector(),
+
+        let is_constructor = matches!(function.kind(), FunctionKind::Constructor);
+        let is_most_derived = is_constructor


is_most_derived_constructor instead?

and combine the:

if is_constructor { if is_most_derived {

below

abinavpp

thank you! lgmt! mostly nits/questions. i don't see any major concerns with the lowering..

abinavpp · 2026-09-21T15:58:16Z

    /// Classifies `call`'s callee into the single kind that emits it. A type conversion is probed
-    /// before the callee's shape, its callee may be an elementary type or `payable` keyword as well
-    /// as a named type, and its one-argument arity is part of the classification, per the variant's
-    /// definition.
+    /// before the callee's shape, since its callee may be an elementary type or `payable` keyword
+    /// as well as a named type. The lookup of a named function is the callee's shape: a bare name is virtual,
+    /// a `super` member resolves after its enclosing contract, and a contract-qualified name names its
+    /// declaration.


This function just classifies a call depending on the callee/syntax, right? I personally find the comment hard to read. Sounds like the overly technical Claude Code English that I fight every day :P. Maybe we could have the technical stuff next to the relevant code instead and keep the header dumb and simple?

Also, maybe this whole thing could be part of slang?

abinavpp · 2026-09-21T16:18:55Z

+/// The constructors and arguments emitted for the object's creation.
+pub struct Constructor<'context> {
+    /// The contracts of the object's linearisation, consumed in call order.
+    pub contracts: IntoIter<ContractDefinition>,


Oh, one more thing, not sure about it: the chain works by mutating Constructor in a fixed order: next_contract advances an iterator, function_definition sets current and forwarded, base_constructor_call consumes them, and None in arguments means "the synthesized constructor". Could Constructor::new compute all of this up front (for each constructor: who it calls, which argument lists it evaluates, what it forwards) so emission just reads a table?

ggiraldez

I left some minor suggestions and questions, but looks good to me! Can't really evaluate the MLIR part, but I can see Abinav already approved as well.

ggiraldez · 2026-09-21T20:44:23Z

-        if call.is_type_conversion()
-            && let ArgumentsDeclaration::PositionalArguments(arguments) = &call.arguments()
-            && arguments.len() == 1
-        {
+        if call.is_type_conversion() {


Should we move the check on the arguments to the AST? Not sure how the removal affects the rest of the backend.

ggiraldez · 2026-09-21T21:14:09Z

+                if is_most_derived {
+                    scope.state_variable_initializers();
+                }
+                scope.base_constructor_call();


AFAIU, this is compatible with the legacy pipeline in solc. I think it would be nice to have it documented with a comment and some kind of marker, so we can easily refer to areas of the backend that conform to legacy vs. via-IR.

ggiraldez · 2026-09-21T21:36:28Z

+            scope.state_variable_initializers();
+            scope.base_constructor_call();


Ditto here re: the legacy vs via-IR compatibility documentation.

ggiraldez · 2026-09-21T22:06:46Z

+                self.contracts()
+                    .iter()
+                    .flat_map(|base| base.functions())
+                    .filter(|function| resolved.contains(&function.node_id()))
+                    .collect()


I assume this is to get the "declaration order per contract of its linearisation". Curious to know, why is this necessary?

ggiraldez · 2026-09-22T04:28:39Z

+//!
+//! Constructor emission in Slang's linearisation order.
+//!


This was by far the hardest file to understand. I think a brief explanation of how the constructor for each contract is generated would help a lot here. Eg. why do we need to forward parameters, how that can change the declared signature of intermediate contracts, how Constructor mutates as the linearised bases are traversed, etc.. The sections "The value that has to travel" and "The trace" from this Claude walkthrough were the most informative.

ggiraldez · 2026-09-22T04:29:34Z

+pub struct Constructor<'context> {
+    /// The contracts of the object's linearisation, consumed in call order.
+    pub contracts: IntoIter<ContractDefinition>,
+    /// The argument lists supplied for bases, keyed by the provider's definition id.


It's unclear to me what "provider definition id" refers to and what it means when it's None.

ggiraldez · 2026-09-22T04:44:22Z

+use crate::scope::function::FunctionScope;
+
+/// The constructors and arguments emitted for the object's creation.
+pub struct Constructor<'context> {


On the first pass I didn't realize this object mutated as the constructor was being built. Maybe rename this to ConstructorBuilder or something similar?

hedgar2017 force-pushed the az-slang-inheritance branch 2 times, most recently from 7cbb0fa to 586eea6 Compare September 8, 2026 07:23

hedgar2017 force-pushed the az-slang-inheritance branch 6 times, most recently from 28da541 to 7fb3184 Compare September 9, 2026 22:15

abinavpp force-pushed the app-slang-inline-assembly branch 2 times, most recently from c237244 to 6d41503 Compare September 16, 2026 13:18

hedgar2017 force-pushed the app-slang-inline-assembly branch from 6d41503 to 96cebda Compare September 16, 2026 16:03

abinavpp force-pushed the app-slang-inline-assembly branch 3 times, most recently from ed6c4df to 31ee803 Compare September 17, 2026 12:29

Base automatically changed from app-slang-inline-assembly to main September 17, 2026 19:59

hedgar2017 force-pushed the az-slang-inheritance branch 9 times, most recently from 2629bb2 to 88459fe Compare September 19, 2026 13:46

hedgar2017 self-assigned this Sep 19, 2026

hedgar2017 marked this pull request as ready for review September 19, 2026 13:47

hedgar2017 force-pushed the az-slang-inheritance branch from 88459fe to 67d6b7c Compare September 19, 2026 13:52

hedgar2017 requested review from a team and a balanced review from Copilot September 19, 2026 13:52

hedgar2017 requested a review from a team September 19, 2026 13:52

Copilot started reviewing on behalf of hedgar2017 September 19, 2026 13:53 View session

Copilot AI reviewed Sep 19, 2026

View reviewed changes

Comment thread solx-mlir/tests/lit/inheritance_fallback.sol Outdated

hedgar2017 force-pushed the az-slang-inheritance branch 3 times, most recently from ceb06b0 to 1793fad Compare September 19, 2026 15:03

hedgar2017 requested a balanced review from Copilot September 19, 2026 15:04

Copilot started reviewing on behalf of hedgar2017 September 19, 2026 15:04 View session

hedgar2017 force-pushed the az-slang-inheritance branch from 1793fad to 3e3b8da Compare September 19, 2026 15:05

Copilot AI reviewed Sep 19, 2026

View reviewed changes

Comment thread solx-slang/src/contract/mod.rs Outdated

hedgar2017 force-pushed the az-slang-inheritance branch from 959f1ae to 204dfbd Compare September 20, 2026 21:27

hedgar2017 requested a balanced review from Copilot September 20, 2026 21:30

Copilot started reviewing on behalf of hedgar2017 September 20, 2026 21:30 View session

Copilot AI reviewed Sep 20, 2026

View reviewed changes

Comment thread solx-slang/src/scope/contract.rs Outdated

Comment thread solx-slang/src/contract/constructor.rs Outdated

hedgar2017 force-pushed the az-slang-inheritance branch from 204dfbd to 95dcff6 Compare September 21, 2026 08:15

hedgar2017 added 2 commits September 21, 2026 12:22

feat(slang): inheritance

b21756a

refactor: dissolve the constructor chain

434b7d6

hedgar2017 force-pushed the az-slang-inheritance branch from 95dcff6 to 434b7d6 Compare September 21, 2026 08:32

abinavpp reviewed Sep 21, 2026

View reviewed changes

abinavpp approved these changes Sep 21, 2026

View reviewed changes

abinavpp reviewed Sep 21, 2026

View reviewed changes

ggiraldez approved these changes Sep 22, 2026

View reviewed changes

		scope.state_variable_initializers();
		scope.base_constructor_call();

Conversation

hedgar2017 commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hedgar2017 Sep 21, 2026 • 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!

abinavpp commented Sep 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abinavpp left a comment

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 left a comment

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hedgar2017 commented Sep 7, 2026 •

edited

Loading

github-actions Bot commented Sep 8, 2026 •

edited

Loading

hedgar2017 Sep 21, 2026 •

edited

Loading