Conversation
|
This could fit well with some combination of the the projection / place / layout consolidation stuff in #1174, which is intended to support chains of nested of accessors and layout consistency I believe Sean will merge that soon and rebase on top of it |
6ab0595 to
f4ad718
Compare
f4ad718 to
2a99422
Compare
There was a problem hiding this comment.
Pull request overview
This PR implements tuple lowering in MIR, transforming tuples from placeholder tuple(a, b, c) Yul representations into proper memory-allocated structs using alloc + store_field sequences. The implementation follows the same pattern as record lowering, with field offsets computed using the centralized layout API.
Key Changes
- Tuples are now lowered to memory allocations in MIR via
try_lower_tuple, matching record lowering behavior - Pre-lowering infrastructure ensures tuples in subexpressions (call arguments, binary/unary operands) are handled before their parent expressions
- Codegen now panics if a non-unit tuple reaches Yul emission, indicating a MIR lowering bug
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/mir/src/lower/prepass.rs | Updated field expression prepass to use try_lower_field with null-safe insertion |
| crates/mir/src/lower/expr.rs | Added tuple lowering case, field access improvements, and pre_lower_if_needed helper for consistent subexpression pre-lowering across Call, MethodCall, Bin, and Un expressions |
| crates/mir/src/lower/aggregates.rs | Implemented try_lower_tuple for tuple allocation and field stores, added zero-sized type handling with is_zero_sized_ty and synthetic_zero_for_ty helpers, fixed emit_alloc to use expression type instead of alloc return type |
| crates/codegen/src/yul/emitter/expr.rs | Added better value resolution for pre-lowered tuples, panic with diagnostics for un-lowered tuples reaching codegen, zero-sized type check in lower_place_load |
| crates/codegen/tests/fixtures/*.snap | Test snapshots showing generated Yul for tuple construction, field access, and zero-sized aggregates |
| crates/codegen/tests/fixtures/*.fe | New test fixtures for tuple operations and zero-sized types |
| crates/codegen/tests/fixtures/erc20_low_level.* | Updated to use tuple keys in StorageMap: StorageMap<(u256, u256), u256> |
Note: The test snapshots reveal critical bugs in the generated write_key functions for tuple StorageMap keys (infinite recursion and incorrect field offsets), but these are in generated code output rather than the source code changes themselves.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Tuples are now lowered to memory-allocated structs in MIR, matching how records are handled. Instead of the placeholder
tuple(a, b, c)Yul output, tuples now generate properalloc+store_fieldsequences with correctly computed offsets based on element sizes.Key changes:
try_lower_tuplein MIR to emit allocation and field stores for tuple literalsStorageMap<(u256, u256), u256>)Tests: New fixtures for tuple construction, field access, and zero-sized aggregates.