From 2f1a99d34c5af2cf56f018f73b2399b86b0b58fb Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Tue, 8 Sep 2026 02:04:12 -0500 Subject: [PATCH] fix(extraction): index C++ pure virtual methods as nodes (#1727) (#1758) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pure-virtual declarations (`virtual int read(int key) = 0;`) parse as field_declaration, not function_definition, so they minted no method node — calls through an abstract base and cpp-override synthesis had nothing to attach to. Mirror Java interface methods: mint the node (TS + kernel), mark isAbstract, and cover with extraction/e2e/parity fixtures. Co-authored-by: Colby McHenry --- __tests__/extraction.test.ts | 68 ++++++++++++++++++++ __tests__/fixtures/kernel-parity/torture.cpp | 2 + __tests__/frameworks-integration.test.ts | 55 ++++++++++++++++ codegraph-kernel/src/ccpp/mod.rs | 56 +++++++++++++++- src/extraction/languages/c-cpp.ts | 41 +++++++++++- src/extraction/tree-sitter-types.ts | 10 ++- src/extraction/tree-sitter.ts | 14 +++- 7 files changed, 240 insertions(+), 6 deletions(-) diff --git a/__tests__/extraction.test.ts b/__tests__/extraction.test.ts index 23555b3..e9588f0 100644 --- a/__tests__/extraction.test.ts +++ b/__tests__/extraction.test.ts @@ -5868,6 +5868,74 @@ end }); }); +describe('C++ pure-virtual method nodes (#1727)', () => { + // Pure-virtual methods are field_declarations (`virtual int read(int key) = 0;`), + // not function_definitions — they previously minted no method node, so calls + // through an abstract base and cpp-override synthesis had nothing to attach to. + // Java interface methods already get nodes; C++ should behave similarly. + it('indexes Store::read from the issue fixture and records the call', () => { + const code = ` +class Store { +public: + virtual ~Store() {} + virtual int read(int key) = 0; +}; + +class DiskStore : public Store { +public: + int read(int key) override { return key + 1; } +}; + +class MemStore : public Store { +public: + int read(int key) override { return key + 2; } +}; + +int fetch(Store* s, int k) { + return s->read(k); +} +`; + const result = extractFromSource('store.cc', code); + const methods = result.nodes.filter((n) => n.kind === 'method').map((n) => n.qualifiedName); + expect(methods).toContain('Store::read'); + expect(methods).toContain('DiskStore::read'); + expect(methods).toContain('MemStore::read'); + + const baseRead = result.nodes.find((n) => n.qualifiedName === 'Store::read'); + expect(baseRead?.isAbstract).toBe(true); + + // Call site unresolved ref targets the method name (resolver types the receiver). + expect( + result.unresolvedReferences.some( + (r) => r.referenceKind === 'calls' && (r.referenceName === 'read' || r.referenceName.endsWith('.read') || r.referenceName.endsWith('->read') || r.referenceName === 's.read') + ) + ).toBe(true); + }); + + it('indexes pure virtuals with pointer/reference return types and operators', () => { + const code = ` +class Cloneable { +public: + virtual Cloneable* clone() = 0; + virtual const Foo& get() = 0; + virtual Cloneable& operator=(const Cloneable&) = 0; + int notPure(int x); + int data = 0; +}; +`; + const result = extractFromSource('clone.hpp', code); + const methods = result.nodes.filter((n) => n.kind === 'method').map((n) => n.name); + expect(methods).toContain('clone'); + expect(methods).toContain('get'); + expect(methods).toContain('operator='); + // Non-pure prototype and data member must NOT become methods here. + expect(methods).not.toContain('notPure'); + expect(methods).not.toContain('data'); + expect(result.nodes.find((n) => n.name === 'clone')?.isAbstract).toBe(true); + }); + +}); + describe('C++ free-function name extraction', () => { let tempDir: string; let cg: CodeGraph; diff --git a/__tests__/fixtures/kernel-parity/torture.cpp b/__tests__/fixtures/kernel-parity/torture.cpp index aa6e382..b6b160f 100644 --- a/__tests__/fixtures/kernel-parity/torture.cpp +++ b/__tests__/fixtures/kernel-parity/torture.cpp @@ -39,6 +39,8 @@ class Session { public: void open(); virtual ~Session() {} + // #1727 — pure virtual must mint a method node (parity between wasm + kernel). + virtual int read(int key) = 0; }; void Session::open() {} } // namespace app::net diff --git a/__tests__/frameworks-integration.test.ts b/__tests__/frameworks-integration.test.ts index be7e3ae..4e59a73 100644 --- a/__tests__/frameworks-integration.test.ts +++ b/__tests__/frameworks-integration.test.ts @@ -353,6 +353,61 @@ describe('C++ end-to-end — virtual override synthesis', () => { cg.close(); }); + + it('indexes pure-virtual base methods and bridges overrides (#1727)', async () => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-cpp-pure-')); + fs.writeFileSync( + path.join(tmpDir, 'store.cc'), + 'class Store {\n' + + 'public:\n' + + ' virtual ~Store() {}\n' + + ' virtual int read(int key) = 0;\n' + + '};\n' + + 'class DiskStore : public Store {\n' + + 'public:\n' + + ' int read(int key) override { return key + 1; }\n' + + '};\n' + + 'class MemStore : public Store {\n' + + 'public:\n' + + ' int read(int key) override { return key + 2; }\n' + + '};\n' + + 'int fetch(Store* s, int k) {\n' + + ' return s->read(k);\n' + + '}\n' + ); + + const cg = CodeGraph.initSync(tmpDir); + await cg.indexAll(); + + const storeRead = cg + .getNodesByKind('method') + .find((n) => n.qualifiedName === 'Store::read'); + expect(storeRead, 'Store::read pure virtual must be a method node').toBeDefined(); + expect(storeRead!.isAbstract).toBe(true); + + const diskRead = cg + .getNodesByKind('method') + .find((n) => n.qualifiedName === 'DiskStore::read'); + const memRead = cg + .getNodesByKind('method') + .find((n) => n.qualifiedName === 'MemStore::read'); + expect(diskRead).toBeDefined(); + expect(memRead).toBeDefined(); + + // cpp-override synthesis: base pure virtual → each override + const out = cg.getOutgoingEdges(storeRead!.id).filter((e) => e.kind === 'calls'); + const targets = out.map((e) => e.target); + expect(targets).toContain(diskRead!.id); + expect(targets).toContain(memRead!.id); + + // Call through abstract base resolves onto Store::read + const fetch = cg.getNodesByKind('function').find((n) => n.name === 'fetch'); + expect(fetch).toBeDefined(); + const callees = cg.getCallees(fetch!.id).map((c) => c.node.qualifiedName); + expect(callees).toContain('Store::read'); + + cg.close(); + }); }); describe('Java end-to-end — field-injected bean trace (issue #389)', () => { diff --git a/codegraph-kernel/src/ccpp/mod.rs b/codegraph-kernel/src/ccpp/mod.rs index 78877d7..e82715d 100644 --- a/codegraph-kernel/src/ccpp/mod.rs +++ b/codegraph-kernel/src/ccpp/mod.rs @@ -52,6 +52,10 @@ //! per-caller targets (insertion-ordered, branch reassignments accumulate); //! a later bare `k(args)` emits one `calls` ref PER target and suppresses //! the local name. Template args stripped like base-class refs (#1043). +//! - pure-virtual methods (#1727): cpp in-class `virtual T f(...) = 0;` is a +//! `field_declaration` (not `function_definition`); mint a method node so +//! abstract-base calls and cpp-override synthesis have a target. Mirrors +//! TS `methodTypes` + `classifyMethodNode` / `isAbstract`. //! - stack construction (#1035): cpp `declaration` with class-like named //! `type` and an init_declarator whose value is argument_list / //! initializer_list → `instantiates` (most-vexing-parse excluded). @@ -66,7 +70,7 @@ use crate::buffers::{ build_meta, edge_kind_index, node_kind_index, Arena, BoolFlags, EdgeRow, EmitOut, NodeRow, - RefRow, StrRef, Tables, FLAG_IS_EXPORTED, FUNCTION_REF_CODE, NONE, NONE_STR, + RefRow, StrRef, Tables, FLAG_IS_ABSTRACT, FLAG_IS_EXPORTED, FUNCTION_REF_CODE, NONE, NONE_STR, }; use crate::docstring::preceding_docstring; use crate::ids; @@ -289,6 +293,7 @@ struct Extra { signature: Option, visibility: Option, is_exported: Option, + is_abstract: Option, return_type: Option, qualified_name: Option, } @@ -508,6 +513,9 @@ impl<'t> Walker<'t> { if let Some(v) = extra.is_exported { flags.set(FLAG_IS_EXPORTED, v); } + if let Some(v) = extra.is_abstract { + flags.set(FLAG_IS_ABSTRACT, v); + } let name_ref = self.arena.put(name); let qn_ref = self.arena.put(&qualified); let id_ref = self.arena.put(&id); @@ -740,6 +748,36 @@ impl<'t> Walker<'t> { .any(|c| c.kind() == "type_qualifier" && self.text(c) == "const") } + /// `#1727`: C++ pure-virtual method declaration (`virtual int read(int key) = 0;`). + /// tree-sitter-cpp shapes these as `field_declaration` whose declarator unwraps + /// to a `function_declarator`, with the pure-virtual `= 0` as a DIRECT + /// `number_literal` "0" child (default-arg `= 0` lives inside + /// `parameter_declaration` and must not match). + fn is_cpp_pure_virtual_method_decl(&self, node: Node<'_>) -> bool { + if node.kind() != "field_declaration" { + return false; + } + let Some(mut declarator) = node.child_by_field_name("declarator") else { + return false; + }; + while matches!(declarator.kind(), "pointer_declarator" | "reference_declarator") { + let inner = declarator + .child_by_field_name("declarator") + .or_else(|| declarator.named_child(0)); + let Some(inner) = inner else { + return false; + }; + declarator = inner; + } + if declarator.kind() != "function_declarator" { + return false; + } + (0..node.named_child_count()) + .filter_map(|i| node.named_child(i)) + .any(|c| c.kind() == "number_literal" && self.text(c) == "0") + } + + /// cppExtractor.isMisparsedFunction (languages/c-cpp.ts:811). cpp only. fn is_misparsed_function(&self, name: &str, node: Node) -> bool { if self.variant != Variant::Cpp { @@ -830,6 +868,17 @@ impl<'t> Walker<'t> { self.extract_variable(node); self.scan_fn_ref_subtree(node, 0); skip_children = true; + } else if self.variant == Variant::Cpp + && kind == "field_declaration" + && self.inside_class_like() + && self.is_cpp_pure_virtual_method_decl(node) + { + // Pure-virtual methods have no `function_definition` body — mint the + // method node so calls through the abstract base and cpp-override + // synthesis have a target (#1727). Non-pure field_declarations fall + // through to the children walk (data members / prototypes). + self.extract_method(node); + skip_children = true; } else if kind == "preproc_include" { self.extract_import(node); } else if kind == "call_expression" { @@ -914,6 +963,11 @@ impl<'t> Walker<'t> { let extra = Extra { docstring: preceding_docstring(node, self.src), visibility: if self.variant == Variant::Cpp { self.visibility_of(node) } else { None }, + is_abstract: if self.variant == Variant::Cpp && self.is_cpp_pure_virtual_method_decl(node) { + Some(true) + } else { + None + }, return_type: self.return_type_of(node), qualified_name: receiver_type .as_ref() diff --git a/src/extraction/languages/c-cpp.ts b/src/extraction/languages/c-cpp.ts index ad4d806..7dff163 100644 --- a/src/extraction/languages/c-cpp.ts +++ b/src/extraction/languages/c-cpp.ts @@ -166,6 +166,35 @@ export function stripCppTemplateArgs(name: string): string { return out.trim(); } + +/** + * Is this C++ `field_declaration` a pure-virtual method (`virtual int read(int key) = 0;`)? + * tree-sitter-cpp shapes those as a field_declaration whose declarator unwraps to a + * `function_declarator`, with the pure-virtual `= 0` as a DIRECT `number_literal` "0" + * child of the field_declaration (default-arg `= 0` lives inside parameter_declaration + * and must not match). Bodiless method prototypes (`int foo();`) and data members + * (`int x = 0;`) are excluded — prototypes usually have an out-of-line definition that + * already mints the method node; pure virtuals never do (#1727). + */ +export function isCppPureVirtualMethodDecl(node: SyntaxNode): boolean { + if (node.type !== 'field_declaration') return false; + let declarator: SyntaxNode | null = getChildByField(node, 'declarator'); + if (!declarator) return false; + while ( + declarator.type === 'pointer_declarator' || + declarator.type === 'reference_declarator' + ) { + const inner: SyntaxNode | null = + getChildByField(declarator, 'declarator') || declarator.namedChild(0); + if (!inner) return false; + declarator = inner; + } + if (declarator.type !== 'function_declarator') return false; + return node.namedChildren.some( + (c: SyntaxNode) => c.type === 'number_literal' && c.text === '0' + ); +} + /** * A function/method's return type lives in the `function_definition`'s `type` * field (`Metrics& Metrics::instance()` → `Metrics`). Constructors, destructors, @@ -1607,7 +1636,17 @@ export const cppExtractor: LanguageExtractor = { // get picked as the blast-radius representative over — the single real // definition, exactly as bodiless struct/enum specifiers are already skipped. (#1093) skipBodilessClass: true, - methodTypes: ['function_definition'], + // `function_definition` covers inline / out-of-line bodies; `field_declaration` + // covers pure-virtual methods (`virtual int read(int key) = 0;`), which have no + // body and would otherwise mint no node — so calls through the abstract base and + // cpp-override synthesis had nothing to attach to (#1727). classifyMethodNode + // keeps ordinary data members / prototypes on the children-walk path. + methodTypes: ['function_definition', 'field_declaration'], + classifyMethodNode: (node) => { + if (node.type !== 'field_declaration') return 'method'; + return isCppPureVirtualMethodDecl(node) ? 'method' : 'skip'; + }, + isAbstract: (node) => (isCppPureVirtualMethodDecl(node) ? true : undefined), interfaceTypes: [], structTypes: ['struct_specifier'], // C++ unions additionally carry member functions, which extract through the diff --git a/src/extraction/tree-sitter-types.ts b/src/extraction/tree-sitter-types.ts index 6808895..5a4f8d3 100644 --- a/src/extraction/tree-sitter-types.ts +++ b/src/extraction/tree-sitter-types.ts @@ -160,6 +160,8 @@ export interface LanguageExtractor { isAsync?: (node: SyntaxNode) => boolean; /** Check if node is static */ isStatic?: (node: SyntaxNode) => boolean; + /** Check if a method/class is abstract (C++ pure virtual, Java abstract, …). Return true to set; undefined/false leaves the flag unset. */ + isAbstract?: (node: SyntaxNode) => boolean | undefined; /** Check if variable declaration is a constant (const vs let/var) */ isConst?: (node: SyntaxNode) => boolean; /** @@ -221,9 +223,13 @@ export interface LanguageExtractor { * both callable and data members (#808): TS/JS class FIELDS * (`public_field_definition` / `field_definition`) are methods only when * their value is callable (`onClick = () => {}`); a plain field - * (`public fonts: Fonts;`, `count = 0`) is a property. Default: 'method'. + * (`public fonts: Fonts;`, `count = 0`) is a property. C++ also lists + * `field_declaration` in methodTypes so pure-virtual methods (`= 0`) can + * mint nodes (#1727); non-callable field_declarations return `'skip'` so + * the walker still descends (data-member initializers keep their call + * edges). Default: 'method'. */ - classifyMethodNode?: (node: SyntaxNode) => 'method' | 'property'; + classifyMethodNode?: (node: SyntaxNode) => 'method' | 'property' | 'skip'; /** * Resolve the body node for a function/method/class when it's not a child field. diff --git a/src/extraction/tree-sitter.ts b/src/extraction/tree-sitter.ts index 2d87412..2e1ae83 100644 --- a/src/extraction/tree-sitter.ts +++ b/src/extraction/tree-sitter.ts @@ -1041,8 +1041,14 @@ export class TreeSitterExtractor { else if (this.extractor.methodTypes.includes(nodeType)) { // TS/JS class fields parse as a methodTypes node; only function-valued // fields are methods — a plain field (`public fonts: Fonts;`) is a - // property (#808). classifyMethodNode is absent for other languages. - if (this.extractor.classifyMethodNode?.(node) === 'property') { + // property (#808). C++ lists `field_declaration` so pure-virtual methods + // mint nodes (#1727); non-callable ones return 'skip' and fall through to + // the children walk. classifyMethodNode is absent for other languages. + const methodClass = this.extractor.classifyMethodNode?.(node) ?? 'method'; + if (methodClass === 'skip') { + // Not a method — leave skipChildren false so data-member initializers + // still contribute call/instantiation edges under the enclosing class. + } else if (methodClass === 'property') { const propNode = this.extractProperty(node); // Walk the initializer so its calls/instantiations attribute to the // property (`history = createHistory()` → history calls @@ -1798,6 +1804,9 @@ export class TreeSitterExtractor { const visibility = this.extractor.getVisibility?.(node); const isAsync = this.extractor.isAsync?.(node); const isStatic = this.extractor.isStatic?.(node); + // Only persist abstract when true — a false return must not mint `isAbstract: false` + // on every ordinary method (breaks kernel↔wasm parity JSON equality). + const isAbstract = this.extractor.isAbstract?.(node) ? true : undefined; const returnType = this.extractor.getReturnType?.(node, this.source); const extraProps: Partial = { docstring, @@ -1805,6 +1814,7 @@ export class TreeSitterExtractor { visibility, isAsync, isStatic, + isAbstract, returnType, }; if (receiverType) {