Skip to content

Skip skip-probe deep clone unless unused-skip reporting is on - #8563

Closed
TomasVotruba wants to merge 1 commit into
mainfrom
tv-drop-clonenode-skip-probe
Closed

TomasVotruba wants to merge 1 commit into
mainfrom
tv-drop-clonenode-skip-probe

Conversation

@TomasVotruba

@TomasVotruba TomasVotruba commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

The class/path skip-probe deep-cloned the node, ran the rule on the clone, and marked the skip used if it would have changed something. That result only feeds the unused-skips report, which is off by default (UnusedSkipResolver::shouldSkipReporting).

So on a normal run the deep clone was pure overhead - its result was thrown away. This gates the probe behind Option::REPORT_UNUSED_SKIPS:

  • reporting off (default): no clone, no probe refactor - just skip
  • reporting on: clone + probe kept, so unused-skip detection stays correct

First attempt dropped the clone entirely and probed the real node, but that mutated skipped nodes and broke skip tests - the clone is load-bearing when reporting is on.

@TomasVotruba
TomasVotruba force-pushed the tv-drop-clonenode-skip-probe branch from 07bcd81 to 3aa3f90 Compare October 9, 2026 13:51
@TomasVotruba TomasVotruba changed the title Drop node cloning in skip-probe of RectorRunner Skip skip-probe deep clone unless unused-skip reporting is on Oct 9, 2026
@TomasVotruba
TomasVotruba force-pushed the tv-drop-clonenode-skip-probe branch from 3aa3f90 to 52da11c Compare October 9, 2026 13:54
@TomasVotruba
TomasVotruba deleted the tv-drop-clonenode-skip-probe branch October 9, 2026 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant