(bug fix): key syntax node child index by kind for stable node ids - #10190
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
This change is |
PR SummaryMedium Risk Overview
Def cache serialization follows the same shape: child ids read Reviewed by Cursor Bugbot for commit 711e476. Bugbot is set up for automated code reviews on this repo. Configure here. |
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on TomerStarkware).
SyntaxNodeId::Child's per-sibling index is part of a node's salsa identity. #9555 dropped kind from the key used to count child occurrences, so siblings of different kinds sharing (usually empty) key_fields were counted together, contradicting the documented per-(parent, kind, key_fields) invariant. Inserting or removing a different-kind sibling then shifts an unchanged node's index, churning its stable id and defeating salsa early cutoff; the wasted re-execution scales with list size. Restore kind to both the occurrence counter and SyntaxNodeId::Child.
8be9353 to
711e476
Compare
Summary
Restores
kindto both the child-occurrence counter andSyntaxNodeId::Child, so a node's stable id no longer shifts when a different-kind sibling is inserted or removed.Type of change
Please check one:
Why is this change needed?
SyntaxNodeId::Child's per-sibling index is part of a node's salsa identity. #9555 droppedkindfrom the key used to count child occurrences, so siblings of different kinds sharing (usually empty)key_fieldswere counted together — contradicting the documented per-(parent, kind, key_fields)invariant.As a result, inserting or removing a different-kind sibling shifts an unchanged node's index, churning its stable id and defeating salsa early cutoff. The wasted re-execution scales with list size.
What was the behavior or documentation before?
Child occurrences were counted by
(parent, key_fields)only. Siblings of different kinds with the same (empty)key_fieldsshared a counter, so an unchanged node's index — and thus itsSyntaxNodeId— could shift when a different-kind sibling was added or removed, invalidating its salsa cutoff.What is the behavior or documentation after?
kindis part of the occurrence counter and ofSyntaxNodeId::Childagain, matching the documented per-(parent, kind, key_fields)invariant. A node's stable id is now unaffected by insertion/removal of different-kind siblings.Related issue or discussion (if any)
Regression from #9555. Part of a stack with #10189 (regression benchmark) and #10191 (relative offsets, which depends on these stable ids to be effective).
Additional context
Measured on the
ls_reexecstructural-edit scenario added in #10189.