[v2] fix(typing): Only convert integers of the same sign implicitly#1828
[v2] fix(typing): Only convert integers of the same sign implicitly#1828teofr wants to merge 2 commits into
Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR fixes Solidity v2 semantic typing by tightening integer implicit-conversion rules so that only integers with the same signedness (intN -> intM and uintN -> uintM) can convert implicitly. This aligns overload resolution (and other implicit-conversion-based typing) with Solidity’s expectation that signed/unsigned conversions require explicit casts.
Changes:
- Restricts integer-to-integer implicit conversion to same-sign widening only.
- Adds a regression test ensuring overload resolution rejects an unsigned argument for a signed parameter.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
crates/solidity-v2/outputs/cargo/semantic/src/types/registry.rs |
Simplifies and tightens integer implicit-conversion logic to require identical signedness and non-narrowing bit widths. |
crates/solidity-v2/outputs/cargo/semantic/src/passes/tests/typing.rs |
Adds a test covering overload-resolution failure when only an unsigned-to-signed implicit conversion would have made a candidate match. |
ggiraldez
left a comment
ggiraldez
left a comment
There was a problem hiding this comment.
This change would only be valid for >= 0.8.1, unfortunately right at the edge of our version support cut. I'd extend the test to check both 0.8.0 and 0.8.1 as well.
| from_bits < to_bits | ||
| } | ||
| } | ||
| ) => from_signed == to_signed && from_bits <= to_bits, |
There was a problem hiding this comment.
This needs a version check. The change in behaviour occurs in 0.8.1. Worth noting that this will also affect validation for explicit casts.
There was a problem hiding this comment.
Thanks! I added the version throughout, and improved the tests
teofr
force-pushed
the
teofr/fix-int-convertibility
branch
from
48cf39a to
0025f37
Compare
|
@ggiraldez good catch, thank you! I'll move this to draft since I think we need at least a moment's thought and maybe a chat on how to make our typing system versioned. |
In v1 we had a |
teofr
force-pushed
the
teofr/fix-int-convertibility
branch
from
0025f37 to
ececba0
Compare
teofr
force-pushed
the
teofr/fix-int-convertibility
branch
from
ececba0 to
ecbf2ea
Compare
Fair point, it was simpler than I thought |
|
Bencher Report| Branch | teofr/fix-int-convertibility |
| Testbed | ci |
⚠️ WARNING: Truncated view!The full continuous benchmarking report exceeds the maximum length allowed on this platform.
🐰 View full continuous benchmarking report in Bencher
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
Fixes https://github.com/NomicFoundation/slang-diagnostics-research/issues/161