diff --git a/gitnexus/src/core/ingestion/languages/cpp/conversion-rank.ts b/gitnexus/src/core/ingestion/languages/cpp/conversion-rank.ts index 2a9e3bc01..bea3600a7 100644 --- a/gitnexus/src/core/ingestion/languages/cpp/conversion-rank.ts +++ b/gitnexus/src/core/ingestion/languages/cpp/conversion-rank.ts @@ -1,32 +1,30 @@ /** - * C++ conversion-rank scoring for overload resolution (#1578). + * C++ conversion-rank scoring for overload resolution (#1578, #1637). * - * Operates on **normalized** type strings (output of - * `normalizeCppParamType` in `arity-metadata.ts`). After normalization: - * - int/long/short/unsigned → 'int' - * - float/double → 'double' - * - char → 'char', bool → 'bool' - * - * Because the normalizer collapses promotion pairs (int↔long, - * float↔double) to the same string, those promotions are invisible at - * this layer — they appear as exact matches (rank 0). + * Operates on normalized type strings (output of `normalizeCppParamType` + * in `arity-metadata.ts`) plus optional shape sidecars from #1630. + * Normalization intentionally collapses cv/ref/pointer spelling for stable + * graph IDs, so pointer/nullptr rules must consult `ParameterTypeClass`. * * Post-normalization ranking: - * - rank 0 — exact (same normalized type) - * - rank 1 — integral promotion (char→int, bool→int) - * - rank 2 — standard arithmetic conversion (int↔double, char→double, - * bool→double) - * - Infinity — mismatch (string↔int, user types, pointers, etc.) + * - rank 0: exact (same normalized type) + * - rank 1: integral promotion (char -> int, bool -> int) + * - rank 2: standard conversion (arithmetic, nullptr -> T*, T* -> bool, + * T* -> void*) + * - rank 3: nullptr -> bool (kept worse than nullptr -> T*) + * - rank 4: ellipsis conversion (worst viable) + * - Infinity: mismatch (string -> int, user types, unsupported shapes) * - * This function is intentionally C++-specific (issue #1578 pitfall: - * keep conversion-rank tables out of shared overload-narrowing). Other - * languages may define their own `ConversionRankFn` in the future. + * This function is intentionally C++-specific. Other languages may define + * their own `ConversionRankFn` in the future. */ +import type { ParameterTypeClass } from 'gitnexus-shared'; + /** Set of normalized arithmetic types that support implicit conversion. */ const ARITHMETIC = new Set(['int', 'double', 'char', 'bool']); -/** Integral promotion targets: char→int and bool→int are rank 1. */ +/** Integral promotion targets: char -> int and bool -> int are rank 1. */ const INTEGRAL_PROMOTION = new Map([ ['char', 'int'], ['bool', 'int'], @@ -35,13 +33,40 @@ const INTEGRAL_PROMOTION = new Map([ /** * Return the conversion rank from `argType` to `paramType`. * - * @returns 0 for exact match, 1 for integral promotion (char/bool→int), - * 2 for standard arithmetic conversion, Infinity for mismatch. + * @returns 0 for exact match, 1 for integral promotion, 2 for standard + * conversion, 3 for nullptr -> bool, 4 for ellipsis, Infinity + * for mismatch. */ -export function cppConversionRank(argType: string, paramType: string): number { - if (argType === paramType) return 0; - // Integral promotions: char→int, bool→int (ISO C++ [conv.prom]) +export function cppConversionRank( + argType: string, + paramType: string, + argTypeClass?: ParameterTypeClass, + paramTypeClass?: ParameterTypeClass, +): number { + if (argType === paramType) { + return exactShapeCompatible(argTypeClass, paramTypeClass) ? 0 : Infinity; + } + if (paramType === '...') return 4; if (INTEGRAL_PROMOTION.get(argType) === paramType) return 1; if (ARITHMETIC.has(argType) && ARITHMETIC.has(paramType)) return 2; + if (argType === 'null' && isPointer(paramTypeClass)) return 2; + if (argType === 'null' && paramType === 'bool') return 3; + if (isPointer(argTypeClass) && paramType === 'bool') return 2; + if (isPointer(argTypeClass) && isPointer(paramTypeClass) && paramType === 'void') return 2; return Infinity; } + +function isPointer(typeClass: ParameterTypeClass | undefined): boolean { + return typeClass?.indirection === 'pointer' && typeClass.pointerDepth > 0; +} + +function exactShapeCompatible( + argTypeClass: ParameterTypeClass | undefined, + paramTypeClass: ParameterTypeClass | undefined, +): boolean { + if (argTypeClass === undefined || paramTypeClass === undefined) return true; + if (argTypeClass.indirection === 'unknown' || paramTypeClass.indirection === 'unknown') { + return true; + } + return isPointer(argTypeClass) === isPointer(paramTypeClass); +} diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts index f3010abf4..0d537db07 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/free-call-fallback.ts @@ -17,7 +17,13 @@ * generalization plan. */ -import type { ParsedFile, Reference, ScopeId, SymbolDefinition } from 'gitnexus-shared'; +import type { + ParameterTypeClass, + ParsedFile, + Reference, + ScopeId, + SymbolDefinition, +} from 'gitnexus-shared'; import type { KnowledgeGraph } from '../../../graph/types.js'; import type { ScopeResolutionIndexes } from '../../model/scope-resolution-indexes.js'; import type { SemanticModel } from '../../model/semantic-model.js'; @@ -277,6 +283,7 @@ export function emitFreeCallFallback( }) : undefined, site.argumentTypes, + site.argumentTypeClasses, options.conversionRankFn, ); } @@ -342,6 +349,7 @@ function pickUniqueGlobalCallable( callArity?: number, isCallerVisible?: (candidate: SymbolDefinition) => boolean, callArgTypes?: readonly string[], + callArgTypeClasses?: readonly ParameterTypeClass[], conversionRankFn?: ConversionRankFn, ): SymbolDefinition | undefined { const scopeDefs: SymbolDefinition[] = []; @@ -380,6 +388,7 @@ function pickUniqueGlobalCallable( // disambiguate (e.g., `f(int)` vs `f(double)` called with `f(2.5)`). if (scopeDefs.length > 1) { const narrowed = narrowOverloadCandidates(scopeDefs, callArity, callArgTypes, { + argumentTypeClasses: callArgTypeClasses, conversionRankFn, }); if (narrowed.length === 1) return narrowed[0]; @@ -420,6 +429,7 @@ function pickUniqueGlobalCallable( // Same argument-type + conversion-rank narrowing for the model pool. if (defs.length > 1) { const narrowed = narrowOverloadCandidates(defs, callArity, callArgTypes, { + argumentTypeClasses: callArgTypeClasses, conversionRankFn, }); if (narrowed.length === 1) return narrowed[0]; diff --git a/gitnexus/src/core/ingestion/scope-resolution/passes/overload-narrowing.ts b/gitnexus/src/core/ingestion/scope-resolution/passes/overload-narrowing.ts index b9b452eb7..f28cd3185 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/passes/overload-narrowing.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/passes/overload-narrowing.ts @@ -38,7 +38,13 @@ * 5. Empty input returns empty output. */ -import type { ArityVerdict, Callsite, ConstraintContext, SymbolDefinition } from 'gitnexus-shared'; +import type { + ArityVerdict, + Callsite, + ConstraintContext, + ParameterTypeClass, + SymbolDefinition, +} from 'gitnexus-shared'; /** * Per-slot conversion-rank function. Returns a numeric cost for @@ -51,7 +57,12 @@ import type { ArityVerdict, Callsite, ConstraintContext, SymbolDefinition } from * Each language provides its own implementation. The function operates * on normalized type strings (output of the language's type normalizer). */ -export type ConversionRankFn = (argType: string, paramType: string) => number; +export type ConversionRankFn = ( + argType: string, + paramType: string, + argTypeClass?: ParameterTypeClass, + paramTypeClass?: ParameterTypeClass, +) => number; /** * Optional hook bundle for narrowing extension points. Threaded in @@ -130,7 +141,16 @@ export function narrowOverloadCandidates( if (params === undefined) return false; for (let i = 0; i < argTypes.length && i < params.length; i++) { if (argTypes[i] === '') continue; - if (argTypes[i] !== params[i]) return false; + if ( + !exactTypeSlotMatches( + argTypes[i], + params[i], + hookCtx?.argumentTypeClasses?.[i], + d.parameterTypeClasses?.[i], + ) + ) { + return false; + } } return true; }); @@ -144,7 +164,12 @@ export function narrowOverloadCandidates( // are returned; multiple survivors are genuinely ambiguous. When // ranking also yields empty, fall through to the arity-filtered // `candidates` set — matches pre-#1606 behavior. - const ranked = rankByConversion(candidates, argTypes, hookCtx.conversionRankFn); + const ranked = rankByConversion( + candidates, + argTypes, + hookCtx.conversionRankFn, + hookCtx.argumentTypeClasses, + ); if (ranked.length > 0) result = ranked; } } @@ -183,6 +208,27 @@ export function narrowOverloadCandidates( return result; } +function exactTypeSlotMatches( + argType: string, + paramType: string, + argTypeClass?: ParameterTypeClass, + paramTypeClass?: ParameterTypeClass, +): boolean { + if (argType !== paramType) return false; + // C++ normalizes away pointer markers (`int*` -> `int`). When both sides + // provide shape sidecars, do not let that collapse make `int` exactly match + // `int*`. Unknown sidecar evidence preserves the previous string-only path. + if (argTypeClass === undefined || paramTypeClass === undefined) return true; + if (argTypeClass.indirection === 'unknown' || paramTypeClass.indirection === 'unknown') { + return true; + } + return isPointerShape(argTypeClass) === isPointerShape(paramTypeClass); +} + +function isPointerShape(typeClass: ParameterTypeClass): boolean { + return typeClass.indirection === 'pointer' && typeClass.pointerDepth > 0; +} + /** * Pairwise dominance comparison (ISO C++ [over.ics.rank]). * @@ -199,6 +245,7 @@ function rankByConversion( candidates: readonly SymbolDefinition[], argTypes: readonly string[], rankFn: ConversionRankFn, + argTypeClasses?: readonly ParameterTypeClass[], ): readonly SymbolDefinition[] { // Step 1: compute per-slot ranks and exclude non-viable candidates. const viable: Array<{ def: SymbolDefinition; ranks: number[] }> = []; @@ -207,12 +254,22 @@ function rankByConversion( if (params === undefined) continue; const ranks: number[] = []; let ok = true; - for (let i = 0; i < argTypes.length && i < params.length; i++) { + for (let i = 0; i < argTypes.length; i++) { + const paramType = parameterTypeAt(params, i); + if (paramType === undefined) { + ok = false; + break; + } if (argTypes[i] === '') { ranks.push(0); // unknown arg → any-match (rank 0) continue; } - const r = rankFn(argTypes[i], params[i]); + const r = rankFn( + argTypes[i], + paramType, + argTypeClasses?.[i], + parameterTypeClassAt(d.parameterTypeClasses, i), + ); if (!isFinite(r)) { ok = false; break; @@ -239,6 +296,20 @@ function rankByConversion( return viable.filter((_, idx) => !dominated.has(idx)).map((v) => v.def); } +function parameterTypeAt(params: readonly string[], argIndex: number): string | undefined { + if (argIndex < params.length) return params[argIndex]; + return params[params.length - 1] === '...' ? '...' : undefined; +} + +function parameterTypeClassAt( + params: readonly ParameterTypeClass[] | undefined, + argIndex: number, +): ParameterTypeClass | undefined { + if (params === undefined) return undefined; + if (argIndex < params.length) return params[argIndex]; + return params[params.length - 1]?.base === '...' ? params[params.length - 1] : undefined; +} + /** * Compare two per-slot rank vectors. * Returns -1 if `a` dominates `b` (not worse everywhere, better somewhere), diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-overload-pointer-null-ellipsis/lib.cpp b/gitnexus/test/fixtures/lang-resolution/cpp-overload-pointer-null-ellipsis/lib.cpp new file mode 100644 index 000000000..c6b0bb5a2 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-overload-pointer-null-ellipsis/lib.cpp @@ -0,0 +1,12 @@ +#include "lib.h" + +void Service::f(int* p) {} +void Service::f(bool flag) {} + +void Service::g(int a, int b) {} +void Service::g(int a, ...) {} + +void Service::h(int a, double b) {} +void Service::h(int a, ...) {} + +void Service::k(int a, ...) {} diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-overload-pointer-null-ellipsis/lib.h b/gitnexus/test/fixtures/lang-resolution/cpp-overload-pointer-null-ellipsis/lib.h new file mode 100644 index 000000000..c85f258a0 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-overload-pointer-null-ellipsis/lib.h @@ -0,0 +1,38 @@ +#pragma once + +class Service { +public: + void f(int* p); + void f(bool flag); + + void g(int a, int b); + void g(int a, ...); + + void h(int a, double b); + void h(int a, ...); + + void k(int a, ...); + + void runNullptr() { + f(nullptr); + } + + void runPointer() { + int* p = nullptr; + f(p); + } + + void runBoolConversion() { + f(42); + } + + void run() { + int* p = nullptr; + f(nullptr); + f(p); + f(42); + g(1, 2); + h(1, 'a'); + k(1, 2, 3); + } +}; diff --git a/gitnexus/test/integration/resolvers/cpp.test.ts b/gitnexus/test/integration/resolvers/cpp.test.ts index 1537c3884..e1376938e 100644 --- a/gitnexus/test/integration/resolvers/cpp.test.ts +++ b/gitnexus/test/integration/resolvers/cpp.test.ts @@ -1836,6 +1836,64 @@ describe('C++ overload resolution — conversion-rank disambiguation (#1578)', ( }); }); +// C++ overload resolution: pointer/nullptr/ellipsis conversion ranks (#1637) +describe('C++ overload resolution — pointer/nullptr/ellipsis ranks (#1637)', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'cpp-overload-pointer-null-ellipsis'), + () => {}, + ); + }, 60000); + + it('f(nullptr) and f(p) resolve to f(int*) while f(42) resolves to f(bool)', () => { + const calls = getRelationships(result, 'CALLS'); + + const nullptrCall = calls.find((c) => c.source === 'runNullptr' && c.target === 'f'); + const pointerCall = calls.find((c) => c.source === 'runPointer' && c.target === 'f'); + const boolCall = calls.find((c) => c.source === 'runBoolConversion' && c.target === 'f'); + + expect( + result.graph.getNode(nullptrCall?.rel.targetId ?? '')?.properties.parameterTypes, + ).toEqual(['int']); + expect( + result.graph.getNode(pointerCall?.rel.targetId ?? '')?.properties.parameterTypes, + ).toEqual(['int']); + expect(result.graph.getNode(boolCall?.rel.targetId ?? '')?.properties.parameterTypes).toEqual([ + 'bool', + ]); + }); + + it('g(1, 2) resolves to fixed-arity g(int, int), not g(int, ...)', () => { + const calls = getRelationships(result, 'CALLS'); + const gCalls = calls.filter((c) => c.source === 'run' && c.target === 'g'); + + expect(gCalls.length).toBe(1); + const tgt = result.graph.getNode(gCalls[0].rel.targetId); + expect(tgt?.properties.parameterTypes).toEqual(['int', 'int']); + }); + + it("h(1, 'a') resolves to h(int, double), not h(int, ...)", () => { + const calls = getRelationships(result, 'CALLS'); + const hCalls = calls.filter((c) => c.source === 'run' && c.target === 'h'); + + expect(hCalls.length).toBe(1); + const tgt = result.graph.getNode(hCalls[0].rel.targetId); + expect(tgt?.properties.parameterTypes).toEqual(['int', 'double']); + }); + + it('k(1, 2, 3) keeps the ellipsis overload viable when it is the only match', () => { + const calls = getRelationships(result, 'CALLS'); + const kCalls = calls.filter((c) => c.source === 'run' && c.target === 'k'); + + expect(kCalls.length).toBe(1); + const tgt = result.graph.getNode(kCalls[0].rel.targetId); + expect(tgt?.properties.parameterCount).toBeUndefined(); + expect(tgt?.properties.parameterTypes).toEqual(['int']); + }); +}); + // --------------------------------------------------------------------------- // U3: anonymous-namespace symbols MUST NOT leak across translation units // (full-pipeline integration test; unit-level coverage exists separately) diff --git a/gitnexus/test/integration/resolvers/helpers.ts b/gitnexus/test/integration/resolvers/helpers.ts index bd5eaa474..296f3e458 100644 --- a/gitnexus/test/integration/resolvers/helpers.ts +++ b/gitnexus/test/integration/resolvers/helpers.ts @@ -196,6 +196,12 @@ const LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES: Readonly` overloads // guarded by mutually-exclusive `enable_if_t` predicates collapse diff --git a/gitnexus/test/unit/scope-resolution/cpp/cpp-overload-ranking.test.ts b/gitnexus/test/unit/scope-resolution/cpp/cpp-overload-ranking.test.ts new file mode 100644 index 000000000..5cb679a37 --- /dev/null +++ b/gitnexus/test/unit/scope-resolution/cpp/cpp-overload-ranking.test.ts @@ -0,0 +1,111 @@ +import { describe, expect, it } from 'vitest'; +import type { ParameterTypeClass, SymbolDefinition } from 'gitnexus-shared'; +import { cppConversionRank } from '../../../../src/core/ingestion/languages/cpp/conversion-rank.js'; +import { narrowOverloadCandidates } from '../../../../src/core/ingestion/scope-resolution/passes/overload-narrowing.js'; + +const value = (base: string): ParameterTypeClass => ({ + base, + cv: 'none', + indirection: 'value', + pointerDepth: 0, +}); + +const pointer = (base: string): ParameterTypeClass => ({ + base, + cv: 'none', + indirection: 'pointer', + pointerDepth: 1, +}); + +const ellipsis = (): ParameterTypeClass => ({ + base: '...', + cv: 'unknown', + indirection: 'unknown', + pointerDepth: 0, +}); + +const mkDef = ( + nodeId: string, + parameterTypes: readonly string[], + parameterTypeClasses: readonly ParameterTypeClass[], +): SymbolDefinition => ({ + nodeId, + filePath: 'service.cpp', + type: 'Method', + parameterCount: parameterTypes.includes('...') ? undefined : parameterTypes.length, + requiredParameterCount: parameterTypes.includes('...') + ? parameterTypes.indexOf('...') + : parameterTypes.length, + parameterTypes: [...parameterTypes], + parameterTypeClasses: [...parameterTypeClasses], +}); + +describe('cppConversionRank pointer/nullptr/ellipsis ranks (#1637)', () => { + it('ranks nullptr -> T* ahead of nullptr -> bool', () => { + expect(cppConversionRank('null', 'int', value('null'), pointer('int'))).toBe(2); + expect(cppConversionRank('null', 'bool', value('null'), value('bool'))).toBe(3); + }); + + it('ranks pointer -> bool and pointer -> void* as standard conversions', () => { + expect(cppConversionRank('int', 'bool', pointer('int'), value('bool'))).toBe(2); + expect(cppConversionRank('int', 'void', pointer('int'), pointer('void'))).toBe(2); + }); + + it('keeps pointer exact matches shape-aware', () => { + expect(cppConversionRank('int', 'int', pointer('int'), pointer('int'))).toBe(0); + expect(cppConversionRank('int', 'int', value('int'), pointer('int'))).toBe(Infinity); + }); + + it('ranks ellipsis as the worst viable conversion', () => { + expect(cppConversionRank('int', '...', value('int'), ellipsis())).toBe(4); + }); +}); + +describe('narrowOverloadCandidates with C++ pointer-rank sidecars (#1637)', () => { + it('selects pointer overload for nullptr over bool overload', () => { + const byPointer = mkDef('f:intptr', ['int'], [pointer('int')]); + const byBool = mkDef('f:bool', ['bool'], [value('bool')]); + + const result = narrowOverloadCandidates([byPointer, byBool], 1, ['null'], { + argumentTypeClasses: [value('null')], + conversionRankFn: cppConversionRank, + }); + + expect(result.map((d) => d.nodeId)).toEqual(['f:intptr']); + }); + + it('does not treat normalized value and pointer types as exact matches', () => { + const byPointer = mkDef('f:intptr', ['int'], [pointer('int')]); + const byBool = mkDef('f:bool', ['bool'], [value('bool')]); + + const result = narrowOverloadCandidates([byPointer, byBool], 1, ['int'], { + argumentTypeClasses: [value('int')], + conversionRankFn: cppConversionRank, + }); + + expect(result.map((d) => d.nodeId)).toEqual(['f:bool']); + }); + + it('selects fixed-arity overload over ellipsis', () => { + const exact = mkDef('g:int-int', ['int', 'int'], [value('int'), value('int')]); + const variadic = mkDef('g:ellipsis', ['int', '...'], [value('int'), ellipsis()]); + + const result = narrowOverloadCandidates([exact, variadic], 2, ['int', 'int'], { + argumentTypeClasses: [value('int'), value('int')], + conversionRankFn: cppConversionRank, + }); + + expect(result.map((d) => d.nodeId)).toEqual(['g:int-int']); + }); + + it('keeps an ellipsis overload viable when it is the only match', () => { + const variadic = mkDef('log:ellipsis', ['int', '...'], [value('int'), ellipsis()]); + + const result = narrowOverloadCandidates([variadic], 3, ['int', 'int', 'double'], { + argumentTypeClasses: [value('int'), value('int'), value('double')], + conversionRankFn: cppConversionRank, + }); + + expect(result.map((d) => d.nodeId)).toEqual(['log:ellipsis']); + }); +});