feat(cpp): resolve template-body this-> + using ns::name calls in scope resolver (#1590)

* 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 <gergomagyar@icloud.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
2026-05-14 18:18:36 +01:00
parent c2193318b5
commit b00ba2ab47
13 changed files with 281 additions and 8 deletions
@@ -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<string>();
+    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;
@@ -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
@@ -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) {
@@ -3,9 +3,12 @@
 #include "base.h"
 #include "helpers.h"

+using utils::ns_helper_2;
+
 template<class T>
 struct D : Base<T> {
  void g() {
    utils::ns_helper();
+    ns_helper_2();
  }
 };
@@ -2,4 +2,5 @@

 namespace utils {
  void ns_helper();
+  void ns_helper_2();
 }
@@ -0,0 +1,6 @@
+#pragma once
+
+template<class T>
+struct Base {
+  void f();
+};
@@ -0,0 +1,14 @@
+#pragma once
+
+#include "base.h"
+
+template<class T>
+struct Derived : Base<T> {
+  void g_unqualified() {
+    f();
+  }
+
+  void g_this() {
+    this->f();
+  }
+};
@@ -0,0 +1,6 @@
+#pragma once
+
+template<class T>
+struct Base {
+  void f();
+};
@@ -0,0 +1,16 @@
+#pragma once
+
+#include "base.h"
+
+template<class T>
+struct Derived : Base<T> {
+  void f(int);
+
+  void g() {
+    this->f();
+  }
+
+  void g_ok() {
+    this->f(42);
+  }
+};
@@ -3,5 +3,6 @@
 template<class T>
 struct Base {
  void f();
+  void base_method();
  int i;
 };
@@ -7,6 +7,9 @@ struct Derived : Base<T> {
  void g() {
    this->f();
  }
+  void k() {
+    this->base_method();
+  }
  int h() {
    return this->i;
  }
@@ -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<T>::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<T>::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<T>::g_unqualified() -> f() does NOT bind to Base<T>::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<T>::g_this() -> this->f() resolves to Base<T>::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<T>::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<T>::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<T>::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<T>::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
@@ -168,6 +168,15 @@ const LEGACY_RESOLVER_PARITY_EXPECTED_FAILURES: Readonly<Record<string, Readonly
    'emits distinct Class nodes for List<User> and List<Order>',
    '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<T>::g() -> this->f() resolves to f (1 edge)',
+    'Derived<T>::k() -> this->base_method() resolves via EXTENDS chain (1 edge)',
+    'Derived<T>::g_unqualified() -> f() does NOT bind to Base<T>::f',
+    'Derived<T>::g_this() -> this->f() resolves to Base<T>::f (1 edge)',
+    'Derived<T>::g() -> this->f() emits zero CALLS edges when only hidden derived overload is arity-incompatible',
  ]),
 };