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 MIR-construction DSL and migrates tests to it.

  • Adds builder::body! macro (with projections, operands, statements, and terminators) plus lightweight helper macros (bb!, operand!, rvalue!, switch!) for concise CFG/test authoring
  • Refactors monolithic builder.rs into structured modules: base.rs, body.rs, basic_block.rs, operand.rs, place.rs, rvalue.rs, switch.rs, mod.rs; removes old builder.rs
  • Rewrites MIR pass tests (analysis: callgraph, data_dependency, liveness; transform: administrative_reduction) from fluent API to the new macro; minor harness tweaks (e.g., snapshot formatting, Changed capture comment)
  • Updates docs: mir-builder-guide.md to macro-first, adds mir-fluent-builder.md (advanced cases), refreshes SKILL guide with “Missing Macro Features” note and macro examples

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

Copy link
Member Author

indietyp commented Dec 30, 2025

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

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 30, 2025

Merging this PR will not alter performance

✅ 14 untouched benchmarks
🗄️ 3 archived benchmarks run1


Comparing bm/be-263-hashql-implement-mir-builder-macro (ed471cd) with main (fd458f2)

Open in CodSpeed

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 87.22%. Comparing base (c4139ea) to head (3c76437).

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                                               59.24%   87.22%   +27.98%     
=======================================================================================
  Files                                                    1191       69     -1122     
  Lines                                                  113321     8853   -104468     
  Branches                                                 4981      252     -4729     
=======================================================================================
- Hits                                                    67134     7722    -59412     
+ Misses                                                  45411     1034    -44377     
+ Partials                                                  776       97      -679     
Flag Coverage Δ
apps.hash-ai-worker-py ?
apps.hash-ai-worker-ts ?
apps.hash-api ?
backend-integration-tests ?
blockprotocol.type-system ?
deer ?
error-stack ?
local.claude-hooks ?
local.harpc-client ?
local.hash-backend-utils ?
local.hash-graph-sdk ?
local.hash-isomorphic-utils ?
local.hash-subgraph ?
rust.antsi ?
rust.deer ?
rust.error-stack ?
rust.harpc-codec ?
rust.harpc-net ?
rust.harpc-tower ?
rust.harpc-types ?
rust.harpc-wire-protocol ?
rust.hash-codec ?
rust.hash-graph-api ?
rust.hash-graph-authorization ?
rust.hash-graph-postgres-store ?
rust.hash-graph-store ?
rust.hash-graph-temporal-versioning ?
rust.hash-graph-types ?
rust.hash-graph-validation ?
rust.hashql-ast ?
rust.hashql-compiletest ?
rust.hashql-core ?
rust.hashql-diagnostics ?
rust.hashql-eval ?
rust.hashql-hir ?
rust.hashql-mir 87.22% <91.59%> (-1.02%) ⬇️
rust.hashql-syntax-jexpr ?
rust.sarif ?
sarif ?
tests.hash-backend-integration ?
unit-tests ?

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.

@indietyp indietyp force-pushed the bm/be-263-hashql-implement-mir-builder-macro branch from 4156c74 to ed471cd Compare January 16, 2026 15:24
@graphite-app graphite-app bot changed the base branch from graphite-base/8229 to main January 16, 2026 15:24
@graphite-app
Copy link
Contributor

graphite-app bot commented Jan 16, 2026

Merge activity

  • Jan 16, 3:24 PM UTC: Graphite rebased this pull request, because this pull request is set to merge when ready.

@indietyp indietyp added this pull request to the merge queue Jan 16, 2026
Merged via the queue into main with commit d451c18 Jan 16, 2026
53 of 74 checks passed
@indietyp indietyp deleted the bm/be-263-hashql-implement-mir-builder-macro branch January 16, 2026 15:39
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.

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.

///
/// | Syntax | Description |
/// | ------ | ----------- |
/// | `let x;` | `StorageLive(x)` |
Copy link

Choose a reason for hiding this comment

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

The table says let x; / drop x; map to StorageLive(x) / StorageDead(x), but bb! expands these to storage_live($name) / storage_dead($name) which take a Local (while body! binds locals as Place). This makes the documented syntax look like it won’t type-check unless callers use something like x.local.

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎

(@type $types:ident; Int) => {
$types.integer()
};
(@type $types:ident; ($($sub:tt),*)) => {
Copy link

Choose a reason for hiding this comment

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

In body!(@type ...), the tuple arm (@type $types; ($($sub:tt),*)) appears before the struct arm (@type $types; ($($name:ident: $sub:tt),*)), so a struct type like (a: Int, b: Bool) will match the tuple arm first and then fail to expand. This seems to contradict the docs that advertise struct-type support in body!.

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎

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.

3 participants