[mlir][Sol] Make sol.revert's signature attribute optional - #133
Merged
Conversation
abinavpp
force-pushed
the
app-dispatcher-revert-strings
branch
7 times, most recently
from
July 28, 2026 12:55
5b8b157 to
8f57970
Compare
Distinguish revert() and revert(""). Lower revert("") to an
ABI-encoded Error(""), while lowering revert() to a bare revert(0, 0).
PavelKopyl
force-pushed
the
kpv-fix-rever-sign
branch
from
July 28, 2026 13:08
d2b97be to
81663a7
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the Sol dialect sol.revert operation to make its signature attribute optional, allowing the IR to represent plain revert() distinctly from revert(""), and adapts the Sol→Yul lowering accordingly.
Changes:
- Make
sol.revert’ssignatureanOptionalAttr<StrAttr>and adjust the textual assembly format to allow omission. - Update
RevertOpLoweringto treat “absent signature” asrevert()and “present (even empty) signature” asrevert("reason").
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| mlir/lib/Conversion/SolToYul/SolToYul.cpp | Updates sol.revert lowering to handle optional signatures and preserve the empty-reason vs no-reason distinction. |
| mlir/include/mlir/Dialect/Sol/SolOps.td | Makes signature optional for sol.revert and updates the op’s assembly format/documentation accordingly. |
Comments suppressed due to low confidence (1)
mlir/include/mlir/Dialect/Sol/SolOps.td:545
- There are currently no MLIR/lit tests covering
sol.revertparsing/printing or the semantic distinction betweenrevert(),revert(""), andrevert("reason"). Since this PR changes the textual assembly format and the meaning of an absent vs emptysignature, adding a round-trip test would help prevent regressions.
let arguments = (ins Variadic<AnyType>:$args,
OptionalAttr<StrAttr>:$signature, UnitAttr:$call);
let assemblyFormat = "($signature^)? ($args^ `:` type($args))? attr-dict";
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
NomicFoundation/solx-solidity#169
Distinguish revert() and revert(""). Lower revert("") to an ABI-encoded Error(""), while lowering revert() to a bare revert(0, 0).