[v2] fix common and mobile type mechanism#1792
Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR refines Solidity v2 typing/unification to better match solc semantics by separating “common type via implicit convertibility” from “mobile-type promotion,” and by applying those rules differently for array literals vs conditional expressions.
Changes:
- Add
TypeRegistry::common_type(implicit-convertibility-only) andTypeRegistry::mobile_type_id, and reworkcommon_mobile_typeto delay mobile-typing for non-first array elements. - Update conditional-expression typing to mobile-type both branches before unifying (matching solc’s stricter behavior vs array literals).
- Add
Type::can_be_array_elementand enforce it for array literals; expand and refactor typing tests with a reusable semantic pipeline setup.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/solidity-v2/outputs/cargo/semantic/src/types/registry.rs | Introduces common_type/mobile_type_id and adjusts array-literal unification to preserve literal-specific conversions where appropriate. |
| crates/solidity-v2/outputs/cargo/semantic/src/types/mod.rs | Updates mobile_type behavior and adds can_be_array_element for array-literal validity checks. |
| crates/solidity-v2/outputs/cargo/semantic/src/passes/p4_resolve_references/visitor.rs | Changes conditional-expression typing to mobile-type both branches before unifying. |
| crates/solidity-v2/outputs/cargo/semantic/src/passes/p4_resolve_references/typing.rs | Enforces can_be_array_element when typing array literals. |
| crates/solidity-v2/outputs/cargo/semantic/src/passes/tests/typing.rs | Refactors test setup and adds coverage for array-vs-conditional unification differences and mapping/array-element constraints. |
ggiraldez
left a comment
There was a problem hiding this comment.
I left some suggestions. I think the semantic of type unification behind the two cases (conditional expressions and array literals) are different enough that it makes sense to separate the code.
The tests refactor may be out of scope for this PR, but since you're already making heavy modifications here, it may make sense to do it altogether. Happy to push to a different PR if you prefer.
|
Also, after working on #1793 to expose |
34f549c to
7e679a2
Compare
7e679a2 to
9673de4
Compare
ggiraldez
left a comment
There was a problem hiding this comment.
Looks great, thank you!
Co-authored-by: Gustavo Giráldez <ggiraldez@manas.tech>
…ing.rs Co-authored-by: Gustavo Giráldez <ggiraldez@manas.tech>
9b2d81b to
918189d
Compare
Various fixes around mobile and common type calculation:
mobile_typeis now more general, checking if a type is array compatible is done latermobilebefore looking for the common type, this matchessolcand disallows certain cases liketrue ? bytes32(0) : 0typingtests to consolidate the pipeline, I think we still need snapshot testing infrastructure for valid typing scenarios. Also added an extra kind of test,try_type_of_function_body_expressioncommon_mobile_type