Fix MultiValueDomain reductions#2541
Merged
Merged
Conversation
PartialProductDomain's constructor invokes the overridable reduce() before subclasses initialize their fields. MultiValueDomain.reduce read this.reductions, which was still undefined at that point, so every construction threw a TypeError - making MultiValueDomain and MultiValueStateDomain unusable, including their reductions feature. Lift the reductions into PartialProductDomain so the field is set before reduce is called, preserving the reduce-after-clone semantics. Add regression tests.
OliverGerstl
approved these changes
Jun 10, 2026
OliverGerstl
left a comment
OliverGerstl
left a comment
Collaborator
There was a problem hiding this comment.
Thank you for identifying and fixing the issue.
The solution is fine for me, but I am also not so happy about the generated tests.
I think we don't need them for now, as we will probably have more meaningful tests as soon as the multi-value domain is actually used :)
Member
|
This pull request is included in v2.10.8 (see Release v2.10.8 (MultiValue fixes, refined input sources, fixed linter rules)). |
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
3 participants
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.
Lift the default reductions processing from the
MultiValueDomainto thePartialProductDomain.@OliverGerstl Feel free to give feedback or edit the PR yourself as you see fit :)
Not sure whether we want to keep the new test cases and their structure, because they deviate from the domain tests we currently have.
The fact that we have two ways to define reductions (via either the constructor parameter or overriding the
reducefunction) adds complexity, but I see why we would want to have both.Closes #2540
Disclaimer: Identified and fixed using OpenCode and Claude Opus 4.8, with some manual minor clean-up by hand