Skip to content

Conversation

@indietyp
Copy link
Member

@indietyp indietyp commented Jan 3, 2026

🌟 What is the purpose of this PR?

This PR optimizes empty tuple handling in the MIR by simplifying empty tuple aggregates to unit constants and fixing tuple type handling in the body builder macro.

🔍 What does this change?

  • Added From<!> implementation for Operand<'_> to support empty tuples
  • Fixed tuple type handling in the body builder macro to properly handle empty tuples
  • Added support for empty tuples in the rvalue macro
  • Added optimization in the instruction simplifier to convert empty tuple aggregates to unit constants
  • Added a test case to verify empty tuple simplification
  • Sorted feature flags in lib.rs for better organization
  • Updated test outputs to reflect the new optimizations

Pre-Merge Checklist 🚀

🚢 Has this modified a publishable library?

This PR:

  • does not modify any publishable blocks or libraries, or modifications do not need publishing

📜 Does this require a change to the docs?

The changes in this PR:

  • are internal and do not require a docs change

🕸️ Does this require a change to the Turbo Graph?

The changes in this PR:

  • do not affect the execution graph

🛡 What tests cover this?

  • Added a specific test case empty_tuple_to_unit() to verify the optimization
  • Existing test outputs have been updated to reflect the changes

❓ How to test this?

  1. Run the MIR tests to verify that empty tuples are properly optimized
  2. Check that the updated test outputs match the expected behavior

@vercel vercel bot temporarily deployed to Preview – petrinaut January 3, 2026 14:33 Inactive
@cursor
Copy link

cursor bot commented Jan 3, 2026

PR Summary

Introduces unit-aware handling across MIR building and simplification.

  • Add From<!> for Operand and support for () in body! and rvalue! (e.g., tuple;) to construct empty tuples
  • In InstSimplify, fold empty tuple aggregates (AggregateKind::Tuple with no operands) into RValue::Load(Constant::Unit)
  • Update tests: new empty_tuple_to_unit and refreshed inline/pre-inlining snapshots to use () directly (removing temporary unit locals)
  • Tidy lib.rs feature flags (reordered, remove unused)

Written by Cursor Bugbot for commit f0ef5be. This will update automatically on new commits. Configure here.

Copy link
Member Author

indietyp commented Jan 3, 2026

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@augmentcode
Copy link

augmentcode bot commented Jan 3, 2026

🤖 Augment PR Summary

Summary: Canonicalizes empty tuple construction in HashQL MIR and optimizes it into unit (()) constants to reduce redundant aggregates/locals.

Changes:

  • Add `From` for `Operand` so empty-array tuple builders can type-check via `Into<Operand>`.
  • Fix `body!` type macro to handle `()` explicitly and require 1+ elements for non-empty tuple types.
  • Extend `rvalue!` with a `tuple;` form to build empty tuple aggregates.
  • InstSimplify: rewrite empty tuple aggregates (`AggregateKind::Tuple` with no operands) to `Constant::Unit` loads.
  • Add `empty_tuple_to_unit()` coverage and update MIR UI snapshots to reflect the simplified output.
  • Minor tidy: reorder `#![feature(...)]` entries.

Technical Notes: This makes unit values flow as constants earlier, reducing MIR noise and improving downstream pass stability.

🤖 Was this summary useful? React with 👍 or 👎

Copy link

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

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

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

@codecov
Copy link

codecov bot commented Jan 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.14%. Comparing base (7e32c71) to head (f0ef5be).

Additional details and impacted files
@@                           Coverage Diff                           @@
##           bm/be-197-hashql-implement-inlining    #8237      +/-   ##
=======================================================================
+ Coverage                                76.02%   81.14%   +5.12%     
=======================================================================
  Files                                      288      191      -97     
  Lines                                    42866    28548   -14318     
  Branches                                  1059      773     -286     
=======================================================================
- Hits                                     32587    23166    -9421     
+ Misses                                    9916     5122    -4794     
+ Partials                                   363      260     -103     
Flag Coverage Δ
rust.hash-graph-api ?
rust.hashql-compiletest 46.65% <ø> (ø)
rust.hashql-mir ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codspeed-hq
Copy link

codspeed-hq bot commented Jan 3, 2026

Merging this PR will not alter performance

✅ 17 untouched benchmarks


Comparing bm/be-270-hashql-simplify-aggregate-to-unit-const (f0ef5be) with bm/be-197-hashql-implement-inlining (7e32c71)

Open in CodSpeed

@vercel
Copy link

vercel bot commented Jan 15, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Review Updated (UTC)
ds-theme Ignored Ignored Preview Jan 17, 2026 5:56pm
hashdotdesign Ignored Ignored Preview Jan 17, 2026 5:56pm

@indietyp indietyp force-pushed the bm/be-270-hashql-simplify-aggregate-to-unit-const branch from f6a654b to ebf1893 Compare January 15, 2026 15:22
@indietyp indietyp force-pushed the bm/be-197-hashql-implement-inlining branch from e4333c8 to f169485 Compare January 15, 2026 15:22
@indietyp indietyp force-pushed the bm/be-197-hashql-implement-inlining branch from f169485 to d9a54d7 Compare January 16, 2026 10:38
@indietyp indietyp force-pushed the bm/be-270-hashql-simplify-aggregate-to-unit-const branch from ebf1893 to 1046b08 Compare January 16, 2026 10:38
@indietyp indietyp force-pushed the bm/be-197-hashql-implement-inlining branch from d9a54d7 to 20c52d2 Compare January 16, 2026 13:46
@indietyp indietyp force-pushed the bm/be-270-hashql-simplify-aggregate-to-unit-const branch from 1046b08 to 01aa550 Compare January 16, 2026 13:46
@indietyp indietyp force-pushed the bm/be-197-hashql-implement-inlining branch from 20c52d2 to c3200c0 Compare January 17, 2026 16:17
@indietyp indietyp force-pushed the bm/be-270-hashql-simplify-aggregate-to-unit-const branch from 01aa550 to 8002918 Compare January 17, 2026 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/libs Relates to first-party libraries/crates/packages (area) area/tests New or updated tests type/eng > backend Owned by the @backend team

Development

Successfully merging this pull request may close these issues.

3 participants