From d698f0be1e2e5d6784fed379ac31cd894ded72cb Mon Sep 17 00:00:00 2001 From: Andrey Kumanyaev Date: Thu, 13 Aug 2026 09:32:56 +0200 Subject: [PATCH] fix(cpp): extract out-of-line member definitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A definition written outside its declaring scope — `void ns::Cls::run() {}` in the .cpp for a header's class — declares itself with a qualified_identifier, a shape no function_definition pattern admitted. The definition emitted no node at all, and every call in its body went with it: a deferred call needs an enclosing function range to attach to. In a translation unit whose class lives in a header, that is the whole file. Admit a qualified declarator in each function_definition pattern and read the qualifier structurally. The trailing segment decides the shape: a type owner yields a method keyed on the owner (member_of the class when this file declares it, so `A::run` and `B::run` no longer collide), and an all-namespace qualifier yields a free function carrying the qualifier as scope_ns. Destructors, operators, pointer and reference returns, and out-of-line template members reach the same walk. Folds cppQualifiedCallName and lastIdentifier into that one walk — a qualified callee and a qualified declarator are the same shape. lastIdentifier scanned direct children only, so once a name nested it returned the first segment rather than the last (`ns::Cls::bar` gave `ns`). --- internal/parser/languages/cpp.go | 69 +++-- internal/parser/languages/cpp_out_of_line.go | 188 ++++++++++++++ .../parser/languages/cpp_out_of_line_test.go | 239 ++++++++++++++++++ 3 files changed, 456 insertions(+), 40 deletions(-) create mode 100644 internal/parser/languages/cpp_out_of_line.go create mode 100644 internal/parser/languages/cpp_out_of_line_test.go diff --git a/internal/parser/languages/cpp.go b/internal/parser/languages/cpp.go index 0b4e13a0..d46006b5 100644 --- a/internal/parser/languages/cpp.go +++ b/internal/parser/languages/cpp.go @@ -36,25 +36,31 @@ const qCppAll = ` (enum_specifier name: (type_identifier) @enum.name) @enum.def + ; A definition written outside its declaring scope — void ns::Cls::run() + ; in the .cpp for a header's class — declares itself with a + ; qualified_identifier, so admitting only a bare identifier here dropped the + ; whole definition: no node, and every call in the body went with it for + ; want of an enclosing function range. emitOutOfLineMember reads the + ; qualifier off the node; the capture stays the plain-name fast path. (function_definition declarator: (function_declarator - declarator: (identifier) @func.name)) @func.def + declarator: [(identifier) (qualified_identifier)] @func.name)) @func.def (function_definition declarator: (pointer_declarator declarator: (function_declarator - declarator: (identifier) @func.name))) @func.def + declarator: [(identifier) (qualified_identifier)] @func.name))) @func.def (function_definition declarator: (pointer_declarator declarator: (pointer_declarator declarator: (function_declarator - declarator: (identifier) @func.name)))) @func.def + declarator: [(identifier) (qualified_identifier)] @func.name)))) @func.def (function_definition declarator: (reference_declarator (function_declarator - declarator: (identifier) @func.name))) @func.def + declarator: [(identifier) (qualified_identifier)] @func.name))) @func.def (template_declaration (function_definition) @tmplfn.inner) @tmplfn.def @@ -156,7 +162,7 @@ func (e *CppExtractor) Extract(filePath string, src []byte) (*parser.ExtractionR switch { case m.Captures["ns.def"] != nil: - e.emitNamespace(m, filePath, fileID, result) + e.emitNamespace(m, filePath, fileID, result, seen) case m.Captures["class.def"] != nil: e.emitClass(m, filePath, fileID, src, result, seen) @@ -194,7 +200,7 @@ func (e *CppExtractor) Extract(filePath string, src []byte) (*parser.ExtractionR case m.Captures["callq.expr"] != nil: expr := m.Captures["callq.expr"] - name := cppQualifiedCallName(m.Captures["callq.name"].Node, src) + _, name := cppQualifiedParts(m.Captures["callq.name"].Node, src) if name == "" { return } @@ -270,10 +276,13 @@ func (e *CppExtractor) Extract(filePath string, src []byte) (*parser.ExtractionR // --- Per-match emit helpers ----------------------------------------- -func (e *CppExtractor) emitNamespace(m parser.QueryResult, filePath, fileID string, result *parser.ExtractionResult) { +func (e *CppExtractor) emitNamespace(m parser.QueryResult, filePath, fileID string, result *parser.ExtractionResult, seen map[string]bool) { name := m.Captures["ns.name"].Text def := m.Captures["ns.def"] id := filePath + "::" + name + // Records that this name is a namespace, so an out-of-line definition + // qualified with it is read as a free function rather than a member. + seen[cppNamespaceMarker(filePath, name)] = true result.Nodes = append(result.Nodes, &graph.Node{ ID: id, Kind: graph.KindPackage, Name: name, FilePath: filePath, StartLine: def.StartLine + 1, EndLine: def.EndLine + 1, @@ -296,6 +305,7 @@ func (e *CppExtractor) emitClass(m parser.QueryResult, filePath, fileID string, return } seen[classID] = true + seen[cppTypeMarker(filePath, className)] = true meta := map[string]any{"type_flavor": "class"} if ns := enclosingCppNamespace(def.Node, src); ns != "" { meta["scope_ns"] = ns @@ -425,6 +435,7 @@ func (e *CppExtractor) emitStruct(m parser.QueryResult, filePath, fileID string, return } seen[id] = true + seen[cppTypeMarker(filePath, name)] = true result.Nodes = append(result.Nodes, &graph.Node{ ID: id, Kind: graph.KindType, Name: name, FilePath: filePath, StartLine: def.StartLine + 1, EndLine: def.EndLine + 1, @@ -502,6 +513,9 @@ func (e *CppExtractor) emitFunction(m parser.QueryResult, filePath, fileID strin if cppTemplateOwnsDefinition(def.Node) { return } + if e.emitOutOfLineMember(def.Node, startLine, def.EndLine+1, filePath, fileID, src, result, seen) { + return + } e.emitFreeFunction(m.Captures["func.name"].Text, def.Node, startLine, def.EndLine+1, filePath, fileID, src, result, seen) } @@ -519,6 +533,12 @@ func (e *CppExtractor) emitTemplateFunction(m parser.QueryResult, filePath, file if def == nil || inner == nil || inner.Node == nil || cppInsideTypeBody(inner.Node) { return } + // `template void Holder::put(T) {…}` is an out-of-line + // member, not a free template function — its span is still the + // template_declaration's, so the header is covered either way. + if e.emitOutOfLineMember(inner.Node, def.StartLine+1, def.EndLine+1, filePath, fileID, src, result, seen) { + return + } e.emitFreeFunction(cppFreeTemplateFuncName(inner.Node, src), inner.Node, def.StartLine+1, def.EndLine+1, filePath, fileID, src, result, seen) } @@ -649,7 +669,8 @@ func extractFuncName(funcNode *sitter.Node, src []byte) string { case "identifier", "field_identifier", "destructor_name", "operator_name": return gc.Content(src) case "qualified_identifier": - return lastIdentifier(gc, src) + _, name := cppQualifiedParts(gc, src) + return name } } return "" @@ -693,38 +714,6 @@ func cppInnerDeclarator(decl *sitter.Node) *sitter.Node { return nil } -// cppQualifiedCallName follows a qualified call's name field until it reaches -// the trailing identifier. tree-sitter-cpp nests another qualified_identifier -// there for every additional :: segment; a templated tail adds one -// template_function wrapper. Unknown shapes return no evidence rather than -// inventing a callee name. -func cppQualifiedCallName(node *sitter.Node, src []byte) string { - for depth := 0; node != nil && depth < 64; depth++ { - switch node.Type() { - case "identifier", "field_identifier": - return node.Content(src) - case "qualified_identifier", "template_function": - node = node.ChildByFieldName("name") - default: - return "" - } - } - return "" -} - -// lastIdentifier extracts the last identifier from a qualified_identifier. -func lastIdentifier(node *sitter.Node, src []byte) string { - name := "" - for i, _nc := 0, int(node.NamedChildCount()); i < _nc; i++ { - child := node.NamedChild(i) - switch child.Type() { - case "identifier", "field_identifier", "destructor_name": - name = child.Content(src) - } - } - return name -} - // enclosingCppNamespace walks node up through the tree-sitter AST // looking for namespace_definition ancestors and concatenates their // names with "::" (so `namespace a { namespace b { void foo() {} } }` diff --git a/internal/parser/languages/cpp_out_of_line.go b/internal/parser/languages/cpp_out_of_line.go new file mode 100644 index 00000000..44842907 --- /dev/null +++ b/internal/parser/languages/cpp_out_of_line.go @@ -0,0 +1,188 @@ +package languages + +import ( + "fmt" + "strings" + + "github.com/zzet/gortex/internal/graph" + "github.com/zzet/gortex/internal/parser" + sitter "github.com/zzet/gortex/internal/parser/tsitter" +) + +// Out-of-line definitions — `void ns::Cls::run() {…}` written in a .cpp whose +// class lives in a header — carry a qualified_identifier declarator. The +// function_definition patterns only admitted a bare identifier, so the whole +// definition matched nothing: no method node, and every call in its body was +// dropped with it (a deferred call needs an enclosing function range to attach +// to). In an idiomatic translation unit that is most of the file. + +// emitOutOfLineMember emits a definition whose declarator names its own scope. +// The trailing segment of the qualifier decides the shape: a type owner yields +// a method (`Cls::run` → `file.cpp::Cls.run`, member_of the class when it is +// declared in this file), and an all-namespace qualifier yields a free function +// carrying the qualifier as scope_ns (`ns::helper`). Reports whether the node +// was handled, so the caller can fall through to its regular emission path. +func (e *CppExtractor) emitOutOfLineMember(fnNode *sitter.Node, startLine, endLine int, filePath, fileID string, src []byte, result *parser.ExtractionResult, seen map[string]bool) bool { + decl := cppQualifiedDeclarator(fnNode) + if decl == nil { + return false + } + scopes, name := cppQualifiedParts(decl, src) + if name == "" || len(scopes) == 0 { + return false + } + + owner := scopes[len(scopes)-1] + nsParts := scopes[:len(scopes)-1] + if !cppScopeNamesType(owner, filePath, seen) { + // Every segment is a namespace — `void ns::helper() {}` declares a + // namespace-scope function, not a member. + owner, nsParts = "", scopes + } + + id := filePath + "::" + name + if owner != "" { + id = filePath + "::" + owner + "." + name + } + if seen[id] { + id += "_L" + fmt.Sprint(startLine) + } + if seen[id] { + return true + } + seen[id] = true + + meta := map[string]any{} + if ns := cppJoinNamespace(enclosingCppNamespace(fnNode, src), nsParts); ns != "" { + meta["scope_ns"] = ns + } + if rt := cppReturnType(fnNode, src); rt != "" { + meta["return_type"] = rt + } + stampCppSignature(meta, fnNode, src) + + kind := graph.KindFunction + if owner != "" { + kind = graph.KindMethod + meta["receiver"] = owner + meta["scope_class"] = owner + } + result.Nodes = append(result.Nodes, &graph.Node{ + ID: id, Kind: kind, Name: name, + FilePath: filePath, StartLine: startLine, EndLine: endLine, + Language: "cpp", Meta: meta, + }) + result.Edges = append(result.Edges, &graph.Edge{ + From: fileID, To: id, Kind: graph.EdgeDefines, FilePath: filePath, Line: startLine, + }) + // The owning class is usually in a header, and this extractor never invents + // a node for a type it has not seen — link the member only when the class + // was emitted from this same file. + if owner != "" && seen[cppTypeMarker(filePath, owner)] { + result.Edges = append(result.Edges, &graph.Edge{ + From: id, To: filePath + "::" + owner, Kind: graph.EdgeMemberOf, + FilePath: filePath, Line: startLine, + }) + } + return true +} + +// cppQualifiedDeclarator returns the qualified_identifier a function definition +// declares itself with, or nil when the declarator is a plain name. Mirrors the +// declarator dispatch in extractFuncName so the same pointer / reference / +// parenthesized wrappers are peeled first. +func cppQualifiedDeclarator(fnNode *sitter.Node) *sitter.Node { + if fnNode == nil { + return nil + } + fd := cppFunctionDeclarator(fnNode.ChildByFieldName("declarator")) + if fd == nil { + return nil + } + for i, _nc := 0, int(fd.NamedChildCount()); i < _nc; i++ { + switch c := fd.NamedChild(i); c.Type() { + case "qualified_identifier": + return c + case "identifier", "field_identifier", "destructor_name", "operator_name": + return nil + } + } + return nil +} + +// cppQualifiedParts splits a qualified_identifier into its scope segments and +// its trailing name: `ns::Cls::run` → ["ns", "Cls"], "run". tree-sitter-cpp +// nests another qualified_identifier in the name field for every extra :: +// segment, so the walk is structural rather than depth-fixed; a templated tail +// (`Cls::run`) adds one template_function wrapper, and a templated scope +// (`Holder::put`) carries the segment inside a template_type. An unknown +// shape yields no name rather than an invented one. +// +// Both sides of a :: name need this walk — the callee of `a::b::c()` and the +// declarator of `void a::b::c() {}` are the same shape — so it is the one +// place either is decoded. Callers that only want the name discard the scopes. +func cppQualifiedParts(node *sitter.Node, src []byte) ([]string, string) { + var scopes []string + for depth := 0; node != nil && depth < 64; depth++ { + switch node.Type() { + case "qualified_identifier": + if s := node.ChildByFieldName("scope"); s != nil { + if seg := cppScopeSegment(s, src); seg != "" { + scopes = append(scopes, seg) + } + } + node = node.ChildByFieldName("name") + case "template_function", "template_method": + node = node.ChildByFieldName("name") + case "identifier", "field_identifier", "destructor_name", "operator_name": + return scopes, node.Content(src) + default: + return scopes, "" + } + } + return scopes, "" +} + +// cppScopeSegment renders one :: segment of a qualifier, dropping the template +// arguments a generic owner carries (`Holder` → `Holder`) so the segment +// matches the type node's name. +func cppScopeSegment(node *sitter.Node, src []byte) string { + if node.Type() == "template_type" { + if inner := node.ChildByFieldName("name"); inner != nil { + return strings.TrimSpace(inner.Content(src)) + } + } + return strings.TrimSpace(node.Content(src)) +} + +// cppScopeNamesType reports whether a qualifier segment names a type rather +// than a namespace. A class or struct emitted from this file is proof; nothing +// else is, because the declaring header is a different translation unit — so +// the same Capitalized-name heuristic the reference-form pass uses decides. +func cppScopeNamesType(seg, filePath string, seen map[string]bool) bool { + if seen[cppTypeMarker(filePath, seg)] { + return true + } + if seen[cppNamespaceMarker(filePath, seg)] { + return false + } + return isCapitalizedCppType(seg) +} + +// cppTypeMarker / cppNamespaceMarker key the seen-set entries that record what +// this file declared, following the "_method_L" marker convention the +// class-body walk already uses to talk to the function_definition dispatch. +func cppTypeMarker(filePath, name string) string { return filePath + "::_type_" + name } + +func cppNamespaceMarker(filePath, name string) string { return filePath + "::_ns_" + name } + +// cppJoinNamespace concatenates the lexically enclosing namespace with the +// namespace segments the declarator spells out — `namespace a { void b::f(){} }` +// puts f in a::b. +func cppJoinNamespace(enclosing string, parts []string) string { + all := parts + if enclosing != "" { + all = append([]string{enclosing}, parts...) + } + return strings.Join(all, "::") +} diff --git a/internal/parser/languages/cpp_out_of_line_test.go b/internal/parser/languages/cpp_out_of_line_test.go new file mode 100644 index 00000000..f21cf183 --- /dev/null +++ b/internal/parser/languages/cpp_out_of_line_test.go @@ -0,0 +1,239 @@ +package languages + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/zzet/gortex/internal/graph" + "github.com/zzet/gortex/internal/parser" +) + +// cppNodeByID indexes an extraction result so a test can assert on one node. +func cppNodeByID(res *parser.ExtractionResult) map[string]*graph.Node { + byID := map[string]*graph.Node{} + for _, n := range res.Nodes { + byID[n.ID] = n + } + return byID +} + +// The declaring class lives in a header, so the translation unit holds nothing +// but out-of-line definitions — the shape of an idiomatic .cpp. Every one of +// them used to extract as no node at all, which also silently cost every call +// in the bodies: a deferred call with no enclosing function range is dropped. +func TestCppExtractor_OutOfLineMemberEmitsNodeAndCalls(t *testing.T) { + src := []byte(`#include "cls.h" + +void Cls::depth2() { + boost::asio::post(ex); +} + +Widget *ns::Cls::depth3() { + std::chrono::duration_cast(value); + return nullptr; +} + +void a::b::Cls::depth4() { + helper(); +} +`) + res, err := NewCppExtractor().Extract("demo.cpp", src) + require.NoError(t, err) + byID := cppNodeByID(res) + byLine := genericCallTargets(res) + + depth2 := byID["demo.cpp::Cls.depth2"] + require.NotNil(t, depth2, "an out-of-line member must emit a node") + assert.Equal(t, graph.KindMethod, depth2.Kind) + assert.Equal(t, "depth2", depth2.Name, "the trailing segment names the member") + assert.Equal(t, "Cls", depth2.Meta["receiver"]) + assert.Contains(t, byLine[4], "unresolved::post", + "a call in an out-of-line body needs the body's own function range") + + depth3 := byID["demo.cpp::Cls.depth3"] + require.NotNil(t, depth3, "a pointer return must not hide the qualified declarator") + assert.Equal(t, "depth3", depth3.Name) + assert.Equal(t, "ns", depth3.Meta["scope_ns"], + "the segments ahead of the owner are its namespace") + assert.Contains(t, byLine[8], "unresolved::duration_cast") + + depth4 := byID["demo.cpp::Cls.depth4"] + require.NotNil(t, depth4, "qualification depth must not cap definition extraction") + assert.Equal(t, "a::b", depth4.Meta["scope_ns"]) + assert.Contains(t, byLine[13], "unresolved::helper") +} + +// Two classes defining a same-named member in one translation unit is ordinary +// C++; keying the node on the owner is what keeps the second from colliding +// with the first and being dropped. +func TestCppExtractor_OutOfLineMembersDoNotCollide(t *testing.T) { + src := []byte(`#include "cls.h" + +void Alpha::run() { + alphaWork(); +} + +void Beta::run() { + betaWork(); +} +`) + res, err := NewCppExtractor().Extract("demo.cpp", src) + require.NoError(t, err) + byID := cppNodeByID(res) + + require.NotNil(t, byID["demo.cpp::Alpha.run"]) + require.NotNil(t, byID["demo.cpp::Beta.run"]) + byLine := genericCallTargets(res) + assert.Contains(t, byLine[4], "unresolved::alphaWork") + assert.Contains(t, byLine[8], "unresolved::betaWork") +} + +// A qualifier naming a namespace declares a free function, not a member. The +// class/struct declared in the same file is the evidence; a name this file +// never declared falls back to the Capitalized-type convention. +func TestCppExtractor_OutOfLineNamespaceFunctionStaysFree(t *testing.T) { + src := []byte(`namespace ns { void helper(); } + +void ns::helper() { + work(); +} +`) + res, err := NewCppExtractor().Extract("demo.cpp", src) + require.NoError(t, err) + byID := cppNodeByID(res) + + fn := byID["demo.cpp::helper"] + require.NotNil(t, fn, "a namespace-qualified definition is a free function") + assert.Equal(t, graph.KindFunction, fn.Kind) + assert.Equal(t, "ns", fn.Meta["scope_ns"]) + assert.Nil(t, fn.Meta["receiver"], "a namespace is not a receiver") + assert.Contains(t, genericCallTargets(res)[4], "unresolved::work") +} + +// When the class is declared in this file, the member links to it — and the +// in-class declaration must not double as a second node for the definition. +func TestCppExtractor_OutOfLineMemberLinksLocalClass(t *testing.T) { + src := []byte(`class Cls { +public: + void run(); +}; + +void Cls::run() { + work(); +} +`) + res, err := NewCppExtractor().Extract("demo.cpp", src) + require.NoError(t, err) + + var memberOf int + for _, e := range res.Edges { + if e.Kind == graph.EdgeMemberOf && e.From == "demo.cpp::Cls.run" && e.To == "demo.cpp::Cls" { + memberOf++ + } + } + assert.Equal(t, 1, memberOf, "the member belongs to the class declared here") + + var runNodes int + for _, n := range res.Nodes { + if n.Name == "run" { + runNodes++ + } + } + assert.Equal(t, 1, runNodes, "one definition is one node") +} + +// A destructor, an operator, and a templated member all reach the same walk; +// none of them names itself with a bare identifier. +func TestCppExtractor_OutOfLineSpecialMembers(t *testing.T) { + src := []byte(`#include "cls.h" + +Cls::~Cls() { + teardown(); +} + +Cls &Cls::operator=(const Cls &other) { + copyFrom(other); + return *this; +} + +template +void Holder::put(T value) { + store(value); +} +`) + res, err := NewCppExtractor().Extract("demo.cpp", src) + require.NoError(t, err) + byID := cppNodeByID(res) + byLine := genericCallTargets(res) + + dtor := byID["demo.cpp::Cls.~Cls"] + require.NotNil(t, dtor, "an out-of-line destructor is a member") + assert.Contains(t, byLine[4], "unresolved::teardown") + + op := byID["demo.cpp::Cls.operator="] + require.NotNil(t, op, "an out-of-line operator is a member") + assert.Contains(t, byLine[8], "unresolved::copyFrom") + + put := byID["demo.cpp::Holder.put"] + require.NotNil(t, put, "a template member drops its type arguments to name its owner") + assert.Equal(t, "Holder", put.Meta["receiver"]) + assert.Contains(t, byLine[14], "unresolved::store") + + // The template_declaration and the function_definition it wraps both reach + // a dispatch that can now name a qualified declarator; only one may emit. + var putNodes int + for _, n := range res.Nodes { + if n.Name == "put" { + putNodes++ + } + } + assert.Equal(t, 1, putNodes, "a template member emits once, not once per dispatch") +} + +// The declarator walk also decodes qualified callees, so a qualified operator +// call names itself the way an out-of-line operator definition does instead of +// being dropped for not ending in a bare identifier. +func TestCppExtractor_QualifiedOperatorCallEmitsEdge(t *testing.T) { + src := []byte(`void run(Vec a, Vec b) { + ns::operator+(a, b); +} +`) + res, err := NewCppExtractor().Extract("demo.cpp", src) + require.NoError(t, err) + assert.Contains(t, genericCallTargets(res)[2], "unresolved::operator+") +} + +// Free functions and inline methods are the paths that already worked; the +// broadened declarator pattern must leave both exactly as they were. +func TestCppExtractor_PlainDefinitionsUnchanged(t *testing.T) { + src := []byte(`namespace ns { + +class Cls { +public: + void inlineM() { inlineWork(); } +}; + +void freeFn() { + freeWork(); +} + +} // namespace ns +`) + res, err := NewCppExtractor().Extract("demo.cpp", src) + require.NoError(t, err) + byID := cppNodeByID(res) + byLine := genericCallTargets(res) + + inlineM := byID["demo.cpp::Cls.inlineM"] + require.NotNil(t, inlineM) + assert.Equal(t, graph.KindMethod, inlineM.Kind) + assert.Contains(t, byLine[5], "unresolved::inlineWork") + + freeFn := byID["demo.cpp::freeFn"] + require.NotNil(t, freeFn) + assert.Equal(t, graph.KindFunction, freeFn.Kind) + assert.Equal(t, "ns", freeFn.Meta["scope_ns"]) + assert.Contains(t, byLine[9], "unresolved::freeWork") +}