mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
fix(cpp): thread call-site types into qualified member lookup (#1632) (#1810)
Some checks are pending
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
Some checks are pending
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
* fix(cpp): thread call-site types into qualified member lookup (#1632) Widen Callsite (arity optional, add argumentTypes) and add optional callsite?: Callsite to ScopeResolver.resolveQualifiedReceiverMember. receiver-bound-calls.ts passes the ReferenceSite through structurally; resolveCppQualifiedNamespaceMember forwards it to narrowOverloadCandidates along with cppConversionRank, enabling exact-type and conversion-rank disambiguation across inline-namespace children. Behavior change: - outer::foo(42) where v1 declares foo(int) and v2 declares foo(double) now resolves to v1::foo (was: 0 edges, conservatively suppressed). - Same-name same-normalized-signature (e.g. foo(int) vs foo(long)) still suppresses at 0 edges via isOverloadAmbiguousAfterNormalization. - ADL using-import path (resolveAdlCandidates) unchanged — passes no callsite, narrowing degrades to existing pass-through behavior. Closes #1632. Part of #1564. * fix(cpp): update legacy parity expected-failure list for #1632 - Remove stale expected-failure entry for old diff-sigs test name (test now expects 1 edge; legacy DAG also emits 1 edge) - Add entry for normalized-signature ambiguity (int vs long) test - Rename describe block from 'conservative suppress' to 'distinct signatures resolved via call-site types' Verified both modes: REGISTRY_PRIMARY_CPP=1: 241/241 passed REGISTRY_PRIMARY_CPP=0: 194 passed, 47 skipped, 0 failed
This commit is contained in:
parent
1c4993251c
commit
73a6a5376e
9 changed files with 80 additions and 30 deletions
|
|
@ -267,8 +267,11 @@ export interface ScopeLookup {
|
|||
|
||||
/** Call-site description passed to `arityCompatibility`. */
|
||||
export interface Callsite {
|
||||
/** Number of arguments at the call site. */
|
||||
readonly arity: number;
|
||||
/** Number of arguments at the call site, if available. */
|
||||
readonly arity?: number;
|
||||
/** Inferred argument types at the call site, one per argument.
|
||||
* An empty string entry means the type was not inferred. */
|
||||
readonly argumentTypes?: readonly string[];
|
||||
}
|
||||
|
||||
// ─── §2.4 ImportEdge ────────────────────────────────────────────────────────
|
||||
|
|
|
|||
|
|
@ -27,12 +27,13 @@
|
|||
* declaration transparently.
|
||||
*/
|
||||
|
||||
import type { ParsedFile, ScopeId, SymbolDefinition } from 'gitnexus-shared';
|
||||
import type { Callsite, 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';
|
||||
import { cppConversionRank } from './conversion-rank.js';
|
||||
|
||||
interface RangeKey {
|
||||
readonly startLine: number;
|
||||
|
|
@ -107,6 +108,7 @@ export function resolveCppQualifiedNamespaceMember(
|
|||
memberName: string,
|
||||
parsedFiles: readonly ParsedFile[],
|
||||
_scopes: ScopeResolutionIndexes,
|
||||
callsite?: Callsite,
|
||||
): SymbolDefinition | 'ambiguous' | undefined {
|
||||
const allHits: SymbolDefinition[] = [];
|
||||
const seenNodeId = new Set<string>();
|
||||
|
|
@ -132,19 +134,17 @@ export function resolveCppQualifiedNamespaceMember(
|
|||
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);
|
||||
// Multi-candidate: thread call-site arity/argument-types through the
|
||||
// `resolveQualifiedReceiverMember` contract so `narrowOverloadCandidates`
|
||||
// can disambiguate via exact-type match and, when available, conversion-rank
|
||||
// scoring (`cppConversionRank`). Same-signature ambiguity is still detected
|
||||
// by `isOverloadAmbiguousAfterNormalization` below.
|
||||
const narrowed = narrowOverloadCandidates(
|
||||
allHits,
|
||||
callsite?.arity,
|
||||
callsite?.argumentTypes,
|
||||
callsite !== undefined ? { conversionRankFn: cppConversionRank } : undefined,
|
||||
);
|
||||
if (narrowed.length === 1) return narrowed[0];
|
||||
if (narrowed.length === 0) return undefined;
|
||||
if (isOverloadAmbiguousAfterNormalization(narrowed, undefined)) return 'ambiguous';
|
||||
|
|
|
|||
|
|
@ -275,6 +275,12 @@ export const cppScopeResolver: ScopeResolver = {
|
|||
// descends transitively through inline-namespace children when
|
||||
// searching for the called member. Returns undefined for non-namespace
|
||||
// receivers so receiver-bound-calls Case 2 still gets a chance.
|
||||
resolveQualifiedReceiverMember: (receiverName, memberName, _callerScope, scopes, parsedFiles) =>
|
||||
resolveCppQualifiedNamespaceMember(receiverName, memberName, parsedFiles, scopes),
|
||||
resolveQualifiedReceiverMember: (
|
||||
receiverName,
|
||||
memberName,
|
||||
_callerScope,
|
||||
scopes,
|
||||
parsedFiles,
|
||||
callsite,
|
||||
) => resolveCppQualifiedNamespaceMember(receiverName, memberName, parsedFiles, scopes, callsite),
|
||||
};
|
||||
|
|
|
|||
|
|
@ -705,6 +705,7 @@ export interface ScopeResolver {
|
|||
callerScope: ScopeId,
|
||||
scopes: ScopeResolutionIndexes,
|
||||
parsedFiles: readonly ParsedFile[],
|
||||
callsite?: Callsite,
|
||||
) => SymbolDefinition | 'ambiguous' | undefined;
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -501,6 +501,7 @@ export function emitReceiverBoundCalls(
|
|||
site.inScope,
|
||||
scopes,
|
||||
parsedFiles,
|
||||
site,
|
||||
);
|
||||
if (memberDef === 'ambiguous') {
|
||||
// Same-name ambiguity across inline-namespace children (#1564):
|
||||
|
|
|
|||
5
gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-normalized/caller.cpp
vendored
Normal file
5
gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-normalized/caller.cpp
vendored
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
#include "lib.h"
|
||||
|
||||
void run() {
|
||||
outer::foo(42);
|
||||
}
|
||||
10
gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-normalized/lib.h
vendored
Normal file
10
gitnexus/test/fixtures/lang-resolution/cpp-inline-namespace-ambiguous-normalized/lib.h
vendored
Normal file
|
|
@ -0,0 +1,10 @@
|
|||
#pragma once
|
||||
|
||||
namespace outer {
|
||||
inline namespace v1 {
|
||||
void foo(int x);
|
||||
}
|
||||
inline namespace v2 {
|
||||
void foo(long y);
|
||||
}
|
||||
}
|
||||
|
|
@ -3044,7 +3044,7 @@ describe('C++ inline namespace — ambiguous same-name across inline children (#
|
|||
});
|
||||
});
|
||||
|
||||
describe('C++ inline namespace — ambiguous distinct signatures (conservative suppress)', () => {
|
||||
describe('C++ inline namespace — distinct signatures resolved via call-site types', () => {
|
||||
let result: PipelineResult;
|
||||
|
||||
beforeAll(async () => {
|
||||
|
|
@ -3054,14 +3054,35 @@ describe('C++ inline namespace — ambiguous distinct signatures (conservative s
|
|||
);
|
||||
}, 60000);
|
||||
|
||||
it('outer::foo(42) emits zero CALLS edges when v1 declares foo(int) and v2 declares foo(double)', () => {
|
||||
it('outer::foo(42) emits exactly 1 CALLS edge to v1::foo(int) 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).
|
||||
// Call-site arity and argument types are now threaded through the
|
||||
// resolveQualifiedReceiverMember contract (#1632). narrowOverloadCandidates
|
||||
// matches the exact type 'int' against v1::foo(int), producing exactly 1 edge.
|
||||
expect(fooCalls).toHaveLength(1);
|
||||
// Verify it resolved to v1::foo(int) at line 4 (0-indexed), not v2::foo(double) at line 7
|
||||
const targetNode = result.graph.getNode(fooCalls[0].rel.targetId);
|
||||
expect(targetNode?.properties.startLine).toBe(4);
|
||||
});
|
||||
});
|
||||
|
||||
describe('C++ inline namespace — ambiguous normalized signatures', () => {
|
||||
let result: PipelineResult;
|
||||
|
||||
beforeAll(async () => {
|
||||
result = await runPipelineFromRepo(
|
||||
path.join(FIXTURES, 'cpp-inline-namespace-ambiguous-normalized'),
|
||||
() => {},
|
||||
);
|
||||
}, 60000);
|
||||
|
||||
it('outer::foo(42) emits zero CALLS edges when v1 declares foo(int) and v2 declares foo(long) — both normalize to int', () => {
|
||||
const calls = getRelationships(result, 'CALLS');
|
||||
const fooCalls = calls.filter((c) => c.source === 'run' && c.target === 'foo');
|
||||
// int and long both normalize to 'int' via normalizeCppParamType, making
|
||||
// the two candidates indistinguishable after normalization. The resolver
|
||||
// must suppress rather than pick arbitrarily (isOverloadAmbiguousAfterNormalization).
|
||||
expect(fooCalls.length).toBe(0);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -332,11 +332,14 @@ const LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES: Readonly<Record<string, Readonly
|
|||
'outer::foo() emits zero CALLS edges when v1 and v2 both declare foo',
|
||||
'records a structured suppression reason for inline namespace ambiguity',
|
||||
// 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)',
|
||||
// `foo(double)` in v2. PR #1810 threads call-site types through the
|
||||
// resolveQualifiedReceiverMember contract — resolved in both mode paths.
|
||||
// Legacy DAG emits an edge via the global callable fallback; the test
|
||||
// now expects 1 edge, so the old expected-failure entry is removed.
|
||||
// Normalized-signature ambiguity: `foo(int)` vs `foo(long)` both map to
|
||||
// `int` via normalizeCppParamType. Scope-resolver suppresses via
|
||||
// isOverloadAmbiguousAfterNormalization; legacy path picks arbitrarily.
|
||||
'outer::foo(42) emits zero CALLS edges when v1 declares foo(int) and v2 declares foo(long) — both normalize to int',
|
||||
// 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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue