From 8500f18e5fd0823d89a42492a15533cf9eae60b4 Mon Sep 17 00:00:00 2001 From: azizur100389 Date: Fri, 15 May 2026 19:11:09 +0100 Subject: [PATCH] fix(cpp): detect same-name ambiguity across inline namespace children (#1564) (#1600) --- .../languages/cpp/inline-namespaces.ts | 63 ++++++++++++++----- .../ingestion/languages/cpp/scope-resolver.ts | 2 +- .../contract/scope-resolver.ts | 7 ++- .../passes/receiver-bound-calls.ts | 6 ++ .../caller.cpp | 5 ++ .../lib.h | 10 +++ .../cpp-inline-namespace-ambiguous/caller.cpp | 5 ++ .../cpp-inline-namespace-ambiguous/lib.h | 10 +++ .../test/integration/resolvers/cpp.test.ts | 41 ++++++++++++ .../test/integration/resolvers/helpers.ts | 12 ++++ 10 files changed, 142 insertions(+), 19 deletions(-) create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-diff-sigs/caller.cpp create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-diff-sigs/lib.h create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous/caller.cpp create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous/lib.h diff --git a/gitnexus/src/core/ingestion/languages/cpp/inline-namespaces.ts b/gitnexus/src/core/ingestion/languages/cpp/inline-namespaces.ts index c08402a85..604c50c0b 100644 --- a/gitnexus/src/core/ingestion/languages/cpp/inline-namespaces.ts +++ b/gitnexus/src/core/ingestion/languages/cpp/inline-namespaces.ts @@ -29,6 +29,10 @@ import type { ParsedFile, ScopeId, SymbolDefinition } from 'gitnexus-shared'; import type { ScopeResolutionIndexes } from '../../model/scope-resolution-indexes.js'; +import { + isOverloadAmbiguousAfterNormalization, + narrowOverloadCandidates, +} from '../../scope-resolution/passes/overload-narrowing.js'; interface RangeKey { readonly startLine: number; @@ -95,15 +99,17 @@ export function isCppInlineNamespaceScope(scopeId: ScopeId): boolean { * Returns the most specific (innermost) match — for `outer::foo()` * where `inline namespace v1` declares `foo`, returns `v1::foo`. When * multiple inline-namespace children declare the same name, ISO C++ - * leaves the call ambiguous; V1 returns the first match in source - * order (stable across runs). + * leaves the call ambiguous; returns `'ambiguous'` so the caller + * suppresses edge emission rather than picking arbitrarily (#1564). */ export function resolveCppQualifiedNamespaceMember( receiverName: string, memberName: string, parsedFiles: readonly ParsedFile[], _scopes: ScopeResolutionIndexes, -): SymbolDefinition | undefined { +): SymbolDefinition | 'ambiguous' | undefined { + const allHits: SymbolDefinition[] = []; + const seenNodeId = new Set(); for (const parsed of parsedFiles) { const scopesById = new Map(); for (const sc of parsed.scopes) scopesById.set(sc.id, sc); @@ -113,19 +119,45 @@ export function resolveCppQualifiedNamespaceMember( if (nsDef === undefined) continue; const nsName = nsDef.qualifiedName?.split('.').pop() ?? nsDef.qualifiedName ?? ''; if (nsName !== receiverName) continue; - // Found a matching namespace scope in this file. Collect the - // member transitively through any inline-namespace children. - const hit = findMemberInNamespaceTransitive(scope, scopesById, memberName); - if (hit !== undefined) return hit; + // Found a matching namespace scope in this file. Collect ALL + // members transitively through any inline-namespace children. + const hits = findMemberInNamespaceTransitive(scope, scopesById, memberName); + for (const hit of hits) { + if (seenNodeId.has(hit.nodeId)) continue; + seenNodeId.add(hit.nodeId); + allHits.push(hit); + } } } - return undefined; + if (allHits.length === 0) return undefined; + if (allHits.length === 1) return allHits[0]; + + // Multi-candidate: the `resolveQualifiedReceiverMember` hook has no + // access to call-site arity or argument types, so + // `narrowOverloadCandidates` cannot actually narrow here — the call + // with `(allHits, undefined, undefined)` is effectively a pass-through. + // We retain it so that `isOverloadAmbiguousAfterNormalization` can + // still detect int/long-style normalization collisions on this path, + // but for any multi-hit case where candidates have genuinely distinct + // signatures (e.g. `foo(int)` vs `foo(double)` in different inline + // children), we conservatively suppress rather than pick arbitrarily. + // A future enhancement could thread call-site argument info through + // the `resolveQualifiedReceiverMember` contract to enable real + // narrowing here. + const narrowed = narrowOverloadCandidates(allHits, undefined, undefined); + if (narrowed.length === 1) return narrowed[0]; + if (narrowed.length === 0) return undefined; + if (isOverloadAmbiguousAfterNormalization(narrowed, undefined)) return 'ambiguous'; + // Multiple surviving candidates (distinct signatures) — conservative + // suppress because we lack call-site info to disambiguate. + return 'ambiguous'; } /** Recursively search a namespace scope and any inline-namespace - * descendants for a callable def with the given simple name. Non-inline + * descendants for callable defs with the given simple name. Non-inline * nested namespaces are NOT traversed — they require explicit - * qualification (`outer::nested::foo`). */ + * qualification (`outer::nested::foo`). Returns ALL matches so the + * caller can detect same-name ambiguity across inline children (#1564). */ function findMemberInNamespaceTransitive( scope: { readonly id: ScopeId; @@ -142,22 +174,23 @@ function findMemberInNamespaceTransitive( } >, memberName: string, -): SymbolDefinition | undefined { +): SymbolDefinition[] { + const results: SymbolDefinition[] = []; // Check this scope's own ownedDefs first. for (const def of scope.ownedDefs) { if (def.type !== 'Function' && def.type !== 'Method' && def.type !== 'Constructor') continue; const simple = def.qualifiedName?.split('.').pop() ?? def.qualifiedName ?? ''; - if (simple === memberName) return def; + if (simple === memberName) results.push(def); } // Descend into inline-namespace children. for (const childScope of scopesById.values()) { if (childScope.parent !== scope.id) continue; if (childScope.kind !== 'Namespace') continue; if (!inlineNamespaceScopeIds.has(childScope.id)) continue; - const hit = findMemberInNamespaceTransitive(childScope, scopesById, memberName); - if (hit !== undefined) return hit; + const childHits = findMemberInNamespaceTransitive(childScope, scopesById, memberName); + for (const hit of childHits) results.push(hit); } - return undefined; + return results; } function findNamespaceDefInScope(scope: { diff --git a/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts b/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts index 8e35c9901..52c575cf4 100644 --- a/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts @@ -235,7 +235,7 @@ export const cppScopeResolver: ScopeResolver = { parsedFiles, scopes, ); - if (member === undefined) continue; + if (member === undefined || member === 'ambiguous') continue; if (seenUsing.has(member.nodeId)) continue; seenUsing.add(member.nodeId); usingNamedHits.push(member); diff --git a/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts b/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts index 16f628b0c..315471de1 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts @@ -615,8 +615,9 @@ export interface ScopeResolver { * * Receiver-bound-calls invokes this hook AFTER Case 1 (namespace * imports) and AFTER Case 2 (class-name receiver) fail to resolve. - * Returns the target def, or `undefined` to fall through to the - * remaining cases. + * Returns the target def, `'ambiguous'` when multiple inline-namespace + * children declare the same name (suppresses edge emission), or + * `undefined` to fall through to the remaining cases. */ readonly resolveQualifiedReceiverMember?: ( receiverName: string, @@ -624,7 +625,7 @@ export interface ScopeResolver { callerScope: ScopeId, scopes: ScopeResolutionIndexes, parsedFiles: readonly ParsedFile[], - ) => SymbolDefinition | undefined; + ) => SymbolDefinition | 'ambiguous' | undefined; /** * Enable the receiver-bound Case 0.5 fallback for explicit `this` diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts index 5aff78261..92b4c1ac1 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/receiver-bound-calls.ts @@ -445,6 +445,12 @@ export function emitReceiverBoundCalls( scopes, parsedFiles, ); + if (memberDef === 'ambiguous') { + // Same-name ambiguity across inline-namespace children (#1564): + // suppress edge emission, mark site handled. + handledSites.add(siteKey); + continue; + } if (memberDef !== undefined) { const ok = tryEmitEdge( graph, diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-diff-sigs/caller.cpp b/gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-diff-sigs/caller.cpp new file mode 100644 index 000000000..fcb416b39 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-diff-sigs/caller.cpp @@ -0,0 +1,5 @@ +#include "lib.h" + +void run() { + outer::foo(42); +} diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-diff-sigs/lib.h b/gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-diff-sigs/lib.h new file mode 100644 index 000000000..c0ebbdfac --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-diff-sigs/lib.h @@ -0,0 +1,10 @@ +#pragma once + +namespace outer { + inline namespace v1 { + void foo(int x); + } + inline namespace v2 { + void foo(double y); + } +} diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous/caller.cpp b/gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous/caller.cpp new file mode 100644 index 000000000..f2aa44454 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous/caller.cpp @@ -0,0 +1,5 @@ +#include "lib.h" + +void run() { + outer::foo(); +} diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous/lib.h b/gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous/lib.h new file mode 100644 index 000000000..54bfb229d --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous/lib.h @@ -0,0 +1,10 @@ +#pragma once + +namespace outer { + inline namespace v1 { + void foo(); + } + inline namespace v2 { + void foo(); + } +} diff --git a/gitnexus/test/integration/resolvers/cpp.test.ts b/gitnexus/test/integration/resolvers/cpp.test.ts index bb50c0eac..de0ad9632 100644 --- a/gitnexus/test/integration/resolvers/cpp.test.ts +++ b/gitnexus/test/integration/resolvers/cpp.test.ts @@ -2776,6 +2776,47 @@ describe('C++ inline namespace — versioned (v1 inline, v0 not)', () => { }); }); +describe('C++ inline namespace — ambiguous same-name across inline children (#1564)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'cpp-inline-namespace-ambiguous'), + () => {}, + ); + }, 60000); + + it('outer::foo() emits zero CALLS edges when v1 and v2 both declare foo', () => { + const calls = getRelationships(result, 'CALLS'); + const fooCalls = calls.filter((c) => c.source === 'run' && c.target === 'foo'); + // ISO C++ leaves this ambiguous — both inline namespace children declare + // the same name. The resolver must suppress rather than pick arbitrarily. + expect(fooCalls.length).toBe(0); + }); +}); + +describe('C++ inline namespace — ambiguous distinct signatures (conservative suppress)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'cpp-inline-namespace-ambiguous-diff-sigs'), + () => {}, + ); + }, 60000); + + it('outer::foo(42) emits zero CALLS edges when v1 declares foo(int) and v2 declares foo(double)', () => { + const calls = getRelationships(result, 'CALLS'); + const fooCalls = calls.filter((c) => c.source === 'run' && c.target === 'foo'); + // Even though the two overloads have distinct signatures and a compiler + // could disambiguate via argument types, the `resolveQualifiedReceiverMember` + // hook lacks call-site arity/argument-type information, so multi-hit cases + // are conservatively suppressed. Documents the limitation noted in + // inline-namespaces.ts (Finding 1 of Claude review on #1600). + expect(fooCalls.length).toBe(0); + }); +}); + describe('C++ inline namespace — nested (STL __1-style)', () => { let result: PipelineResult; diff --git a/gitnexus/test/integration/resolvers/helpers.ts b/gitnexus/test/integration/resolvers/helpers.ts index ffc2ced6c..ae2a75f04 100644 --- a/gitnexus/test/integration/resolvers/helpers.ts +++ b/gitnexus/test/integration/resolvers/helpers.ts @@ -175,6 +175,18 @@ const LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES: Readonly::g_unqualified() -> f() does NOT bind to Base::f', 'Derived::g_this() -> this->f() resolves to Base::f (1 edge)', 'Derived::g() -> this->f() emits zero CALLS edges when only hidden derived overload is arity-incompatible', + // The legacy DAG path has no inline-namespace same-name ambiguity + // detection. When two inline children declare the same name, the + // legacy path picks an arbitrary match. The scope-resolver returns + // 'ambiguous' and suppresses edge emission. Scope-resolver-only + // correctness win (#1564); backporting to legacy is out of scope. + 'outer::foo() emits zero CALLS edges when v1 and v2 both declare foo', + // Distinct-signature inline-namespace ambiguity: `foo(int)` in v1 and + // `foo(double)` in v2. The scope-resolver conservatively suppresses + // because `resolveQualifiedReceiverMember` lacks call-site argument + // types. Legacy DAG has no inline-namespace resolver. Scope-resolver- + // only correctness win (#1600 / Claude review Finding 1). + 'outer::foo(42) emits zero CALLS edges when v1 declares foo(int) and v2 declares foo(double)', // PR #1598: ADL free-function reference arg negative fixtures rely on // scope-resolver-only correctness. The legacy DAG falls back to // `pickUniqueGlobalCallable` which resolves the callee by simple-name