mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-19 14:32:59 +08:00
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<T...>` 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 <noreply@anthropic.com>
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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<string, string> = {
|
||||
'include/lib/base.h': `#pragma once
|
||||
#define LIB_BEGIN_NAMESPACE namespace lib {
|
||||
#define LIB_END_NAMESPACE }
|
||||
|
||||
LIB_BEGIN_NAMESPACE
|
||||
template <typename... T> struct format_string {};
|
||||
template <typename... T> struct wformat_string {};
|
||||
LIB_END_NAMESPACE
|
||||
`,
|
||||
'include/lib/color.h': `#pragma once
|
||||
#include "base.h"
|
||||
|
||||
LIB_BEGIN_NAMESPACE
|
||||
struct text_style {};
|
||||
|
||||
template <typename... T>
|
||||
inline auto format(const text_style& ts, format_string<T...> fmt, T&&... args) -> int {
|
||||
return 0;
|
||||
}
|
||||
LIB_END_NAMESPACE
|
||||
`,
|
||||
'include/lib/format.h': `#pragma once
|
||||
#include "base.h"
|
||||
|
||||
LIB_BEGIN_NAMESPACE
|
||||
template <typename... T>
|
||||
inline auto format(format_string<T...> fmt, T&&... args) -> int {
|
||||
return 1;
|
||||
}
|
||||
LIB_END_NAMESPACE
|
||||
`,
|
||||
'include/lib/xchar.h': `#pragma once
|
||||
#include "base.h"
|
||||
|
||||
LIB_BEGIN_NAMESPACE
|
||||
template <typename... T>
|
||||
inline auto format(wformat_string<T...> 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']);
|
||||
});
|
||||
});
|
||||
@@ -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<e, f>` 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<T...>`.
|
||||
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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user