fix(cpp): keep symbols for nested C++ types and C++/CLI sources (#2876) - #2886
fix(cpp): keep symbols for nested C++ types and C++/CLI sources (#2876)#2886rajarshidattapy wants to merge 4 commits into
Conversation
…hify-Labs#2876) Two independent gaps in the C++ extractor, both hit by .NET interop wrappers (a 141-method C++/CLI class extracted 12 symbols). Nested types: a nested `class Inner { … };` is a `field_declaration` whose `type` field IS the class_specifier. The C++ member-variable branch in engine.py consumed that node and returned before the walk could descend, so Inner and everything it declares vanished — silently, with no parse error. Walk the class_specifier with parent_class_nid threaded through, which routes it into the engine's existing nested-type handling and gives it a `contains` edge from the enclosing type. The declarator loop still runs, so `class Inner { } inst;` yields both the type and the member. C++/CLI: tree-sitter-cpp implements none of `ref class`, `Type^`, `Type%`, `gcnew` or `[assembly:…]`, and because the ERROR lands on the type header the whole class body dissolves and recovery invents junk symbols. Normalize those spellings to the nearest standard C++ ones before parsing. The rewrite is byte-length preserving — dropped tokens are overwritten with spaces, `gcnew` becomes `new` plus padding — so every offset, line and column still points at the same place in the file on disk. It engages only on files carrying a C++/CLI marker, so plain C/C++/CUDA parses byte-for-byte as before, and `^`/`%` are rewritten only in their attached suffix spelling so XOR and modulo keep their meaning. Both reproducers from the issue now give the expected labels: `['nested.h', 'Outer', 'Inner', '.Method()']` and `['cli.h', 'Wrapper', '.Init()', '.Name()', 'String']`.
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Adds C++/CLI support to extract_cpp via a new _normalize_cpp_cli that rewrites ref/value class, Type^/Type%, gcnew, and [assembly:…] spellings into standard C++ using byte-length-preserving substitutions (spaces/padding) so offsets and source locations stay intact, gated behind a marker regex so plain C/C++ is untouched. Fixes nested-type extraction in _extract_generic: a field_declaration whose type field is a class body is now walked as a class (getting its contains edge) instead of being consumed by the member-variable branch and silently dropped. Adds tests/test_cpp_nested_and_cli.py covering nested types, type+instance declarations, CLI body survival, offset preservation, and operator (^/%) safety.
Worth a look
- Multiline C++/CLI attributes destroy line offsets —
graphify/extract.py:2008· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1772 functions depend on the 418 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 472 callers, 42 callees - new:
_rebuild_code()— 98 callers, 51 callees - new:
_extract_generic()— 18 callers, 24 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
extract_objc()— 27 callers, 9 callees - new:
extract_js()— 80 callers, 3 callees - new:
dispatch_command()— 2 callers, 117 callees - new:
_get_extractor()— 26 callers, 6 callees - …and 32 more — each is listed as a finding
Verification — 1772 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1629 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify extract\_cpp.
The verifier did not have enough to check extract\_cpp, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 39 more finding(s) on lines outside this diff (see the check run).
| return _CPP_CLI_ATTR_RE.sub(blank, out) | ||
|
|
||
|
|
||
| def extract_cpp(path: Path) -> dict: |
There was a problem hiding this comment.
extract_cpp()
24 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Review catch: byte length alone was not enough. `_CPP_CLI_ATTR_RE` matches with re.S, so `[assembly:AssemblyVersion(\n "1.0"\n)]` — ordinary formatting — was overwritten with spaces including its newlines. The `\s+` in the class-header pattern could swallow one the same way when the access specifier sits on its own line. The byte count stayed right and the file still parsed, but two source lines merged into one, so every symbol below the attribute reported a line number that was too low. That contradicts the property the rewrite is supposed to guarantee. Blank everything except CR and LF, so line and column stay stable along with the offset. Two tests: reported source_location for a class and method below a multiline attribute, and newline positions unchanged for a class header split across lines.
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 1 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Adds C++/CLI normalization to extract_cpp via a new _normalize_cpp_cli helper that rewrites ref class, Type^/%, gcnew, and [assembly:…] into standard C++ while preserving byte offsets and line breaks, so tree-sitter no longer dissolves the class body into junk nodes (#2876). Fixes _extract_generic to descend into nested type declarations (class Inner { … }; inside a class body) instead of consuming them as member variables, giving them a contains edge from the enclosing type. Adds tests/test_cpp_nested_and_cli.py covering nested types, CLI rewrites, operator preservation, and line-number stability.
Worth a look
- _CPP_CLI_SUFFIX_RE rewrites '^'/'%' as XOR/mod inside a CLI file when attached to an identifier token —
graphify/extract.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1777 functions depend on the 423 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 472 callers, 42 callees - new:
_rebuild_code()— 98 callers, 51 callees - new:
_extract_generic()— 18 callers, 24 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
extract_objc()— 27 callers, 9 callees - new:
extract_js()— 80 callers, 3 callees - new:
dispatch_command()— 2 callers, 117 callees - new:
_get_extractor()— 26 callers, 6 callees - …and 32 more — each is listed as a finding
Verification — 1777 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1634 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify extract\_cpp.
The verifier did not have enough to check extract\_cpp, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 39 more finding(s) on lines outside this diff (see the check run).
| return _CPP_CLI_ATTR_RE.sub(_blank_keeping_newlines, out) | ||
|
|
||
|
|
||
| def extract_cpp(path: Path) -> dict: |
There was a problem hiding this comment.
extract_cpp()
25 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
Review catch, and it reproduces. Attachment to the preceding token does not
separate a handle from an operator — `String^ s` and `a^ b` are lexically
identical, and `a% b` / `hash^ mask` are attached too. The old rule keyed on
attachment alone, so inside a CLI-marked file:
int m = a% b; -> int m = a b;
int x = hash^ mask; -> int x = hash mask;
Both are syntax errors. On a file mixing CLI types with that formatting the
whole thing went from clean to `parse_errors` at the first such line.
Split the rule in two, both requiring more than attachment:
1. followed by a token that cannot begin an operand — `f(String^, int)`,
`List<String^>`, `Object^;`. `a^,` is not valid C++, so no arithmetic is
at stake. `*` and `&` are dropped from that set, since `a^*p` and `a^&b`
are valid XOR and `String^*` is rarer than they are.
2. a type-shaped left side followed by a declarator — qualified names, a
closing generic bracket, PascalCase per .NET convention, or a primitive
value type. That covers `System::String^ s`, `List<int>^ items`,
`DataTable^ t` and `int% n` while leaving `count% 2` and `hash^ mask` as
arithmetic.
The substitution captures and re-emits the type, so it stays byte-length
preserving.
Residual, and deliberate: an uppercase local doing XOR (`Mask^ value`) is
still rewritten, and a lowercase-named handle type is missed. Both are rare
against .NET naming, and the trade now favours arithmetic, which is common.
Tests: the mixed CLI-plus-arithmetic file end to end (no parse errors, all
four methods extracted), seven arithmetic forms that must survive, and six
CLI type suffixes that must still be rewritten. Six fail without the change.
There was a problem hiding this comment.
Graphify reviewed this change.
Worth a look — the grounded gate found no coupling regressions or blocking issues, but 2 advisory finding(s) below merit a look before merge.
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Adds C++/CLI support to extract_cpp via a new _normalize_cpp_cli pass that rewrites ref class, Type^/Type%, gcnew, and [assembly:…] spellings to standard C++ in a byte-length- and line-preserving way before parsing. Fixes nested C++ types (class Inner { … }; inside a class body) being silently dropped in _extract_generic by walking the nested class_specifier and emitting a contains edge from the enclosing type. Includes tests/test_cpp_nested_and_cli.py covering nested types, CLI normalization, offset/line preservation, and that plain-C++ operators aren't rewritten.
Worth a look
- _CPP_CLI_SUFFIX_DECL_RE can corrupt XOR expressions with type-like LHS —
graphify/extract.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
- _CPP_CLI_SUFFIX_DECL_RE substitution is not byte-length preserving when whitespace differs —
graphify/extract.py· Escalate · medium- agreed by 2 of 2 members but NOT verified (no proof, no reproducing execution) — consensus is not a verdict; needs human review Execution auto-disposal is off for this run; enable it (with sandbox isolation) to have Graphify try to confirm or refute this automatically.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1781 functions depend on the 427 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 472 callers, 42 callees - new:
_rebuild_code()— 98 callers, 51 callees - new:
_extract_generic()— 18 callers, 24 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
extract_objc()— 27 callers, 9 callees - new:
extract_js()— 80 callers, 3 callees - new:
dispatch_command()— 2 callers, 117 callees - new:
_get_extractor()— 26 callers, 6 callees - …and 32 more — each is listed as a finding
Verification — 1781 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1638 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify extract\_cpp.
The verifier did not have enough to check extract\_cpp, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 39 more finding(s) on lines outside this diff (see the check run).
| return _CPP_CLI_ATTR_RE.sub(_blank_keeping_newlines, out) | ||
|
|
||
|
|
||
| def extract_cpp(path: Path) -> dict: |
There was a problem hiding this comment.
extract_cpp()
26 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
There was a problem hiding this comment.
Graphify reviewed this change.
Looks safe to merge — no coupling regressions and no blocking issues, checked against the code graph (not a self-assessment).
Formal verification. No changes could be formally verified in this run.
Graphify review — findings
Adds C++/CLI support to extract_cpp via a new _normalize_cpp_cli pass that rewrites ref class, Type^/Type%, gcnew, and [assembly:…] spellings into standard C++, preserving byte offsets and line breaks so reported source locations stay accurate. Fixes nested-type dropping in _extract_generic by walking a class_specifier in a field_declaration's type field instead of consuming it in the member-variable branch. Adds tests/test_cpp_nested_and_cli.py covering nested types, CLI normalization, byte/line preservation, and that operators and SCREAMING_CASE constants stay arithmetic.
No blocking issues surfaced. 6 lower-confidence candidates did not survive cross-model review.
Analysis details — impact, health, verification
Impact & health
Graphify review
Impact — 1783 functions depend on the 429 functions this change touches.
Health — this change adds coupling hotspots:
- new:
extract()— 472 callers, 42 callees - new:
_rebuild_code()— 98 callers, 51 callees - new:
_extract_generic()— 18 callers, 24 callees - new:
extract_xaml()— 19 callers, 17 callees - new:
extract_objc()— 27 callers, 9 callees - new:
extract_js()— 80 callers, 3 callees - new:
dispatch_command()— 2 callers, 117 callees - new:
_get_extractor()— 26 callers, 6 callees - …and 32 more — each is listed as a finding
Verification — 1783 functions in the blast radius were not formally verified this run (proofs are advisory here).
Gate & verification
graphify gate
PASS — objectively clean (no health regressions, tests not run — proofs not run this pass (advisory)). Grounded, not self-assessed.
Advisory (not blocking):
- verification_scope: 1640 function(s) in the blast radius were not formally verified this run
Formal verification
Could not verify: Could not verify extract\_cpp.
The verifier did not have enough to check extract\_cpp, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
Could not verify: Could not verify \_extract\_generic.
The verifier did not have enough to check \_extract\_generic, so it is saying so rather than guessing. No false assurance is the whole point.
Guarantee: No guarantee either way, this is an honest abstention, not a pass.
Note: Reason: parameter `path` is annotated `Path` — outside the synthesizable primitive/collection set
· 1 grounded finding(s) anchored inline below; 39 more finding(s) on lines outside this diff (see the check run).
| return _CPP_CLI_ATTR_RE.sub(_blank_keeping_newlines, out) | ||
|
|
||
|
|
||
| def extract_cpp(path: Path) -> dict: |
There was a problem hiding this comment.
extract_cpp()
27 callers depend on it (afferent coupling).
Grounded coupling-delta finding (deterministic), not an LLM guess.
|
Shipped in v0.9.48 via authorship-preserving cherry-pick, with a note recording nested enums as a known out-of-scope gap. Thanks @rajarshidattapy! Release: https://github.com/Graphify-Labs/graphify/releases/tag/v0.9.48 |
Fixes #2876.
Two independent gaps in the C++ extractor, both reproduced verbatim from the issue.
1. Nested C++ types were dropped silently
A nested type is a
field_declarationwhosetypefield is theclass_specifier. The C++ member-variable branch inextractors/engine.pyconsumed that node and returned before the walk could descend, so the nested type and everything it declares vanished — no parse error, no warning.['nested.h', 'Outer']['nested.h', 'Outer', 'Inner', '.Method()']The fix walks the
class_specifierwithparent_class_nidthreaded through, which routes it into the engine's existing nested-type handling —Innergets acontainsedge fromOuterrather than from the file, exactly as for the other languages. The declarator loop still runs afterwards, soclass Inner { } inst;yields both the type and the member.2. C++/CLI took the whole class body down with it
tree-sitter-cpp implements none of
ref class,Type^,Type%,gcnewor[assembly:…]. Because the ERROR lands on the type header, everything inside dissolves and recovery invents symbols that were never written:['cli.h', 'Wrapper()', 'public']— plusparse_errorson line 2['cli.h', 'Wrapper', '.Init()', '.Name()', 'String'], no parse errors_normalize_cpp_cli()maps each C++/CLI spelling onto the nearest standard C++ one before parsing, via_extract_generic's existingsource_overridehook (the same one Vue SFCs use). Three properties keep it safe:gcnewbecomesnewplus padding. Every offset, line and column still points at the same place in the file on disk, so reported source locations stay accurate.ref/value class,interface class,gcnew,[assembly:). Plain C/C++/CUDA is parsed byte-for-byte as before —^/%alone are not markers, since they are the ordinary XOR and modulo operators.^/%are rewritten only in their attached suffix spelling (String^ s,int% n), so a spaceda ^ b/a % bin the same file keeps its meaning. Covered by a test.Tests
tests/test_cpp_nested_and_cli.py— both issue reproducers, theclass Inner { } inst;case,Outer contains Inner, byte-offset preservation (newline positions unchanged), plain C++ left untouched, and operators surviving inside a CLI file. The existing 82 C++/ObjC tests pass unchanged.Not included
The issue's closing "unrelated, one line" note about
--forcenot invalidatinggraphify-out/cache/ast/<version>. Worth noting for anyone hitting it: that directory is namespaced by graphify version (cache_dir()→v{_EXTRACTOR_VERSION}-s{_AST_CACHE_SCHEMA}), so a released extractor change invalidates it automatically. The stale-cache problem is specific to running a locally patched extractor at an unchanged version. Happy to add a--forceAST-cache bypass in a separate PR if that is wanted.