From 116feb462d97027cc9193c70e9fba3793fa3b142 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Wed, 30 Sep 2026 20:36:51 +0000 Subject: [PATCH] fix(cpp): pick the overload a macro-namespaced call's arguments fit (#2201) `fmt::format("{}", x)` resolves through the macro-namespace matcher (#2176), which took the first same-named function of the namespace - color.h's `format(const text_style&, format_string, T&&...)` - for all of fmt's 1,400 `fmt::format` calls, and likewise `print`, `format_to`, `vformat`. Among an overload set the call's arguments are now read (to a dozen lines) and each overload's parameter list from source (C++ signatures are empty): an argument count outside the parameters' is out; a string-literal argument fits a parameter that names a string type best, a template parameter (`const S&`) less, anything else not; a narrow literal never fits a wide parameter (`wformat_string`, `wstring_view`) nor a wide one a narrow one. `...` inside `format_string` is not a pack. fmt: `format` -> format.h (1,363) and xchar.h for `L"..."` (57), `format_to`/`print` -> core.h, `vformat` -> format-inl.h; spdlog's bundled fmt the same; pybind11's `py::set()` -> pytypes.h's `set` class. json, rapidjson, leveldb unchanged. Edge counts unchanged (targets only). Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/cpp-overload-choice.test.ts | 100 ++++++++++++++++++++++++++ src/resolution/name-matcher.ts | 91 +++++++++++++++++++++-- 3 files changed, 188 insertions(+), 4 deletions(-) create mode 100644 __tests__/cpp-overload-choice.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c8426c1e..b5c6e79dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- A C++ call into a macro-opened namespace, like `fmt::format("{}", x)`, now picks the overload its arguments fit. Before, the first overload indexed won: fmt's 1,400 `fmt::format` calls all reached the `text_style` overload in `color.h`. A narrow string literal now fits a narrow format string, a wide `L"…"` fits the wide overload, and the argument count has to fit too. - Calls inside an inline route handler now keep their receivers, so they resolve like any other method call. Before, Hono's `app.get('/', (c) => c.text('Hello'))` linked the route to the one other `text` method in the project, its client's `ClientResponse.text`; that happened in over 400 routes. A call like `userService.lookup(id)` reaches the service it names. - A Scala chained call like `xs.map { … }` no longer lands on an object that happens to be named `map`, as with cats' `alleycats.std.map` and its `syntax`/`instances` objects. A type or object in a chain is only matched when the receiver names what holds it, as in `sttp.client4.Response(…)`. - A Python call to a name the function or module binds itself now means that local value, not a same-named function in another file. In Django REST Framework's tests, `view = SomeView.as_view()` followed by `view(request)` had pointed at one test file's `def view`. The same goes for a parameter or a `for`/`with … as` target. A pytest fixture a test takes as a parameter is still the fixture. diff --git a/__tests__/cpp-overload-choice.test.ts b/__tests__/cpp-overload-choice.test.ts new file mode 100644 index 000000000..b69a1007d --- /dev/null +++ b/__tests__/cpp-overload-choice.test.ts @@ -0,0 +1,100 @@ +/** + * `lib::format("{}", x)` among overloads opened by a namespace macro picks the + * one the arguments fit: fmt's 1,400 `fmt::format("{}", …)` calls all went to + * color.h's `format(const text_style&, …)` — the first of the set indexed. + * A narrow literal fits a `format_string`, a wide one (`L"{}"`) the + * `wformat_string` overload, and a style argument the style overload. + */ +import { describe, it, expect, afterAll, beforeAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +let root = ''; +let cg: CodeGraph; + +beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-cpp-overload-')); + const files: Record = { + 'include/lib/base.h': `#pragma once +#define LIB_BEGIN_NAMESPACE namespace lib { +#define LIB_END_NAMESPACE } + +LIB_BEGIN_NAMESPACE +template struct format_string {}; +template struct wformat_string {}; +LIB_END_NAMESPACE +`, + 'include/lib/color.h': `#pragma once +#include "base.h" + +LIB_BEGIN_NAMESPACE +struct text_style {}; + +template +inline auto format(const text_style& ts, format_string fmt, T&&... args) -> int { + return 0; +} +LIB_END_NAMESPACE +`, + 'include/lib/format.h': `#pragma once +#include "base.h" + +LIB_BEGIN_NAMESPACE +template +inline auto format(format_string fmt, T&&... args) -> int { + return 1; +} +LIB_END_NAMESPACE +`, + 'include/lib/xchar.h': `#pragma once +#include "base.h" + +LIB_BEGIN_NAMESPACE +template +inline auto format(wformat_string fmt, T&&... args) -> int { + return 2; +} +LIB_END_NAMESPACE +`, + 'test/use.cc': `#include "lib/color.h" +#include "lib/format.h" +#include "lib/xchar.h" + +int narrow() { return lib::format("{}", 1); } +int wide() { return lib::format(L"{}", 2); } +int styled() { return lib::format(lib::text_style{}, "{}", 3); } +`, + }; + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), content); + } + cg = await CodeGraph.init(root, { index: true }); +}); + +afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); +}); + +const formatFileCalledBy = (fn: string): string[] => { + const node = cg.getNodesInFile('test/use.cc').find((n) => n.name === fn)!; + return cg.getOutgoingEdges(node.id).filter((e) => e.kind === 'calls').map((e) => cg.getNode(e.target)!) + .filter((t) => t.name === 'format').map((t) => t.filePath); +}; + +describe('C++ overloads in a macro-opened namespace', () => { + it('a narrow literal picks the format_string overload', () => { + expect(formatFileCalledBy('narrow')).toEqual(['include/lib/format.h']); + }); + + it('a wide literal picks the wformat_string overload', () => { + expect(formatFileCalledBy('wide')).toEqual(['include/lib/xchar.h']); + }); + + it('a style argument picks the style overload', () => { + expect(formatFileCalledBy('styled')).toEqual(['include/lib/color.h']); + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 58dbee7b7..1d189cbe6 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -8178,7 +8178,7 @@ function matchCppMacroNamespaced(ref: UnresolvedRef, context: ResolutionContext) if (alias) target = alias + target.slice(head.length); const name = target.slice(target.lastIndexOf('::') + 2); if (!/^[A-Za-z_~]\w*$/.test(name)) return null; - let best: Node | null = null; + const matches: Node[] = []; for (const n of context.getNodesByName(name)) { if (n.language !== 'cpp' && n.language !== 'c') continue; if (!['function', 'method', 'class', 'struct', 'enum', 'type_alias', 'union', 'variable', 'constant'].includes(n.kind)) continue; @@ -8187,9 +8187,92 @@ function matchCppMacroNamespaced(ref: UnresolvedRef, context: ResolutionContext) .sort((a, b) => a.start - b.start) .flatMap((f) => f.path); const effective = prefix.length > 0 ? `${prefix.join('::')}::${n.qualifiedName}` : alias ? n.qualifiedName : ''; - if (effective !== target) continue; - // One of an overload set is as good as another; a declaration outside the tests over one in them. - if (!best || (isTestPath(best.filePath) && !isTestPath(n.filePath))) best = n; + if (effective === target) matches.push(n); + } + // A declaration outside the tests over one in them; among an overload set, + // the one the call's arguments fit: `fmt::format("{}", v)` is format.h's + // `format(format_string, T&&...)`, not color.h's `format(const text_style&, …)`. + const args = matches.length > 1 && ref.referenceKind === 'calls' ? cppCallArguments(ref, name, context) : null; + let best: Node | null = null; + let bestScore = -Infinity; + for (const n of matches) { + const score = (isTestPath(n.filePath) ? -10 : 0) + (args ? cppOverloadFit(n, name, args, context) : 0); + if (score > bestScore) { best = n; bestScore = score; } } return best ? { original: ref, targetNodeId: best.id, confidence: 0.8, resolvedBy: 'qualified-name' } : null; } + +/** Split `a, f(b, c), d` at its top-level commas. */ +function splitCppTopLevel(text: string): string[] { + const out: string[] = []; + let depth = 0; + let cur = ''; + for (let i = 0; i < text.length; i++) { + const ch = text[i]!; + if (ch === '"' || ch === "'") { + let j = i + 1; + while (j < text.length && text[j] !== ch) j += text[j] === '\\' ? 2 : 1; + cur += text.slice(i, j + 1); + i = j; + continue; + } + if (ch === '(' || ch === '[' || ch === '{' || ch === '<') depth++; + else if (ch === ')' || ch === ']' || ch === '}' || (ch === '>' && text[i - 1] !== '-')) depth = Math.max(0, depth - 1); + if (ch === ',' && depth === 0) { out.push(cur.trim()); cur = ''; continue; } + cur += ch; + } + if (cur.trim()) out.push(cur.trim()); + return out; +} + +/** The balanced `( … )` after `name` from `line`/`column` of `file` (up to a dozen lines), or null. */ +function cppParenListAfter(file: string, line: number, column: number, name: string, context: ResolutionContext): string | null { + const lines = context.getFileLines?.(file) ?? context.readFile(file)?.split(/\r?\n/) ?? []; + const text = lines.slice(line - 1, line + 11).join('\n'); + const at = new RegExp(`\\b${name.replace(/[~]/g, '\\$&')}\\s*(?:<[^<>()]*>)?\\s*\\(`).exec(text.slice(column)); + if (!at) return null; + const open = column + at.index + at[0].length - 1; + let depth = 0; + for (let i = open; i < text.length; i++) { + if (text[i] === '(') depth++; + else if (text[i] === ')' && --depth === 0) return text.slice(open + 1, i); + } + return null; +} + +function cppCallArguments(ref: UnresolvedRef, name: string, context: ResolutionContext): string[] | null { + const list = cppParenListAfter(ref.filePath, ref.line, Math.max(0, ref.column), name, context); + return list === null ? null : splitCppTopLevel(list); +} + +/** How well a call's arguments fit an overload's parameters: arity, and a string literal's first slot. */ +function cppOverloadFit(n: Node, name: string, args: string[], context: ResolutionContext): number { + if (n.kind !== 'function' && n.kind !== 'method') return -1; + const list = cppParenListAfter(n.filePath, n.startLine, 0, name, context); + if (list === null) return 0; + const params = splitCppTopLevel(list).filter((p) => p !== 'void'); + // A pack is `T&&... args`, not the `...` inside `format_string`. + const isPack = (p: string): boolean => { + let flat = p; + for (let prev = ''; prev !== flat;) { prev = flat; flat = flat.replace(/<[^<>]*>/g, ''); } + return flat.includes('...'); + }; + const variadic = params.some(isPack); + const required = params.filter((p) => !isPack(p) && !/=/.test(p)).length; + let score = 0; + if (args.length < required || (!variadic && args.length > params.length)) score -= 3; + // Each string literal against its parameter: a string type by name over a + // template parameter that might be one (`const S&`), and a narrow literal + // never a wide parameter (`fmt::join(v, ", ")` is not xchar.h's `wstring_view`). + for (let i = 0; i < args.length && i < params.length; i++) { + const arg = args[i]!; + const param = params[i]!; + if (isPack(param)) break; + if (!/^(?:u8|u|U|L)?"|^FMT_STRING\s*\(/.test(arg)) continue; + const wideArg = /^L"/.test(arg); + const wideParam = /\bw(?:string|char_t|format|string_view)|wchar_t/.test(param); + if (wideArg !== wideParam && /string|char|Char|format/.test(param)) score -= 2; + else score += /string|char|Char|\bstr\b/.test(param) ? 3 : /^(?:const\s+)?[A-Z]\w{0,2}\s*[&*]{0,2}\s*\w*$/.test(param) ? 1 : -2; + } + return score; +}