The Pattern-Matching Bug: remove check_partial
#13153
Open
Conversation
This file contains 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
This was referenced May 7, 2024
Open
gasche
force-pushed
the
matching-bug-remove-check_partial
branch
2 times, most recently
from
718bf95
to
a4e732d
Compare
The [check_partial] heuristics are a coarse-grained approach degrade matching totality information in certain cases -- when clauses contain both reads of mutable fields and either guards or forcing of lazy patterns. It is not quite correct (it is not enough to prevent ocaml#7241), and is not sufficient conceptually for OCaml Multicore. In a Multicore world, other domains may race to update values concurrently, so we cannot assume that only a fixed set of matching constructions may result in side-effects. This heuristic is subsumed by the recent changes to: - make context information accurate by binding mutable fields eagerly - make totality information accurate by tracking the mutability of the current position and can now be retired.
gasche
force-pushed
the
matching-bug-remove-check_partial
branch
from
a4e732d
to
ccdbcd2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
(This PR sits on top of #13152, and is thus best review by looking at the last commit only.)
This is a non-obvious code simplification PR after #7241 is fixed. Before #7241 was reported and worked on, there already was a heuristic named
check_partialto degrade pattern-matching compilation in some cases involving side-effects during matching. Now that #7241 is fixed by other changes in the pattern-matching compiler, the heuristic is not necessary anymore and complicates the code, it can be removed.The testsuite
testsuite/tests/match-side-effects/check_partialwas introduced in #13121 to test various cases where thecheck_partialheuristic would fire. The code generated for these tests is not modified by the present PR, demonstrating that, on those tests, removingcheck_partialdoes not change the compiler behavior.Details
The
check_partialheuristics was a coarse-grained approach to degrade matching totality information in certain cases -- when clauses contain both reads of mutable fields and either guards or forcing of lazy patterns.This heuristic is unfortunately not quite correct (it is not enough to prevent #7241), and is not sufficient conceptually for OCaml Multicore. In a Multicore world, other domains may race to update values concurrently, so we cannot assume that only a fixed set of matching constructions may result in side-effects.