fix(slang): convert operands to the types their dialect ops admit by PavelKopyl · Pull Request #676 · NomicFoundation/solx

PavelKopyl · 2026-08-27T14:17:53Z

This includes:

aligning 'sol.byte' with 'bytes1' at conversion boundaries
converting arguments for 'sol.ecrecover', 'sol.send', and 'sol.transfer'
folding constant identifiers at their declared type

Fixed semantic tests:

array/indexAccess/bytes_index_access.sol
functionCall/send_zero_ether.sol
isoltestTesting/balance_other_contract.sol
revertStrings/transfer.sol

Copilot

Pull request overview

This PR tightens Slang→Sol dialect lowering by ensuring operands are converted to the exact MLIR types required by the Sol dialect ops, with particular focus on bridging sol.byte vs bytes1 at the right boundaries and fixing built-in/member-call argument typing.

Changes:

Convert ecrecover operands (hash, r, s) to !sol.fixedbytes<32> at the call boundary; convert send/transfer amounts to ui256.
Adjust compound-assignment lowering to operate at the semantic element type while writing back through Place::assign (fixing sol.byte element cases).
Extend Value::convert to correctly convert dynamic bytes/string values into !sol.byte via !sol.fixedbytes<1>, and expand lit coverage for these conversion paths.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
`solx-slang/src/contract/function/expression/call/mod.rs`	Ensures built-ins/member calls (`ecrecover`, `send`, `transfer`) convert operands to the types their dialect ops admit.
`solx-slang/src/contract/function/expression/assignment.rs`	Fixes compound assignment to load/operate in the semantic type and write back via `Place::assign` (important for byte element cases).
`solx-mlir/src/ir/value.rs`	Adds a dedicated string/bytes → `!sol.byte` conversion bridge through `!sol.fixedbytes<1>`.
`solx-mlir/tests/lit/value_transfer.sol`	Adds coverage for literal `send`/`transfer` amounts folding narrow then casting to `ui256` before the op.
`solx-mlir/tests/lit/ecrecover.sol`	Adds coverage for folded constants and verifies byte-cast behavior for `ecrecover` operands.
`solx-mlir/tests/lit/compound_assignment_bytes_element.sol`	New test validating correct conversion/storing behavior for compound ops on `bytes` elements.
`solx-mlir/tests/lit/array_string_ops.sol`	Splits FileCheck expectations for solx vs solc output and adds coverage for pushing byte-sized string/hex literals.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

hedgar2017

Thanks!

You may also ask AI to add tests that got green if any!

PavelKopyl · 2026-08-31T13:38:24Z

Thanks!

You may also ask AI to add tests that got green if any!

Yes, those tests exist. I am actually adding them right now in the MLIR test migration PR: #674"

hedgar2017 · 2026-09-03T18:13:14Z

Thanks!
You may also ask AI to add tests that got green if any!

Yes, those tests exist. I am actually adding them right now in the MLIR test migration PR: #674"

My apologies, I meant adding the solx-tester tests that start passing with this PR to the PR description.

hedgar2017

Thanks! Can we merge it into main separately?

PavelKopyl · 2026-09-04T00:00:09Z

Thanks! Can we merge it into main separately?

Sure, rebased onto main. I also added folding constant identifiers at their declared type form Bas's PR-686

github-actions · 2026-09-04T00:06:29Z

Coverage Summary

Crate	Line Coverage	Function Coverage
solx	🟢 83.6%	🔴 20.0%
solx-benchmark-converter	🔴 0.0%	🔴 0.0%
solx-codegen-evm	🔴 25.6%	🔴 13.4%
solx-compiler-downloader	🔴 0.0%	🔴 0.0%
solx-core	🔴 38.6%	🔴 46.9%
solx-dev	🔴 2.4%	🔴 3.0%
solx-evm-assembly	🔴 0.0%	🔴 0.0%
solx-mlir	🟡 58.7%	🟡 55.1%
solx-slang	🔴 23.1%	🔴 34.0%
solx-solc-test-adapter	🔴 1.7%	🔴 2.1%
solx-standard-json	🔴 42.8%	🔴 47.7%
solx-tester	🔴 36.1%	🔴 34.0%
solx-utils	🔴 29.0%	🔴 32.5%
solx-yul	🔴 0.0%	🔴 0.0%
Total	🔴 10.9%	🔴 13.1%

Codecov Report | HTML Report | Workflow Run

PavelKopyl · 2026-09-05T00:02:55Z

Thanks!

You may also ask AI to add tests that got green if any!

Yeah, I added the tests that started passing to the description. I also updated the folding of constant identifiers, as the initial implementation had a bug, and added a new runtime test.

This includes: - aligning 'sol.byte' with 'bytes1' at conversion boundaries; - converting arguments for 'sol.ecrecover', 'sol.send', and 'sol.transfer'; - folding constant identifiers at their declared type

hedgar2017 · 2026-09-05T21:24:26Z

-                let values = scope.positional_arguments(arguments);
-                vec![Value::send(address, values[0], scope)]
+                let amount = scope.converted(
+                    arguments


nit: can be just arguments[0]

hedgar2017 · 2026-09-05T21:24:35Z

-                Value::transfer(address, values[0], scope);
+                let amount = scope.converted(
+                    arguments
+                        .iter()


nit: can be just arguments[0]

hedgar2017 · 2026-09-05T21:28:04Z

+// CHECK:   %[[FD:.*]] = sol.bytes_cast %[[FDC]] : ui128 to !sol.fixedbytes<16>
+// CHECK:   sol.fixed_bytes_index %[[FD]][%{{.*}}] : !sol.fixedbytes<16>, ui256 -> !sol.fixedbytes<1>
+
+bytes16 constant FILE_DIGITS = "EDCBA98765432100";


Please include the Solidity code by following the evaluation_order examples to avoid duplication.

hedgar2017 · 2026-09-05T21:30:56Z

+
+    /// The type a constant folds at: its declared type, except that a `string`/`bytes` constant
+    /// folds in memory.
+    fn constant_folding_type(


Is string the only such exception?

Can it be moved to MlirType as a method?

PavelKopyl force-pushed the kpv-fix-type-conversions branch from 973f2eb to 22c1544 Compare August 27, 2026 14:47

PavelKopyl requested a lite review from Copilot August 27, 2026 14:47

Copilot started reviewing on behalf of PavelKopyl August 27, 2026 14:47 View session

Copilot AI reviewed Aug 27, 2026

View reviewed changes

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

hedgar2017 reviewed Aug 27, 2026

View reviewed changes

Comment thread solx-mlir/src/ir/value.rs Outdated

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

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

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

PavelKopyl force-pushed the kpv-fix-type-conversions branch from 22c1544 to 16b9db7 Compare August 31, 2026 13:46

PavelKopyl mentioned this pull request Sep 2, 2026

fix(slang): constants at their declared type, compound assignment on bytes elements #686

Draft

hedgar2017 approved these changes Sep 3, 2026

View reviewed changes

PavelKopyl force-pushed the kpv-move-mlir-semantic-tests branch from 9fdb02d to 4b4ba04 Compare September 3, 2026 18:29

PavelKopyl force-pushed the kpv-fix-type-conversions branch from 16b9db7 to 73476e7 Compare September 3, 2026 23:56

PavelKopyl force-pushed the kpv-fix-type-conversions branch 3 times, most recently from e6b2bb7 to a92fa3b Compare September 4, 2026 23:54

fix(slang): convert operands to the types their dialect ops admit

3de543d

This includes: - aligning 'sol.byte' with 'bytes1' at conversion boundaries; - converting arguments for 'sol.ecrecover', 'sol.send', and 'sol.transfer'; - folding constant identifiers at their declared type

PavelKopyl force-pushed the kpv-fix-type-conversions branch from a92fa3b to 3de543d Compare September 5, 2026 00:05

hedgar2017 requested changes Sep 5, 2026

View reviewed changes

Uh oh!

Conversation

PavelKopyl commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

hedgar2017 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

PavelKopyl commented Aug 31, 2026

Uh oh!

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

Uh oh!

Uh oh!

hedgar2017 left a comment • 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!

PavelKopyl commented Sep 4, 2026

Uh oh!

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

Uh oh!

Coverage Summary

Uh oh!

PavelKopyl commented Sep 5, 2026

Uh oh!

hedgar2017 Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

hedgar2017 Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

hedgar2017 Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

hedgar2017 Sep 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

PavelKopyl commented Aug 27, 2026 •

edited

Loading

hedgar2017 commented Sep 3, 2026 •

edited

Loading

hedgar2017 left a comment •

edited

Loading

github-actions Bot commented Sep 4, 2026 •

edited

Loading