Skip to content

Conversation

@indietyp
Copy link
Member

@indietyp indietyp commented Nov 30, 2025

🌟 What is the purpose of this PR?

Fix SSA repair to handle reassignment of variables in the same block correctly. This PR addresses an issue where the SSA repair pass wasn't properly handling cases where a variable is reassigned within the same block before being used as a block parameter.

🔍 What does this change?

  • Added a new Existing variant to FindDefFromTop enum to handle cases where a reaching definition is already present in block parameters
  • Modified SsaViolationRepair to check if a local declaration already exists for block parameters before creating a new one
  • Updated the visitor implementations to ensure right-hand side expressions are visited before left-hand side in assignments
  • Added a new test case reassign_rodeo to verify the fix works correctly

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 new test case reassign_rodeo that specifically tests the fixed behavior
  • Existing tests continue to pass with the updated implementation

@indietyp indietyp force-pushed the bm/be-223-hashql-fix-order-of-operations-in-ssa-rename-pass branch from 52a95f7 to 9bcc313 Compare November 30, 2025 22:31
@indietyp indietyp force-pushed the bm/be-221-hashql-dead-block-elimination branch from 9058cd8 to 1b3b01b Compare November 30, 2025 22:31
@vercel vercel bot temporarily deployed to Preview – petrinaut November 30, 2025 22:31 Inactive
@codecov
Copy link

codecov bot commented Nov 30, 2025

Codecov Report

❌ Patch coverage is 97.05882% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.83%. Comparing base (1b3b01b) to head (aa93afe).

Files with missing lines Patch % Lines
...al/hashql/mir/src/pass/transform/ssa_repair/mod.rs 94.44% 2 Missing ⚠️
Additional details and impacted files
@@                  Coverage Diff                   @@
##           graphite-base/8136    #8136      +/-   ##
======================================================
+ Coverage               57.80%   57.83%   +0.02%     
======================================================
  Files                    1166     1166              
  Lines                  109121   109179      +58     
  Branches                 4963     4965       +2     
======================================================
+ Hits                    63081    63139      +58     
- Misses                  45306    45307       +1     
+ Partials                  734      733       -1     
Flag Coverage Δ
rust.hashql-compiletest 49.36% <ø> (ø)
rust.hashql-mir 83.78% <97.05%> (+0.18%) ⬆️

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.

@graphite-app graphite-app bot changed the base branch from bm/be-221-hashql-dead-block-elimination to graphite-base/8136 December 1, 2025 10:44
@indietyp indietyp force-pushed the bm/be-223-hashql-fix-order-of-operations-in-ssa-rename-pass branch from aa93afe to 8c8fd79 Compare December 1, 2025 14:18
@vercel vercel bot temporarily deployed to Preview – petrinaut December 1, 2025 14:18 Inactive
@graphite-app graphite-app bot changed the base branch from graphite-base/8136 to main December 1, 2025 14:18
@graphite-app
Copy link
Contributor

graphite-app bot commented Dec 1, 2025

Merge activity

  • Dec 1, 2:18 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 Dec 1, 2025
Merged via the queue into main with commit f5ae2ac Dec 1, 2025
42 of 63 checks passed
@indietyp indietyp deleted the bm/be-223-hashql-fix-order-of-operations-in-ssa-rename-pass branch December 1, 2025 14:37
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