Skip to content

Align code generation of local decl dimensions with top-level decls - #1691

Merged
WardBrian merged 2 commits into
masterfrom
fix/local-type-dim-codegen
Sep 2, 2026
Merged

Align code generation of local decl dimensions with top-level decls#1691
WardBrian merged 2 commits into
masterfrom
fix/local-type-dim-codegen

Conversation

@WardBrian

Copy link
Copy Markdown
Member

Fixes the issue @nhuurre noted here where a local variable size check could be invalid in the presence of RNGs.

In particular, we had two independent code paths in the AST->MIR lowering, one for top-level (possibly transformed) decls, and one for local decls. They both did their own independent things in terms of avoiding generating redundant checks, how the names were constructed, etc.

By combining them, these differences go away and it naturally fixes the above issue by generating a temporary and using that as the size in the check and declaration.

Submission Checklist

  • Run unit tests
  • Documentation
    • If a user-facing facing change was made, the documentation PR is here:
    • OR, no user-facing changes were made

Release notes

Improved code-generation of local variable dimensions.

Copyright and Licensing

By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)

@WardBrian
WardBrian requested a review from nhuurre September 2, 2026 19:04
@WardBrian WardBrian changed the title Fix/local type dim codegen Align code generation of local decl dimensions with top-level decls Sep 2, 2026
@WardBrian WardBrian mentioned this pull request Sep 2, 2026
3 tasks
@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.37%. Comparing base (689d502) to head (cc323ae).

Files with missing lines Patch % Lines
src/frontend/Ast_to_Mir.ml 92.45% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1691      +/-   ##
==========================================
- Coverage   92.42%   92.37%   -0.05%     
==========================================
  Files          69       69              
  Lines       10202    10188      -14     
==========================================
- Hits         9429     9411      -18     
- Misses        773      777       +4     
Files with missing lines Coverage Δ
src/middle/Utils.ml 97.36% <100.00%> (+0.39%) ⬆️
src/stan_math_backend/Transform_Mir.ml 95.55% <100.00%> (ø)
src/frontend/Ast_to_Mir.ml 95.00% <92.45%> (-1.00%) ⬇️
🚀 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.

@WardBrian
WardBrian merged commit 3079376 into master Sep 2, 2026
3 checks passed
@WardBrian
WardBrian deleted the fix/local-type-dim-codegen branch September 2, 2026 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants