Repository navigation
sum: preserve iterator validity when moving sum terms - #732
Merged
Merged
Conversation
The move-only Sum constructor advanced the iterator after erasing a summand. This could skip the next term or increment the end iterator, and leave the recorded constant position inconsistent with the terms. Use the iterator returned by erase, and advance the iterator and position only when retaining a summand. Add regression tests for merging a final constant and for constants and zeros interleaved with variables.
Member
Author
|
Formatting check failure should be resolved after #733 |
… tests The interleaved-constants section located the merged constant with find_if and checked only its value, so neither the retained order nor the recorded constant position was verified; with asserts disabled the old loop would have passed it. Check the size and order directly, and verify the recorded position through a later append, which folds into the constant only if the position is right. Add a section with adjacent and trailing zeros: the old loop skipped the summand after an erased one, so the second of two adjacent zeros survived.
evaleev
approved these changes
Oct 26, 2026
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
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.
The move-only
Sumconstructor advanced the iterator after erasing a summand. This could skip the next term or increment the end iterator, and leave the recorded constant position inconsistent with the terms.Use the iterator returned by erase, and advance the iterator and position only when retaining a summand.
Regression tests cover merging a final constant (a segmentation fault without the fix), constants and zeros interleaved with variables (an assertion failure without the fix; the test also checks the retained order and, through a later
append, the recorded constant position), and adjacent and trailing zeros (a zero is retained without the fix).