Conversation
A nested beginEco suspended the enclosing journal on the ECO stack, but commitEco/undoEco of the inner ECO left no active journal. Edits made after the inner ECO ended were not recorded, so the outer undoEco did not revert them. The stack now records whether an entry was suspended by a nested beginEco (resumed after the inner ECO ends) or stopped by endEco (stays stopped). Fixes The-OpenROAD-Project-private/OpenROAD#3836 Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces support for nested ECOs (Engineering Change Order) in OpenDB by refactoring the journal stack to track suspended journals and adding a helper function to resume them. It also includes comprehensive unit tests for nested undo and commit behaviors. The review feedback suggests adding assertions in resumeSuspendedEco to verify state validity before resuming a journal, and setting block->journal_ to nullptr immediately after pushing to the stack in beginEco to ensure exception safety in case of allocation failures.
Member
Author
|
@codex review |
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
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
A nested
beginEcosets the enclosing journal aside on the ECO stack. When the inner ECO finished,commitEco/undoEcoleft no journal active, so edits made after that point were not recorded and the outerundoEcocould not revert them. This affected both nested undo (the reported case) and nested commit.beginEcoset it aside orendEcostopped it.endEcostays stopped, which is howrmpand thereplace_hier_mod_undotests use it.TestJournalcases: nested undo then edit, nested commit then edit, andendEcofollowed by a nested commit.Fixes The-OpenROAD-Project-private/OpenROAD#3836