From fe9dcd009d1434d7690182e1e5048efb67b9869e Mon Sep 17 00:00:00 2001 From: azizur100389 Date: Tue, 26 May 2026 00:42:27 +0100 Subject: [PATCH] fix(cpp): use function-type ADL entities --- .../src/core/ingestion/languages/cpp/adl.ts | 132 ++++++++++++------ .../core/ingestion/languages/cpp/captures.ts | 24 ++++ .../app.cpp | 7 + .../cpp-adl-free-func-ref-return-strict/lib.h | 11 ++ .../cpp-adl-free-func-ref-strict/app.cpp | 7 + .../cpp-adl-free-func-ref-strict/lib.h | 11 ++ .../test/integration/resolvers/cpp.test.ts | 84 ++++++----- .../test/integration/resolvers/helpers.ts | 5 + 8 files changed, 203 insertions(+), 78 deletions(-) create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-adl-free-func-ref-return-strict/app.cpp create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-adl-free-func-ref-return-strict/lib.h create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-adl-free-func-ref-strict/app.cpp create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-adl-free-func-ref-strict/lib.h diff --git a/gitnexus/src/core/ingestion/languages/cpp/adl.ts b/gitnexus/src/core/ingestion/languages/cpp/adl.ts index 0bcda9322..86c826b57 100644 --- a/gitnexus/src/core/ingestion/languages/cpp/adl.ts +++ b/gitnexus/src/core/ingestion/languages/cpp/adl.ts @@ -24,22 +24,18 @@ * V2 additionally walks class ancestors (via MRO), so base-class enclosing * namespaces also contribute associated namespaces. * - * **GitNexus approximation (not strict ISO C++ ADL):** passing a qualified - * function reference like `utils::worker` contributes `utils` to the associated - * set, enabling resolution of unqualified calls like `with_callback(utils::worker)` - * to `utils::with_callback`. Under ISO C++ `[basic.lookup.argdep]`, associated - * entities for function-type arguments come from the **parameter types and return - * type** of each function in the overload set — NOT the function's enclosing - * namespace. For `void worker()`, the standard-compliant associated set is empty. - * GitNexus instead contributes the enclosing namespace of any Function/Method - * def whose simple name matches, because it enables the dominant real-world ADL - * pattern at reasonable precision cost. + * Function-reference arguments follow ISO C++ `[basic.lookup.argdep]`: + * associated entities come from the parameter types and return type of each + * referenced function in the overload set, not from the function's enclosing + * namespace. For `void worker()`, the associated set is empty. For + * `void worker(api::Token)` or `api::Token make_token()`, `api` is associated + * through `Token`. * - * For qualified refs (e.g. `utils::worker`) the namespace is confirmed via a - * workspace lookup (only contributed when a Function/Method named `worker` exists - * in `utils`). For unqualified refs the workspace is searched for any Function - * def with that simple name. Locally-declared function-pointer variables - * (e.g. `void (*g)()`) and function parameters are excluded from this path. + * For qualified refs (e.g. `utils::worker`) the workspace lookup is restricted + * to functions/methods named `worker` in `utils`; for unqualified refs the + * workspace is searched for matching functions/methods by simple name. Locally + * declared function-pointer variables and function parameters are excluded + * from this path. * * ADL candidates are merged with ordinary unqualified-lookup candidates * in the free-call fallback before overload narrowing. @@ -70,6 +66,7 @@ import type { ParsedFile, ScopeId, SymbolDefinition } from 'gitnexus-shared'; import type { ScopeResolutionIndexes } from '../../model/scope-resolution-indexes.js'; +import { normalizeCppParamType } from './arity-metadata.js'; import { isCppInlineNamespaceScope } from './inline-namespaces.js'; /** @@ -97,11 +94,8 @@ export interface CppAdlArgInfo { /** When set, the arg is a potential free-function reference (not a locally- * declared function-pointer variable or function parameter). Contains the * identifier text as written in source (e.g. `"utils::worker"` or - * `"worker"`). GitNexus approximation: the function's enclosing namespace - * is contributed to the ADL associated set. For qualified refs a workspace - * lookup confirms a Function/Method with that simple name exists in the - * namespace before contributing; for unqualified refs every namespace - * containing a matching Function/Method def is contributed. */ + * `"worker"`). Resolution contributes associated namespaces from each + * referenced Function/Method def's parameter and return types. */ readonly functionRefText?: string; } @@ -207,7 +201,12 @@ export function pickCppAdlCandidates( for (const arg of args) { collectAssociatedNamespacesForAdlArg(arg, scopes, associatedNamespaces); if (arg.functionRefText !== undefined) { - collectFunctionRefNamespaces(arg.functionRefText, parsedFiles, associatedNamespaces); + collectFunctionTypeAssociatedNamespaces( + arg.functionRefText, + scopes, + parsedFiles, + associatedNamespaces, + ); } } if (associatedNamespaces.size === 0) return undefined; @@ -472,23 +471,12 @@ function findCppClassDefBySimpleName( } /** - * Contribute associated namespaces for a function-reference argument. - * - * - **Qualified refs** (`utils::worker`, `outer::inner::fn`): the namespace - * is extracted from the qualifier text (converting `::` to `.` for dot-joined - * QName matching). A workspace lookup then **verifies** that a Function or - * Method def named `worker` (the simple name after the last `::`) actually - * exists in the extracted namespace. This prevents false positives from - * namespace-qualified variables, enum values, and static data members, which - * also produce `qualified_identifier` AST nodes in tree-sitter-cpp (the - * AST node type alone does not distinguish functions from non-function names). - * - **Unqualified refs** (`worker`): the workspace is searched for any - * Function/Method def whose simple name matches. Every distinct enclosing - * namespace found is added — overloads across the same namespace produce - * a single entry; GitNexus does not select a specific overload at this stage. + * Contribute associated namespaces for a function-reference argument by walking + * the referenced overload set's parameter and return types. */ -function collectFunctionRefNamespaces( +function collectFunctionTypeAssociatedNamespaces( refText: string, + scopes: ScopeResolutionIndexes, parsedFiles: readonly ParsedFile[], out: Set, ): void { @@ -511,30 +499,82 @@ function collectFunctionRefNamespaces( for (const def of scope.ownedDefs) { if (def.type !== 'Function' && def.type !== 'Method') continue; const simple = def.qualifiedName?.split('.').pop() ?? def.qualifiedName ?? ''; - if (simple === simpleName) { - out.add(nsText); - return; // Namespace confirmed; no need to scan further files. - } + if (simple === simpleName) collectAssociatedNamespacesForFunctionDef(def, scopes, out); } } } return; } - // Unqualified: search all namespace scopes for a Function def with this - // simple name and contribute its enclosing namespace. for (const parsed of parsedFiles) { - const scopesById = new Map(); - for (const sc of parsed.scopes) scopesById.set(sc.id, sc); for (const scope of parsed.scopes) { if (scope.kind !== 'Namespace') continue; for (const def of scope.ownedDefs) { if (def.type !== 'Function' && def.type !== 'Method') continue; const simple = def.qualifiedName?.split('.').pop() ?? def.qualifiedName ?? ''; if (simple !== refText) continue; - const nsQName = computeNamespaceQName(scope, scopesById); - if (nsQName !== '') out.add(nsQName); + collectAssociatedNamespacesForFunctionDef(def, scopes, out); } } } } + +function collectAssociatedNamespacesForFunctionDef( + def: SymbolDefinition, + scopes: ScopeResolutionIndexes, + out: Set, +): void { + const parameterTypes = def.parameterTypeClasses?.map((typeClass) => typeClass.base); + for (const paramType of parameterTypes ?? def.parameterTypes ?? []) { + collectAssociatedNamespacesForFunctionTypeText(paramType, scopes, out); + } + if (def.returnType !== undefined) { + collectAssociatedNamespacesForFunctionTypeText(def.returnType, scopes, out); + } +} + +function collectAssociatedNamespacesForFunctionTypeText( + typeText: string, + scopes: ScopeResolutionIndexes, + out: Set, +): void { + for (const token of extractCppTypeNameTokens(typeText)) { + addAssociatedNamespaceForClassName(token.simpleName, scopes, out); + if (token.namespaceName !== '') out.add(token.namespaceName); + } +} + +function extractCppTypeNameTokens(typeText: string): readonly { + readonly simpleName: string; + readonly namespaceName: string; +}[] { + const cleaned = normalizeCppParamType(typeText); + if (cleaned === '' || isPrimitiveCppAdlType(cleaned)) return []; + const out: { simpleName: string; namespaceName: string }[] = []; + for (const rawToken of cleaned.match(/[A-Za-z_]\w*(?:::[A-Za-z_]\w*)*/g) ?? []) { + if (isPrimitiveCppAdlType(rawToken)) continue; + const segments = rawToken.split('::').filter((part) => part.length > 0); + const simpleName = segments.at(-1) ?? ''; + if (simpleName === '' || isPrimitiveCppAdlType(simpleName)) continue; + out.push({ + simpleName, + namespaceName: segments.length > 1 ? segments.slice(0, -1).join('.') : '', + }); + } + return out; +} + +function isPrimitiveCppAdlType(typeText: string): boolean { + return ( + typeText === 'void' || + typeText === 'bool' || + typeText === 'char' || + typeText === 'int' || + typeText === 'double' || + typeText === 'float' || + typeText === 'string' || + typeText === 'null' || + typeText === 'unknown' || + typeText === '...' + ); +} diff --git a/gitnexus/src/core/ingestion/languages/cpp/captures.ts b/gitnexus/src/core/ingestion/languages/cpp/captures.ts index de8cd058e..e5a16f856 100644 --- a/gitnexus/src/core/ingestion/languages/cpp/captures.ts +++ b/gitnexus/src/core/ingestion/languages/cpp/captures.ts @@ -126,6 +126,14 @@ export function emitCppScopeCaptures( JSON.stringify(arity.parameterTypeClasses), ); } + const returnType = extractCppDeclarationReturnType(fnNode); + if (returnType !== undefined) { + grouped['@declaration.return-type'] = syntheticCapture( + '@declaration.return-type', + fnNode, + returnType, + ); + } // Detect static storage class (file-local linkage) if (hasStaticStorageClass(fnNode)) { @@ -410,6 +418,22 @@ export function emitCppScopeCaptures( return out; } +function extractCppDeclarationReturnType(fnNode: SyntaxNode): string | undefined { + const typeNode = fnNode.childForFieldName('type'); + if (typeNode === null) return undefined; + const typeText = typeNode.text.trim(); + if (typeText !== 'auto') return typeText.length > 0 ? typeText : undefined; + const funcDeclarator = findFunctionDeclarator(fnNode); + if (funcDeclarator === null) return typeText; + for (let i = 0; i < funcDeclarator.namedChildCount; i++) { + const child = funcDeclarator.namedChild(i); + if (child?.type !== 'trailing_return_type') continue; + const typeDesc = child.firstNamedChild; + return typeDesc?.text.trim() || typeText; + } + return typeText; +} + /** * Walk every C++ class/struct base clause and emit `@reference.inherits` * captures for each base so scope resolution can resolve them into EXTENDS diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-adl-free-func-ref-return-strict/app.cpp b/gitnexus/test/fixtures/lang-resolution/cpp-adl-free-func-ref-return-strict/app.cpp new file mode 100644 index 000000000..d8d768ded --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-adl-free-func-ref-return-strict/app.cpp @@ -0,0 +1,7 @@ +#include "lib.h" + +namespace caller { + void run() { + run_callback(utils::make_token); + } +} diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-adl-free-func-ref-return-strict/lib.h b/gitnexus/test/fixtures/lang-resolution/cpp-adl-free-func-ref-return-strict/lib.h new file mode 100644 index 000000000..857d9cc42 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-adl-free-func-ref-return-strict/lib.h @@ -0,0 +1,11 @@ +#pragma once + +namespace api { + struct Token { + friend void run_callback(Token t) {} + }; +} + +namespace utils { + api::Token make_token(); +} diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-adl-free-func-ref-strict/app.cpp b/gitnexus/test/fixtures/lang-resolution/cpp-adl-free-func-ref-strict/app.cpp new file mode 100644 index 000000000..6c3eb0786 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-adl-free-func-ref-strict/app.cpp @@ -0,0 +1,7 @@ +#include "lib.h" + +namespace caller { + void run() { + run_callback(utils::worker); + } +} diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-adl-free-func-ref-strict/lib.h b/gitnexus/test/fixtures/lang-resolution/cpp-adl-free-func-ref-strict/lib.h new file mode 100644 index 000000000..9463986aa --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-adl-free-func-ref-strict/lib.h @@ -0,0 +1,11 @@ +#pragma once + +namespace api { + struct Token { + friend void run_callback(Token t) {} + }; +} + +namespace utils { + void worker(api::Token token); +} diff --git a/gitnexus/test/integration/resolvers/cpp.test.ts b/gitnexus/test/integration/resolvers/cpp.test.ts index b00aa9632..06b7ddda2 100644 --- a/gitnexus/test/integration/resolvers/cpp.test.ts +++ b/gitnexus/test/integration/resolvers/cpp.test.ts @@ -2898,41 +2898,66 @@ describe('C++ ADL — block-scope function declaration suppresses ADL', () => { }); // --------------------------------------------------------------------------- -// ADL V2 — free-function reference args contribute their namespace. +// ADL V2 - strict function-type associated entities. // -// GitNexus approximation (not strict ISO C++ ADL): when a qualified_identifier -// like `utils::worker` is passed as an argument, GitNexus contributes the -// enclosing namespace (`utils`) to the associated set, provided a Function or -// Method named `worker` is found in the `utils` namespace at resolution time. -// Under ISO C++ [basic.lookup.argdep] the associated entities for a function-type -// argument come from the parameter types and return type of the overload set — -// NOT the function's enclosing namespace. For `void worker()`, the standard- -// compliant associated set is empty. The approximation captures the dominant -// real-world pattern (pass a utility function → find its sibling) at the cost -// of potential false positives when an unrelated function with the same simple -// name exists in the same namespace (bounded by the workspace-function lookup). +// Function-reference arguments follow strict ISO C++ ADL: GitNexus walks the +// referenced overload set's parameter and return types instead of contributing +// the referenced function's enclosing namespace. +// For `void worker()`, the associated set is empty; for `void worker(api::Token)` +// or `api::Token make_token()`, `api` is associated through `Token`. // --------------------------------------------------------------------------- -describe('C++ ADL — qualified free-function reference contributes its namespace', () => { +describe('C++ ADL - free-function reference does not contribute its namespace', () => { let result: PipelineResult; beforeAll(async () => { result = await runPipelineFromRepo(path.join(FIXTURES, 'cpp-adl-free-func-ref'), () => {}); }, 60000); - it('with_callback(utils::worker) resolves to utils::with_callback via ADL', () => { + it('with_callback(utils::worker) emits zero CALLS edges when worker has no class parameter or return type', () => { const calls = getRelationships(result, 'CALLS'); const cbCalls = calls.filter((c) => c.source === 'run' && c.target === 'with_callback'); - // Ordinary lookup inside caller::run finds nothing (no `using`, no local - // declaration). utils::worker is a qualified_identifier argument, so ADL - // contributes `utils` to the associated-namespace set. utils::with_callback - // is then discovered as the sole candidate. - expect(cbCalls.length).toBe(1); - expect(cbCalls[0].targetFilePath).toContain('utils.h'); + expect(cbCalls.length).toBe(0); }); }); -describe('C++ ADL — overloaded free-function reference does not crash', () => { +describe('C++ ADL - free-function reference contributes parameter-type associated namespace', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'cpp-adl-free-func-ref-strict'), + () => {}, + ); + }, 60000); + + it('run_callback(utils::worker) resolves hidden friend through worker(api::Token)', () => { + const calls = getRelationships(result, 'CALLS'); + const cbCalls = calls.filter((c) => c.source === 'run' && c.target === 'run_callback'); + expect(cbCalls.length).toBe(1); + expect(cbCalls[0].targetFilePath).toContain('lib.h'); + }); +}); + +describe('C++ ADL - free-function reference contributes return-type associated namespace', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'cpp-adl-free-func-ref-return-strict'), + () => {}, + ); + }, 60000); + + it('run_callback(utils::make_token) resolves hidden friend through api::Token return type', () => { + const calls = getRelationships(result, 'CALLS'); + const cbCalls = calls.filter((c) => c.source === 'run' && c.target === 'run_callback'); + expect(cbCalls.length).toBe(1); + expect(cbCalls[0].targetFilePath).toContain('lib.h'); + }); +}); + +describe('C++ ADL - overloaded free-function reference stays strict', () => { let result: PipelineResult; beforeAll(async () => { @@ -2942,15 +2967,10 @@ describe('C++ ADL — overloaded free-function reference does not crash', () => ); }, 60000); - it('with_callback(utils::worker) with overloaded utils::worker still resolves utils::with_callback via ADL', () => { + it('with_callback(utils::worker) with overloaded utils::worker still emits zero CALLS edges', () => { const calls = getRelationships(result, 'CALLS'); const cbCalls = calls.filter((c) => c.source === 'run' && c.target === 'with_callback'); - // utils::worker has two overloads (worker() and worker(int)). V1 - // simplification: contribute the namespace if ANY overload exists in the - // workspace, regardless of which one would be selected. The namespace - // `utils` is still added, and utils::with_callback is discovered. - expect(cbCalls.length).toBe(1); - expect(cbCalls[0].targetFilePath).toContain('utils.h'); + expect(cbCalls.length).toBe(0); }); }); @@ -2970,10 +2990,10 @@ describe('C++ ADL — namespace-qualified variable arg does NOT contribute names // data::value is a namespace-qualified integer variable. tree-sitter-cpp // produces a qualified_identifier AST node regardless of whether `value` // denotes a function, variable, enum, or static member. The GitNexus guard - // in collectFunctionRefNamespaces verifies that a Function/Method named - // `value` exists in the `data` namespace before contributing it. Since - // `data::value` is an int variable, `data` is never added to the associated - // set, so data::process is never found as an ADL candidate. + // in collectFunctionTypeAssociatedNamespaces verifies that a Function/Method + // named `value` exists in the `data` namespace before walking any function + // type. Since `data::value` is an int variable, no function type is walked, + // so data::process is never found as an ADL candidate. expect(processCalls.length).toBe(0); }); }); diff --git a/gitnexus/test/integration/resolvers/helpers.ts b/gitnexus/test/integration/resolvers/helpers.ts index 26c586ad1..91912e125 100644 --- a/gitnexus/test/integration/resolvers/helpers.ts +++ b/gitnexus/test/integration/resolvers/helpers.ts @@ -349,6 +349,11 @@ const LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES: Readonly