Skip to content

Tuple lowering#1188

Merged
sbillig merged 1 commit intomasterfrom
tuple_lowering
Dec 22, 2025
Merged

Tuple lowering#1188
sbillig merged 1 commit intomasterfrom
tuple_lowering

Conversation

@cburgdorf
Copy link
Collaborator

@cburgdorf cburgdorf commented Dec 17, 2025

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 proper alloc + store_field sequences with correctly computed offsets based on element sizes.

Key changes:

  • Added try_lower_tuple in MIR to emit allocation and field stores for tuple literals
  • Tuples in call arguments and other subexpressions are pre-lowered before the parent expression
  • Codegen now panics if a non-unit tuple reaches Yul emission (indicates MIR bug)
  • Enabled tuple keys in StorageMap (e.g., StorageMap<(u256, u256), u256>)

Tests: New fixtures for tuple construction, field access, and zero-sized aggregates.

@micahscopes
Copy link
Collaborator

micahscopes commented Dec 17, 2025

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

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@sbillig sbillig merged commit 55750f1 into master Dec 22, 2025
11 checks passed
@sbillig sbillig deleted the tuple_lowering branch December 22, 2025 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants