Skip to content

Statically forbid promotion nodes in the untyped AST#1545

Merged
WardBrian merged 5 commits intomasterfrom
untyped-promotion-refutation
Sep 10, 2025
Merged

Statically forbid promotion nodes in the untyped AST#1545
WardBrian merged 5 commits intomasterfrom
untyped-promotion-refutation

Conversation

@WardBrian
Copy link
Member

This adds another type parameter to the ast which can be made empty for the untyped ast, allowing us to replace a couple ICEs in the happy path with refutation cases (.) that the compiler proves unreachable.

This has bugged me since adding the Promotion node a while back.
The main annoyance with doing this is that it would add another (broadly useless) argument to @@deriving functions like map_expression (and fold_...). I get around this by re-defining those helper functions towards the bottom of the file without the arguments we don't use, and cleaning up the usage sites.

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

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 9, 2025 19:09
@codecov
Copy link

codecov bot commented Sep 9, 2025

Codecov Report

❌ Patch coverage is 75.00000% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.07%. Comparing base (f1c2143) to head (cf57a79).
⚠️ Report is 16 commits behind head on master.

Files with missing lines Patch % Lines
src/frontend/Ast.ml 56.00% 11 Missing ⚠️
src/analysis_and_optimization/Factor_graph.ml 60.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1545      +/-   ##
==========================================
+ Coverage   90.01%   90.07%   +0.06%     
==========================================
  Files          63       63              
  Lines        9842     9836       -6     
==========================================
+ Hits         8859     8860       +1     
+ Misses        983      976       -7     
Files with missing lines Coverage Δ
...analysis_and_optimization/Debug_data_generation.ml 80.85% <100.00%> (ø)
src/analysis_and_optimization/Memory_patterns.ml 89.31% <100.00%> (ø)
src/analysis_and_optimization/Optimize.ml 92.45% <100.00%> (-0.02%) ⬇️
src/analysis_and_optimization/Pedantic_analysis.ml 95.02% <ø> (-0.12%) ⬇️
src/frontend/Ast_to_Mir.ml 93.89% <100.00%> (ø)
src/frontend/Canonicalize.ml 96.36% <100.00%> (ø)
src/frontend/Deprecation_analysis.ml 75.00% <100.00%> (+2.36%) ⬆️
src/frontend/Info.ml 93.10% <100.00%> (+1.43%) ⬆️
src/frontend/Pretty_printing.ml 92.68% <ø> (+0.32%) ⬆️
src/frontend/Promotion.ml 76.14% <100.00%> (ø)
... and 4 more

... and 1 file with indirect coverage changes

🚀 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.

Comment on lines -70 to +33
ICE.with_exn_message (fun () -> Typechecker.check_program ast)
ICE.with_exn_message (fun () ->
Middle.(
Expr.Helpers.infer_type_of_indexed UnsizedType.UReal
[Index.Single Expr.Helpers.loop_bottom]))
Copy link
Collaborator

Choose a reason for hiding this comment

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

So this test is just looking for some random ICE?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. It's not the most valuable test ever, but it prevents the ICE code from being marked as 0% test coverage

@WardBrian WardBrian requested a review from nhuurre September 9, 2025 20:24
@WardBrian WardBrian merged commit a6b98eb into master Sep 10, 2025
3 checks passed
@WardBrian WardBrian deleted the untyped-promotion-refutation branch September 10, 2025 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants