Fix replace *rel = if/case with comprehension branches crashes - #73
Merged
Conversation
…shes
replace *rel = if True then do { x <- *other; yield x } else [] crashed
with 'source_write expects a Relation, got IO'. The desugar pass
unwrap_wrappers_mut did not see through If/Case expressions, so a
branch containing a comprehension compiled as an IO thunk instead of
a relation.
Fix: compile_set_value_expr now recognizes if/case in set-value
position. It records spans of do-blocks in result position — walking
through wrappers, if branches, and case arms, nested arbitrarily —
into a relational_do_spans set. The Do arm of compile_expr consults
that set and uses compile_do (relational) instead of compile_io_do.
2 tests in regress_codegen_fixes.rs: if form (with nested else if and
non-comprehension branch) and case form (both arms at runtime).
cargo test -p knot-compiler --test regress_codegen_fixes: 29 passed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
replace *rel = if True then do { x <- *other; yield x } else []crashed with "source_write expects a Relation, got IO". The desugar passunwrap_wrappers_mutdid not see throughIf/Caseexpressions, so a branch containing a comprehension compiled as an IO thunk instead of a relation.Fix
compile_set_value_exprnow recognizesif/casein set-value position. It records spans of do-blocks in result position — walking through wrappers,ifbranches, andcasearms, nested arbitrarily — into arelational_do_spansset. TheDoarm ofcompile_exprconsults that set and usescompile_do(relational) instead ofcompile_io_do.Tests
2 tests in
regress_codegen_fixes.rs:ifform (with nestedelse ifand non-comprehension branch) andcaseform (both arms exercised at runtime).Verification