feat(slang): emit external getters for public state variables#463
Draft
hedgar2017 wants to merge 1 commit into
Draft
feat(slang): emit external getters for public state variables#463hedgar2017 wants to merge 1 commit into
hedgar2017 wants to merge 1 commit into
Conversation
Solidity auto-generates an external `view` getter for every `public` state variable. The Slang frontend was registering only user-defined functions, so any test that read a public state var through its getter selector failed at the test harness's selector-lookup step (`selector of the method 'X' not found`). For each `ContractMember::StateVariableDefinition`, when `compute_abi_entry()` returns a `Function` entry with no inputs (scalar getter) the emitter now lays down a `sol.func` named after the slang canonical signature, carrying the slang-computed selector and `View` mutability, with a body that loads the corresponding storage slot and returns the value. Indexed getters for `T[]`, mappings, and structs still need `sol.gep` / `sol.map` plumbing and are deliberately skipped so the rest of the contract still compiles. `AstEmitter::emit` also includes state-variable selectors in the method-identifier table the caller hands to the test harness. `tests/solidity/simple -O M3B3`: 1896/3/307 baseline → 1952/13/298, net +56 passing tests (+10 failed, -9 invalid).
There was a problem hiding this comment.
Pull request overview
Adds Slang-frontend support for Solidity’s auto-generated external getters for public state variables by (1) emitting sol.func getter stubs for supported cases and (2) exposing state-variable selectors to the test harness selector-lookup table, improving compatibility with Solidity tests that call these getters.
Changes:
- Extend
AstEmittermethod-identifier collection to include state-variable getter selectors. - Emit external
viewgettersol.funcs for some public state variables by loading from the computed storage slot and returning the value.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
solx-slang/src/ast/mod.rs |
Adds state-variable signatures/selectors to the method-identifier map returned to the harness. |
solx-slang/src/ast/contract/mod.rs |
Emits MLIR sol.func getter bodies for state variables (currently keyed off ABI “no-input” getters). |
Comment on lines
+69
to
+77
| ContractMember::StateVariableDefinition(state_variable) => { | ||
| let Some(signature) = state_variable.compute_canonical_signature() else { | ||
| continue; | ||
| }; | ||
| let Some(selector) = state_variable.compute_selector() else { | ||
| continue; | ||
| }; | ||
| method_identifiers.insert(signature, format!("{selector:08x}")); | ||
| } |
Comment on lines
+143
to
+147
| // Scalar getters only for now — indexed forms need sol.gep / sol.map. | ||
| if !abi.inputs().is_empty() { | ||
| return Ok(()); | ||
| } | ||
| let Some(signature) = state_variable.compute_canonical_signature() else { |
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.
Solidity auto-generates an external
viewgetter for everypublicstate variable. The Slang frontend was registering only user-defined functions, so any test that read a public state var through its getter selector failed at the test harness's selector-lookup step (selector of the method 'X' not found).For each
ContractMember::StateVariableDefinition, whencompute_abi_entry()returns aFunctionentry with no inputs (scalar getter) the emitter now lays down asol.funcnamed after the slang canonical signature, carrying the slang-computed selector andViewmutability, with a body that loads the corresponding storage slot and returns the value. Indexed getters forT[], mappings, and structs still needsol.gep/sol.mapplumbing and are deliberately skipped so the rest of the contract still compiles.AstEmitter::emitalso includes state-variable selectors in the method-identifier table the caller hands to the test harness.tests/solidity/simple -O M3B3moves from 1896 passed / 3 failed / 307 invalid to 1952 passed / 13 failed / 298 invalid — net +56 passing tests (+10 failed, -9 invalid). The new failures are tests where the getter resolves but some other behavior in the same test mismatches; the bulk of the formerly-invalidselector not foundcluster (num,count,text,MY_ADDRESS,oneWei, etc.) now runs and most pass.