Fix cpp class_start forward-declaration false positive (closes #2011) - #2019
Merged
Merged
Conversation
The tree-sitter walker fix in the previous commit surfaced this as a real CI
regression in tests/tree_sitter_accuracy_baseline_cpp.json (found_classes'
denominator, real_classes, correctly dropped once the walker no longer counted
forward declarations -- exposing that GitGalaxy's own class_start had the
identical bug, previously masked by both sides agreeing on the same wrong
answer). Fixed rather than left filed, since the regression made the gap
impossible to defer further.
A naive copy of C's existing _CLASS_START_REQUIRES_BODY_ANCHOR flat lookahead
(stops at the first {/;/,/)/=) is unsafe for cpp: C++ multiple inheritance
(`class Foo : public A, public B {`) hits the lookahead's own comma stop-char
before the real `{`, falsely excluding a legitimate class definition --
confirmed via direct regex testing before this fix was written.
Fixed instead with _cpp_class_has_body(), a depth-aware scanner (paren/
bracket/angle-bracket, same style as the existing _dart_scan_terminator/
_count_top_level_args helpers) that correctly walks through an inheritance
clause's own top-level commas and template args before checking for a real
body opener. Verified via 11 hand-built regression cases (multi-inheritance,
templated bases, the pre-existing type-use-in-declarator-list protection),
the full 122-test cpp extraction gauntlet, and crucible_check.py against the
full ~80-repo corpus (zero golden-master diff -- confirmed via a direct DB
query that this is because the golden master's audit report only exposes the
raw class_start signal count, not the named-class list this fix touches, not
because the fix is a no-op).
Regenerates the cpp tree-sitter-accuracy baseline (found_classes/real_classes
171->65, both now matching cleanly -- class recall 87.1% -> 100.0%) and the
tri-comparison ledger/chart/docs to reflect the shape no longer reproducing.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
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.
Summary
Fixes #2011, found during the cpp tri-comparison-ledger-sweep merged in #2015. That
PR fixed the comparison tooling's own tree-sitter walker (which had the identical
lang == "c"-only forward-declaration guard gap as GitGalaxy's owndetector.py),but deliberately left the production
detector.pyfix filed rather than patchedinline, since a naive copy of C's existing flat lookahead regex is unsafe for cpp.
That decision turned out to have a real, immediate consequence: once the tree-sitter
walker stopped agreeing with GitGalaxy's forward-declaration false positives,
mainwas left with a standing inconsistency —
tests/tree_sitter_accuracy_baseline_cpp.jsonstill commits the old, both-sides-wrong numbers (
real_classes: 171,found_classes: 149), while the actual tree-sitter ground truth now computes 65. Any PR touching cpptooling would inherit this as a
tree-sitter-accuracy-auditCI failure through nofault of its own (confirmed: this is what happened while finishing out PR #2015's own
review). This PR closes that gap by actually fixing the underlying bug rather than
leaving main in a broken state.
The bug: GitGalaxy's own
class_startregex counted a bare forward declaration(
class AudioStreamPreviewGenerator;,godot/editor_node.h:68and 83 more of the sameshape in the same file) as if it were a real class definition —
_CLASS_START_REQUIRES_BODY_ANCHORindetector.py, the exact mechanism that alreadyguards against this for C, was never extended to cpp.
Why not just copy C's fix: C's existing lookahead (
re.search(r"^[^\{;,)=]{0,200}?([\{;,)=])", ...))stops at the first
{/;/,/)/=. C++ multiple inheritance(
class Foo : public A, public B {) hits the comma stop-char before the real{,falsely excluding a legitimate multi-inheritance class definition — confirmed via
direct regex testing before this fix was written specifically to avoid that
regression.
The fix:
_cpp_class_has_body(), a depth-aware scanner (paren/bracket/angle-brackettracking, same style as this file's existing
_dart_scan_terminator/_count_top_level_argshelpers) that correctly walks through an inheritance clause'sown top-level commas and template args before checking for a real body opener. A
top-level
,before any inheritance-list:has been seen is still treated as anon-definition signal, preserving the pre-existing protection against a type-use in a
declarator list (
struct Foo *a, *b;) or a function parameter default(
void f(struct Foo* p = nullptr)).Also regenerates
tests/tree_sitter_accuracy_baseline_cpp.json(class recall 87.1% →100.0%, both
found_classesandextra_classesnow correctly reflect zero forward-declaration false positives) and the tri-comparison ledger/chart/
docs/language_status/ cpp.mdto record the fix.Test plan
_cpp_class_has_body(forward decl, plaindefinition, single/multi inheritance, templated bases, type-use-in-declarator-
list, function-parameter-default,
finalspecifier) — all passtests/core_engine/test_detector.py(141 tests) + C's own extraction gauntlet (128 tests, confirmed unaffected)
python tests/ruff_audit.py --ci/mypy_audit.py --ci— no new findingsruff format --checkcleantests/tools/crucible_check.py(full-precision + zero-dependency) against thefull ~80-repo corpus — zero golden-master diff, confirmed legitimate (not a
blind spot) via a direct DB query showing the named-class list this fix touches
dropped from ~95 to 11 for
editor_node.hspecificallytree_sitter_accuracy_audit.py --lang cpp --ci— passes cleanly against theregenerated baseline
🤖 Generated with Claude Code