From b00ba2ab4755acb3eb20ee92232bd4091ee36352 Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Thu, 14 May 2026 18:18:36 +0100 Subject: [PATCH] feat(cpp): resolve template-body `this->` + `using ns::name` calls in scope resolver (#1590) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Initial plan * fix(cpp): resolve this-> and using-name calls in template bodies Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/d9d91945-f19c-4fd2-9b52-b0ebc9aa34b6 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * fix(cpp): treat duplicate using-name hits as ambiguous Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/d9d91945-f19c-4fd2-9b52-b0ebc9aa34b6 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * chore(autofix): apply prettier + eslint fixes via /autofix command * fix(cpp): gate this-receiver path and harden overload semantics Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/030a1842-c698-460d-ae2a-95037e6def73 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * test(cpp): add positive this-> overload case and document field shadowing Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/030a1842-c698-460d-ae2a-95037e6def73 Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> * test(cpp): skip new template-this assertions in legacy parity lane Agent-Logs-Url: https://github.com/abhigyanpatwari/GitNexus/sessions/27002f6e-6331-41e3-8175-9d9e4691927c Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: magyargergo <11230420+magyargergo@users.noreply.github.com> Co-authored-by: Gergő Magyar Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --- .../ingestion/languages/cpp/scope-resolver.ts | 33 ++++++ .../contract/scope-resolver.ts | 12 +++ .../passes/receiver-bound-calls.ts | 83 ++++++++++++++ .../derived.h | 3 + .../helpers.h | 1 + .../cpp-two-phase-paired/base.h | 6 ++ .../cpp-two-phase-paired/derived.h | 14 +++ .../base.h | 6 ++ .../derived.h | 16 +++ .../cpp-two-phase-this-qualified/base.h | 1 + .../cpp-two-phase-this-qualified/derived.h | 3 + .../test/integration/resolvers/cpp.test.ts | 102 ++++++++++++++++-- .../test/integration/resolvers/helpers.ts | 9 ++ 13 files changed, 281 insertions(+), 8 deletions(-) create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-two-phase-paired/base.h create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-two-phase-paired/derived.h create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-two-phase-this-name-hiding-arity/base.h create mode 100644 gitnexus/test/fixtures/lang-resolution/cpp-two-phase-this-name-hiding-arity/derived.h diff --git a/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts b/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts index 02aaff3bf..431717d27 100644 --- a/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/languages/cpp/scope-resolver.ts @@ -36,6 +36,10 @@ import { resolveCppQualifiedNamespaceMember, } from './inline-namespaces.js'; import { populateCppRangeBindings } from './range-bindings.js'; +import { + isOverloadAmbiguousAfterNormalization, + narrowOverloadCandidates, +} from '../../scope-resolution/passes/overload-narrowing.js'; /** * C++ `ScopeResolver` registered in `SCOPE_RESOLVERS` and consumed by @@ -178,6 +182,8 @@ export const cppScopeResolver: ScopeResolver = { // for cross-file propagation and compound-receiver chain resolution. // cppBindingScopeFor hoists @type-binding.return to Module scope. hoistTypeBindingsToModule: true, + // Enable receiver-bound explicit-`this` fallback only for C++. + resolveThisViaEnclosingClass: true, // The `isFileLocalDef` hook on the global free-call fallback names // file-local linkage historically, but semantically gates "logically // invisible cross-file" defs. C++ extends this to also reject class- @@ -219,6 +225,33 @@ export const cppScopeResolver: ScopeResolver = { // V1 limitation: only direct enclosing-namespace closure for value // class-typed args; pointer/reference/template-spec args excluded. resolveAdlCandidates: (site, callerParsed, scopes, parsedFiles) => { + // `using ns::name;` introduces `name` into ordinary unqualified lookup. + // For template-class method bodies, lexical scope walks can miss this + // named-using visibility; recover by resolving the imported namespace + // member directly when the local call name matches a named using import. + const usingNamedHits: SymbolDefinition[] = []; + const seenUsing = new Set(); + for (const imp of callerParsed.parsedImports) { + if (imp.kind !== 'named') continue; + if (imp.localName !== site.name) continue; + const member = resolveCppQualifiedNamespaceMember( + imp.targetRaw, + imp.importedName, + parsedFiles, + scopes, + ); + if (member === undefined) continue; + if (seenUsing.has(member.nodeId)) continue; + seenUsing.add(member.nodeId); + usingNamedHits.push(member); + } + if (usingNamedHits.length > 0) { + const narrowed = narrowOverloadCandidates(usingNamedHits, site.arity, site.argumentTypes); + if (isOverloadAmbiguousAfterNormalization(narrowed, site.arity)) return 'ambiguous'; + if (narrowed.length === 1) return narrowed[0]; + if (narrowed.length > 1) return 'ambiguous'; + } + const result = pickCppAdlCandidates(site, callerParsed, scopes, parsedFiles); if (result === ADL_AMBIGUOUS) return 'ambiguous'; return result; 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 d29b6efa8..36856e6a8 100644 --- a/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts +++ b/gitnexus/src/core/ingestion/scope-resolution/contract/scope-resolver.ts @@ -627,6 +627,18 @@ export interface ScopeResolver { parsedFiles: readonly ParsedFile[], ) => SymbolDefinition | undefined; + /** + * Enable the receiver-bound Case 0.5 fallback for explicit `this` + * receivers (`this->m()` / `this.m()`) that resolves against the + * enclosing class + MRO even when no explicit `this` typeBinding is + * present in scope. + * + * Keep disabled for languages where the existing type-binding path + * (Case 4) already handles `this` correctly and overload ambiguity + * suppression must remain unchanged. + */ + readonly resolveThisViaEnclosingClass?: boolean; + /** * Optional post-finalize hook to inject cross-file bindings that * aren't modeled via explicit imports. Runs after 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 80ff3a200..5aff78261 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 @@ -72,6 +72,7 @@ type ReceiverBoundProviderSubset = Pick< | 'unwrapCollectionAccessor' | 'hoistTypeBindingsToModule' | 'resolveQualifiedReceiverMember' + | 'resolveThisViaEnclosingClass' >; function normalizeTemplateArgToken(value: string): string { @@ -321,6 +322,88 @@ export function emitReceiverBoundCalls( } } + // ── Case 0.5: implicit `this` receiver ─────────────────────── + // C++ `this->member()` (and same-shape receivers in other OO + // languages) should resolve against the enclosing class + MRO + // even when there is no explicit `this` typeBinding in scope. + if (provider.resolveThisViaEnclosingClass === true && receiverName === 'this') { + const enclosingClass = findEnclosingClassDef(site.inScope, scopes); + if (enclosingClass !== undefined) { + const chain = [ + enclosingClass.nodeId, + ...scopes.methodDispatch.mroFor(enclosingClass.nodeId), + ]; + let memberDef: SymbolDefinition | undefined; + let ambiguous = false; + let hiddenByName = false; + for (const ownerId of chain) { + const methodOverloads = model.methods.lookupAllByOwner(ownerId, memberName); + if (methodOverloads.length > 0) { + const narrowed = narrowOverloadCandidates( + methodOverloads, + site.arity, + site.argumentTypes, + ); + if (isOverloadAmbiguousAfterNormalization(narrowed, site.arity)) { + ambiguous = true; + break; + } + if (narrowed.length === 0) { + // C++ name hiding: if the derived class declares `f`, base-class + // overloads named `f` are hidden for member lookup + // ([basic.lookup.classref]). A non-viable derived overload set + // therefore terminates lookup instead of falling through to base. + hiddenByName = true; + break; + } + memberDef = narrowed[0] ?? methodOverloads[0]; + break; + } + + // Field/property lookup intentionally runs only after the method + // lookup above: in C++ member-name lookup, functions with this + // name hide same-named base members; we therefore prefer method + // candidates first and only target a field when no methods with + // this name exist on the current owner. + memberDef = model.fields.lookupFieldByOwner(ownerId, memberName); + if (memberDef !== undefined) { + break; + } + } + if (ambiguous) { + handledSites.add(siteKey); + continue; + } + if (hiddenByName) { + handledSites.add(siteKey); + continue; + } + if (memberDef !== undefined) { + const reason = + site.kind === 'write' || site.kind === 'read' + ? site.kind + : memberDef.filePath !== parsed.filePath + ? 'import-resolved' + : 'global'; + const confidence = site.kind === 'write' || site.kind === 'read' ? 1.0 : 0.85; + const ok = tryEmitEdge( + graph, + scopes, + nodeLookup, + site, + memberDef, + reason, + seen, + confidence, + collapse, + ); + if (ok) emitted++; + handledSites.add(siteKey); + continue; + } + } + } + // ── Case 1: namespace receiver ─────────────────────────────── const targetFiles = namespaceTargets.get(receiverName); if (targetFiles !== undefined) { diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-namespace-free-call-inside-template/derived.h b/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-namespace-free-call-inside-template/derived.h index ef57810fc..e3ea61977 100644 --- a/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-namespace-free-call-inside-template/derived.h +++ b/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-namespace-free-call-inside-template/derived.h @@ -3,9 +3,12 @@ #include "base.h" #include "helpers.h" +using utils::ns_helper_2; + template struct D : Base { void g() { utils::ns_helper(); + ns_helper_2(); } }; diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-namespace-free-call-inside-template/helpers.h b/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-namespace-free-call-inside-template/helpers.h index 5e291aba6..4bd7a9833 100644 --- a/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-namespace-free-call-inside-template/helpers.h +++ b/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-namespace-free-call-inside-template/helpers.h @@ -2,4 +2,5 @@ namespace utils { void ns_helper(); + void ns_helper_2(); } diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-paired/base.h b/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-paired/base.h new file mode 100644 index 000000000..84bc1a954 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-paired/base.h @@ -0,0 +1,6 @@ +#pragma once + +template +struct Base { + void f(); +}; diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-paired/derived.h b/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-paired/derived.h new file mode 100644 index 000000000..c7cc1a53b --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-paired/derived.h @@ -0,0 +1,14 @@ +#pragma once + +#include "base.h" + +template +struct Derived : Base { + void g_unqualified() { + f(); + } + + void g_this() { + this->f(); + } +}; diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-this-name-hiding-arity/base.h b/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-this-name-hiding-arity/base.h new file mode 100644 index 000000000..84bc1a954 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-this-name-hiding-arity/base.h @@ -0,0 +1,6 @@ +#pragma once + +template +struct Base { + void f(); +}; diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-this-name-hiding-arity/derived.h b/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-this-name-hiding-arity/derived.h new file mode 100644 index 000000000..ad867e820 --- /dev/null +++ b/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-this-name-hiding-arity/derived.h @@ -0,0 +1,16 @@ +#pragma once + +#include "base.h" + +template +struct Derived : Base { + void f(int); + + void g() { + this->f(); + } + + void g_ok() { + this->f(42); + } +}; diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-this-qualified/base.h b/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-this-qualified/base.h index 1c7084ee6..286f877a1 100644 --- a/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-this-qualified/base.h +++ b/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-this-qualified/base.h @@ -3,5 +3,6 @@ template struct Base { void f(); + void base_method(); int i; }; diff --git a/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-this-qualified/derived.h b/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-this-qualified/derived.h index 5c13c1737..9b154a429 100644 --- a/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-this-qualified/derived.h +++ b/gitnexus/test/fixtures/lang-resolution/cpp-two-phase-this-qualified/derived.h @@ -7,6 +7,9 @@ struct Derived : Base { void g() { this->f(); } + void k() { + this->base_method(); + } int h() { return this->i; } diff --git a/gitnexus/test/integration/resolvers/cpp.test.ts b/gitnexus/test/integration/resolvers/cpp.test.ts index eafa76cf9..cd95cfae7 100644 --- a/gitnexus/test/integration/resolvers/cpp.test.ts +++ b/gitnexus/test/integration/resolvers/cpp.test.ts @@ -1972,14 +1972,100 @@ describe('C++ two-phase template lookup — dependent base suppression', () => { }); }); -// NOTE: positive guards (this->f() resolves, non-dependent-base unqualified -// f() resolves, namespace-qualified utils::ns_helper() resolves) inside -// template bodies are documented gaps in C++ template-context resolution -// independent of U3's dependent-base suppression. The U3 core asserts only -// the negative behavior (dependent-base members are NOT bound by unqualified -// calls); the positive cases would require additional `this` type-binding -// and template-body member-lookup work tracked separately. See plan -// 2026-05-13-001 follow-ups. +describe('C++ two-phase template lookup — positive this-qualified calls', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'cpp-two-phase-this-qualified'), + () => {}, + ); + }, 60000); + + it('Derived::g() -> this->f() resolves to f (1 edge)', () => { + const calls = getRelationships(result, 'CALLS'); + const thisCalls = calls.filter((c) => c.source === 'g' && c.target === 'f'); + expect(thisCalls.length).toBe(1); + expect(thisCalls[0].targetFilePath).toContain('base.h'); + }); + + it('Derived::k() -> this->base_method() resolves via EXTENDS chain (1 edge)', () => { + const calls = getRelationships(result, 'CALLS'); + const inheritedCalls = calls.filter((c) => c.source === 'k' && c.target === 'base_method'); + expect(inheritedCalls.length).toBe(1); + expect(inheritedCalls[0].targetFilePath).toContain('base.h'); + }); +}); + +describe('C++ two-phase template lookup — paired unqualified + this-qualified in one fixture', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo(path.join(FIXTURES, 'cpp-two-phase-paired'), () => {}); + }, 60000); + + it('Derived::g_unqualified() -> f() does NOT bind to Base::f', () => { + const calls = getRelationships(result, 'CALLS'); + const leaks = calls.filter((c) => c.source === 'g_unqualified' && c.target === 'f'); + expect(leaks.length).toBe(0); + }); + + it('Derived::g_this() -> this->f() resolves to Base::f (1 edge)', () => { + const calls = getRelationships(result, 'CALLS'); + const resolved = calls.filter((c) => c.source === 'g_this' && c.target === 'f'); + expect(resolved.length).toBe(1); + expect(resolved[0].targetFilePath).toContain('base.h'); + }); +}); + +describe('C++ two-phase template lookup — namespace calls inside template body', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'cpp-two-phase-namespace-free-call-inside-template'), + () => {}, + ); + }, 60000); + + it('D::g() -> utils::ns_helper() resolves (1 edge)', () => { + const calls = getRelationships(result, 'CALLS'); + const qualifiedCalls = calls.filter((c) => c.source === 'g' && c.target === 'ns_helper'); + expect(qualifiedCalls.length).toBe(1); + expect(qualifiedCalls[0].targetFilePath).toContain('helpers.h'); + }); + + it('D::g() -> ns_helper_2() resolves after using-declaration (1 edge)', () => { + const calls = getRelationships(result, 'CALLS'); + const usingCalls = calls.filter((c) => c.source === 'g' && c.target === 'ns_helper_2'); + expect(usingCalls.length).toBe(1); + expect(usingCalls[0].targetFilePath).toContain('helpers.h'); + }); +}); + +describe('C++ two-phase template lookup — this-> name-hiding arity mismatch', () => { + let result: PipelineResult; + + beforeAll(async () => { + result = await runPipelineFromRepo( + path.join(FIXTURES, 'cpp-two-phase-this-name-hiding-arity'), + () => {}, + ); + }, 60000); + + it('Derived::g() -> this->f() emits zero CALLS edges when only hidden derived overload is arity-incompatible', () => { + const calls = getRelationships(result, 'CALLS'); + const fCalls = calls.filter((c) => c.source === 'g' && c.target === 'f'); + expect(fCalls.length).toBe(0); + }); + + it('Derived::g_ok() -> this->f(42) resolves to derived overload (1 edge)', () => { + const calls = getRelationships(result, 'CALLS'); + const fCalls = calls.filter((c) => c.source === 'g_ok' && c.target === 'f'); + expect(fCalls.length).toBe(1); + expect(fCalls[0].targetFilePath).toContain('derived.h'); + }); +}); // --------------------------------------------------------------------------- // U3 cross-file namespace variant: Base lives in a different file AND diff --git a/gitnexus/test/integration/resolvers/helpers.ts b/gitnexus/test/integration/resolvers/helpers.ts index 5149e3e69..396e7905b 100644 --- a/gitnexus/test/integration/resolvers/helpers.ts +++ b/gitnexus/test/integration/resolvers/helpers.ts @@ -168,6 +168,15 @@ const LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES: Readonly and List', 'callSave() in each specialization resolves to its own save()', 'save specialization bodies route to their own sibling method', + // PR #1590 follow-up: explicit `this->` resolution in template class + // bodies and paired two-phase assertions are scope-resolver-only. + // Legacy DAG lacks this receiver-bound template semantics and + // dependent-base suppression parity for these shapes. + 'Derived::g() -> this->f() resolves to f (1 edge)', + 'Derived::k() -> this->base_method() resolves via EXTENDS chain (1 edge)', + 'Derived::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', ]), };