Skip to content

Conversation

@indietyp
Copy link
Member

🌟 What is the purpose of this PR?

Introduces the body! macro, a declarative DSL for constructing MIR bodies in tests. This replaces the verbose fluent builder API as the primary approach, making tests significantly more readable and concise (~940 net lines removed).

🔍 What does this change?

  • Adds new body! macro with IR-like syntax for MIR construction
  • Refactors builder module from single file into organized submodules (base.rs, body.rs, rvalue.rs, switch.rs, etc.)
  • Converts all MIR pass tests (both transform and analysis) from fluent builder to body! macro
  • Updates documentation to prioritize body! macro, with fluent builder as secondary option
  • Updates skill documentation with new MIR builder guide

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

📹 Demo

Before (fluent builder):

scaffold!(heap, interner, builder);
let env = Environment::new(&heap);
let int_ty = TypeBuilder::synthetic(&env).integer();
let x = builder.local("x", int_ty);
let const_1 = builder.const_int(1);
let bb0 = builder.reserve_block([]);
builder.build_block(bb0)
    .assign_place(x, |rv| rv.load(const_1))
    .ret(x);
let body = builder.finish(0, int_ty);

After (body! macro):

let heap = Heap::new();
let interner = Interner::new(&heap);
let env = Environment::new(&heap);

let body = body!(interner, env; fn@0/0 -> Int {
    decl x: Int;
    bb0() {
        x = load 1;
        return x;
    }
});

@cursor
Copy link

cursor bot commented Dec 30, 2025

PR Summary

Introduces a declarative DSL for MIR test construction and reorganizes the builder, significantly simplifying tests.

  • Add body! macro with IR-like syntax for building MIR bodies in tests
  • Refactor builder from a single file to modular structure (base.rs, body.rs, rvalue.rs, switch.rs, etc.); remove old builder.rs
  • Migrate MIR analysis/transform tests to body! (e.g., callgraph, data_dependency, liveness, administrative_reduction)
  • Update docs: expand mir-builder-guide.md, add mir-fluent-builder.md, and revise SKILL to prefer body! with examples and caveats
  • Test harness snippets updated to capture Changed in snapshots; minor lint/format adjustments

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

Copy link
Member Author

indietyp commented Dec 30, 2025

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.

@vercel vercel bot temporarily deployed to Preview – petrinaut December 30, 2025 23:00 Inactive
@codspeed-hq
Copy link

codspeed-hq bot commented Dec 30, 2025

CodSpeed Performance Report

Merging #8229 will not alter performance

Comparing bm/be-263-hashql-implement-mir-builder-macro (9530312) with bm/be-261-hashql-mir-administrative-reduction-pass (c680dde)

Summary

✅ 14 untouched
🗄️ 3 archived benchmarks run1

Footnotes

  1. 3 benchmarks were run, but are now archived. If they were deleted in another branch, consider rebasing to remove them from the report. Instead if they were added back, click here to restore them.

@codecov
Copy link

codecov bot commented Dec 30, 2025

Codecov Report

❌ Patch coverage is 91.59562% with 69 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.78%. Comparing base (c680dde) to head (9530312).

Files with missing lines Patch % Lines
libs/@local/hashql/mir/src/builder/rvalue.rs 83.44% 25 Missing ⚠️
libs/@local/hashql/mir/src/builder/basic_block.rs 78.30% 23 Missing ⚠️
libs/@local/hashql/mir/src/builder/place.rs 86.79% 7 Missing ⚠️
libs/@local/hashql/mir/src/builder/operand.rs 75.00% 6 Missing ⚠️
libs/@local/hashql/mir/src/builder/base.rs 85.71% 5 Missing ⚠️
libs/@local/hashql/mir/src/builder/switch.rs 88.88% 3 Missing ⚠️
Additional details and impacted files
@@                                  Coverage Diff                                   @@
##           bm/be-261-hashql-mir-administrative-reduction-pass    #8229      +/-   ##
======================================================================================
- Coverage                                               60.36%   59.78%   -0.58%     
======================================================================================
  Files                                                    1060     1066       +6     
  Lines                                                  107742   106191    -1551     
  Branches                                                 4478     4478              
======================================================================================
- Hits                                                    65039    63488    -1551     
  Misses                                                  41963    41963              
  Partials                                                  740      740              
Flag Coverage Δ
rust.hashql-compiletest 46.65% <ø> (ø)
rust.hashql-mir 87.22% <91.59%> (-1.91%) ⬇️

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.

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. 2 suggestions posted.

Fix All in Augment

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/infra Relates to version control, CI, CD or IaC (area) 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.

2 participants