mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-23 06:50:54 +08:00
fix(cpp): a call through a written class template specialization, like formatter<int>::format(…), never reaches another specialization's member (#2725)
The index names a class template specialization's out-of-line member
after the template: fmt's test-only `auto
fmt::formatter<incomplete_type>::format(…)` is `fmt::formatter::format`.
So `formatter<int>::format(42, ctx)` in fmt's tests reached that member,
defined in another file, and `formatter<std::tm, Char>::parse(ctx)`,
where the template declares no such member, fell through to a guess by
the receiver's name: `native_formatter::parse`.
- A C++ call whose qualifier is written with template arguments keeps a
candidate defined out of line for a full specialization of its class
only when that specialization's arguments are the call's. A template's
own members, which have a `template <…>` header with parameters,
partial specializations' members and members defined in their class
stay. This applies beside the existing specialization pool in the exact
and partial qualified-name matches and in cppMethodOf's own members.
- When the class strategy finds no member for a call through a project
class template's written specialization, matchMethodCall stops: a
method picked by the receiver's name would be another class's.
Measured on main f6d8f19b: fmt 17 wrong edges removed; json's 16
changed sites all point at a phantom node (a noexcept clause read as a
declaration, filed separately); rocksdb, protobuf, leveldb, abseil,
redis and seven controls in other languages are byte-identical.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
f6d8f19b29
commit
226ac930de
@@ -112,6 +112,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
- In C++, a call on a variable, parameter or member declared with template arguments nested inside its template arguments, like `autovector<std::pair<int, FileMetaData*>>` or `Striped<CacheAlignedWrapper<port::Mutex>>`, now links to the method of the class it is declared as. Before, the nested arguments hid that class: in RocksDB, `files_marked_for_compaction_.clear()` on such an `autovector` linked to an unrelated `CompactionInputFiles::clear` that only shares words with the variable's name, and `mutex_.Get(key)` on a `Striped` linked to nothing. Re-index C++ projects after upgrading.
|
||||
- In C++, a base class like `Message` in `class DynamicMessage final : public Message` now links to the class C++ itself would pick: the one declared in the namespaces around the class, or brought in by the file's `using` declarations and namespace aliases, and the class a `typedef` or `using` alias names. Before, a base class was matched by its name alone, so it could land on a class of the same name in another namespace or another language: in protocolbuffers/protobuf, `DynamicMessage` derived from the JSON parser's internal `Message` class, and in google/leveldb and RocksDB, iterators derived from an unrelated nested `Iterator` class such as the skip list's. The wrong base showed up in `codegraph_explore`'s type hierarchy and in impact, and tied the base's methods to the wrong overrides. A template parameter used as a base no longer links to a class that shares its name, and a base written from the global scope, like `public ::testing::Test`, now links to its class. Re-index C++ projects after upgrading.
|
||||
- In C++ and Rust, a call written with a qualifier, like `Status::OK()`, now links to a method of the class it names, never to one of a class whose name only ends the same way. Before, facebook/rocksdb's calls to `Status::OK()`, `Status::InvalidArgument(…)` and `Status::Corruption(…)` linked to `IOStatus`'s methods of those names, a subclass calling its base, like `return FileSystemWrapper::NewWritableFile(…)` or google/leveldb's `return DB::Put(…)`, linked to another subclass or to the calling method itself, and BurntSushi/ripgrep's `Path::new(…)` on the standard library's `Path` linked to its own `PrinterPath::new`. When classes in several languages share the name, the one in the call's own language is taken, so protocolbuffers/protobuf's C++ `DescriptorPool::generated_pool()` calls, which met its Ruby namesake first and linked to nothing, now link to the C++ method. A C++ call through a template parameter, like `CacheShard::ComputeHash(key)` in rocksdb's `template <class CacheShard> class ShardedCache`, no longer links to a class that merely shares words with the parameter's name, unless that class is the parameter's default, like `TcParser` in `template <typename Parser = TcParser>`. A template's value parameter, like `const PrefetchOpts& kOpts`, is no longer mistaken for a type parameter, so references to `PrefetchOpts` inside it now link. Re-index C++ and Rust projects after upgrading.
|
||||
- In C++, a call through a class template's specialization written with its arguments, like fmtlib/fmt's `formatter<int>::format(42, ctx)`, no longer links to a member defined for another specialization, like a test's `formatter<incomplete_type>::format`, and when the template declares no such member, as for `formatter<std::tm, Char>::parse(ctx)`, the call is no longer matched to another class's method of that name, like `native_formatter::parse`. A call to the specialization a member is defined for, and to a template's own members, links as before. Re-index C++ projects after upgrading.
|
||||
- In C and C++, a header or source file named like a test, such as protocolbuffers/protobuf's `conformance_test.h` and `test_runner.h`, now counts as part of the code that `#include`s it, directly or through other headers. Before, calls from that code into it were dropped, so protobuf's conformance suites' `suite_.ReportFailure(…)` and `RunValidInputTest(…)` calls, and its unit tests' calls to the shared `TestUtil` helpers, linked to nothing. A test file that only tests include is still kept out of reach of the rest of your code. Re-index C and C++ projects after upgrading.
|
||||
- In C++, a class, struct or method now keeps the namespaces and the class it is declared in when the parser misreads something earlier in the file, such as an unknown macro in front of a member or in a class header. Before, everything after the misread code could lose them, or land inside a class it isn't in: in protocolbuffers/protobuf, `FieldDescriptor` was indexed without `google::protobuf::`, and in RocksDB, the `Opts` struct declared inside a cache table class was indexed outside it, so finding a class's base classes and the methods called on its objects fell back to guessing by name. Methods that had come loose from their class are its members again, and classes the misread code had hidden, like leveldb's POSIX file and environment classes, are now indexed. Re-index C++ projects after upgrading.
|
||||
- In C++, a call on a variable, parameter or member declared as a standard-library or other outside type with a lowercase name, like `std::string`, `std::vector`, `std::set` or `absl::flat_hash_set`, no longer links to a project method that only shares the method's name, even one of a project class in another namespace with the type's name, like your own `util::flat_hash_set`. A variable declared outside the calling function, like another function's or a file's `std::string text`, no longer links its calls to such a class either. In google/leveldb, `saved_key_.clear()` on a `std::string saved_key_;` reached `Slice::clear`, and in protocolbuffers/protobuf, `proto.append(…)` on a `std::string` reached an unrelated `LeftoverBuffer::append`. The same goes for `->` through a raw pointer like `std::string* out`, while `->` through a smart pointer, iterator or optional still links to the type it holds. Comments also no longer pass for a variable's declaration, so more calls now link to the method of the type the variable is really declared with. A call on a member reached through other members, like facebook/rocksdb's `options.cf_paths.emplace_back(…)` on a `std::vector`, `props->rep.db_id.size()` on a `std::string` or `bbto.filter_policy.reset(…)` on a `std::shared_ptr`, no longer links to a project method of that name either: those had reached `autovector::emplace_back`, `BlobContents::size` and `FSWritableFilePtr::reset`. The same goes for a member of an outside type like `absl::Status`. Re-index C++ projects after upgrading.
|
||||
|
||||
@@ -0,0 +1,167 @@
|
||||
/**
|
||||
* A C++ call through a class template's specialization written with its
|
||||
* arguments — fmtlib/fmt's `formatter<int>::format(42, ctx)` — is a member of
|
||||
* that specialization. The index names every specialization's out-of-line
|
||||
* member after the template (`fmt::formatter::format` for `auto
|
||||
* fmt::formatter<incomplete_type>::format(…)`), so the call reached a test's
|
||||
* `formatter<incomplete_type>` member defined in another file, and when the
|
||||
* template had no such member at all a guess by the receiver's name took
|
||||
* another class's: `formatter<std::tm, Char>::parse(ctx)` → `native_formatter::parse`.
|
||||
*/
|
||||
import { describe, it, expect, afterEach } from 'vitest';
|
||||
import * as fs from 'fs';
|
||||
import * as os from 'os';
|
||||
import * as path from 'path';
|
||||
import { CodeGraph } from '../src';
|
||||
import type { Node } from '../src/types';
|
||||
|
||||
const roots: string[] = [];
|
||||
afterEach(() => {
|
||||
for (const root of roots.splice(0)) fs.rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
async function indexed(files: Record<string, string>): Promise<CodeGraph> {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cpp-spec-qualifiers-'));
|
||||
roots.push(root);
|
||||
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);
|
||||
}
|
||||
return CodeGraph.init(root, { index: true });
|
||||
}
|
||||
|
||||
function callable(cg: CodeGraph, qualifiedName: string): Node {
|
||||
const node = [...cg.getNodesByKind('method'), ...cg.getNodesByKind('function')].find((n) => n.qualifiedName === qualifiedName);
|
||||
if (!node) throw new Error(`no function or method ${qualifiedName}`);
|
||||
return node;
|
||||
}
|
||||
|
||||
/** `calls` callees of a function or method, as `qualifiedName` and file. */
|
||||
function calls(cg: CodeGraph, caller: string): string[] {
|
||||
return cg
|
||||
.getCallees(callable(cg, caller).id)
|
||||
.filter((r) => r.edge.kind === 'calls')
|
||||
.map((r) => `${r.node.qualifiedName} (${r.node.filePath})`)
|
||||
.sort();
|
||||
}
|
||||
|
||||
const lines = (...l: string[]): string => [...l, ''].join('\n');
|
||||
|
||||
/** fmt's primary `formatter` (no members of its own) and the native types' base. */
|
||||
const BASE = lines(
|
||||
'namespace fmt {',
|
||||
'template <typename T, typename Char = char, typename Enable = void>',
|
||||
'struct formatter {',
|
||||
' formatter() = delete;',
|
||||
'};',
|
||||
'namespace detail {',
|
||||
'template <typename T, typename Char>',
|
||||
'struct native_formatter {',
|
||||
' int format(const T& val, int& ctx) const { return ctx; }',
|
||||
' int parse(int& ctx) { return ctx; }',
|
||||
'};',
|
||||
'} // namespace detail',
|
||||
'} // namespace fmt',
|
||||
);
|
||||
|
||||
/** format-test.cc's specialization, its member defined out of line. */
|
||||
const INCOMPLETE = lines(
|
||||
'#include "include/fmt/base.h"',
|
||||
'struct incomplete_type;',
|
||||
'namespace fmt {',
|
||||
'template <> struct formatter<incomplete_type> {',
|
||||
' int format(const incomplete_type& x, int& ctx) const;',
|
||||
'};',
|
||||
'} // namespace fmt',
|
||||
'struct incomplete_type {};',
|
||||
'int fmt::formatter<incomplete_type>::format(const incomplete_type&, int& ctx) const {',
|
||||
' return ctx;',
|
||||
'}',
|
||||
'int Use(const incomplete_type& x, int& ctx) {',
|
||||
' return fmt::formatter<incomplete_type>::format(x, ctx);',
|
||||
'}',
|
||||
);
|
||||
|
||||
describe('a C++ call through a written class template specialization', () => {
|
||||
it('reproduction: `formatter<int>::format(42, ctx)` is not formatter<incomplete_type>\'s member from another file', async () => {
|
||||
const cg = await indexed({
|
||||
'include/fmt/base.h': BASE,
|
||||
'test/format-test.cc': INCOMPLETE,
|
||||
'test/compile-test.cc': lines(
|
||||
'#include "include/fmt/base.h"',
|
||||
'struct type_with_get {};',
|
||||
'namespace fmt {',
|
||||
'int FormatInt(int& ctx) {',
|
||||
' return formatter<int>::format(42, ctx);',
|
||||
'}',
|
||||
'} // namespace fmt',
|
||||
),
|
||||
});
|
||||
try {
|
||||
expect(calls(cg, 'fmt::FormatInt')).toEqual([]);
|
||||
} finally {
|
||||
cg.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('a specialization the template doesn\'t declare the member for gets no guess: `formatter<std::tm, char>::parse` is not native_formatter::parse', async () => {
|
||||
const cg = await indexed({
|
||||
'include/fmt/base.h': BASE,
|
||||
'include/fmt/chrono.h': lines(
|
||||
'#include "include/fmt/base.h"',
|
||||
'namespace fmt {',
|
||||
'int ParseTm(int& ctx) {',
|
||||
' return formatter<std::tm, char>::parse(ctx);',
|
||||
'}',
|
||||
'} // namespace fmt',
|
||||
),
|
||||
});
|
||||
try {
|
||||
expect(calls(cg, 'fmt::ParseTm')).toEqual([]);
|
||||
} finally {
|
||||
cg.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('the specialization the call names still links its out-of-line member', async () => {
|
||||
const cg = await indexed({
|
||||
'include/fmt/base.h': BASE,
|
||||
'test/format-test.cc': INCOMPLETE,
|
||||
});
|
||||
try {
|
||||
expect(calls(cg, 'Use')).toEqual(['fmt::formatter::format (test/format-test.cc)']);
|
||||
} finally {
|
||||
cg.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('a template\'s own member defined out of line, `Box<T>::make`, is what `Box<int>::make()` calls', async () => {
|
||||
const cg = await indexed({
|
||||
'include/box.h': lines(
|
||||
'namespace lib {',
|
||||
'template <typename T>',
|
||||
'struct Box {',
|
||||
' static int make();',
|
||||
'};',
|
||||
'template <typename T>',
|
||||
'int Box<T>::make() {',
|
||||
' return 1;',
|
||||
'}',
|
||||
'} // namespace lib',
|
||||
),
|
||||
'src/use.cc': lines(
|
||||
'#include "include/box.h"',
|
||||
'namespace lib {',
|
||||
'int Make() {',
|
||||
' return Box<int>::make();',
|
||||
'}',
|
||||
'} // namespace lib',
|
||||
),
|
||||
});
|
||||
try {
|
||||
expect(calls(cg, 'lib::Make')).toEqual(['lib::Box::make (include/box.h)']);
|
||||
} finally {
|
||||
cg.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -11588,7 +11588,7 @@ export function matchByQualifiedName(
|
||||
};
|
||||
}
|
||||
|
||||
let candidates = cppSpecializationPool(keepForRef(context.getNodesByQualifiedName(ref.referenceName)), ref, context);
|
||||
let candidates = cppQualifierSpecializationPool(cppSpecializationPool(keepForRef(context.getNodesByQualifiedName(ref.referenceName)), ref, context), ref, context);
|
||||
// A C# `using X.Y;` names a namespace: one the project declares, else it is
|
||||
// the file's own (external) using — never another file's using of that name.
|
||||
// That own using ends the lookup here, and resolveOne drops it (an import
|
||||
@@ -11663,8 +11663,8 @@ export function matchByQualifiedName(
|
||||
const parts = ref.referenceName.split(/[:.]/);
|
||||
const lastName = parts[parts.length - 1];
|
||||
if (lastName) {
|
||||
const partialCandidates = cppSpecializationPool(keepForRef(context.getNodesByName(lastName))
|
||||
.filter((candidate) => endsAtScopeBoundary(candidate.qualifiedName, ref.referenceName)), ref, context);
|
||||
const partialCandidates = cppQualifierSpecializationPool(cppSpecializationPool(keepForRef(context.getNodesByName(lastName))
|
||||
.filter((candidate) => endsAtScopeBoundary(candidate.qualifiedName, ref.referenceName)), ref, context), ref, context);
|
||||
const ownLanguage = partialCandidates.filter((candidate) => sameLanguageFamily(candidate.language, ref.language));
|
||||
const pool = ownLanguage.length > 0 ? ownLanguage : partialCandidates;
|
||||
const chosen = preferCallSiteFile(pool, ref.filePath)[0];
|
||||
@@ -14630,7 +14630,7 @@ function cppMethodOf(cls: Node, name: string, ref: UnresolvedRef, context: Resol
|
||||
const named = context.getMethodMatches?.(cls.name, name, ref.language) ??
|
||||
context.getNodesByName(name).filter((n) => n.kind === 'method' && sameLanguageFamily(n.language, ref.language));
|
||||
const family = named.filter((n) => n.qualifiedName === want);
|
||||
const own = cppSpecializationPool(family, ref, context);
|
||||
const own = cppQualifierSpecializationPool(cppSpecializationPool(family, ref, context), ref, context);
|
||||
// Specializations of the class's own member template, none the call's:
|
||||
// what C++ finds is the template, which has no body here.
|
||||
if (own.length === 0 && family.length > 0) return null;
|
||||
@@ -15225,6 +15225,79 @@ function cppSpecializationPool(candidates: Node[], ref: UnresolvedRef, context:
|
||||
return named.length > 0 ? named : others;
|
||||
}
|
||||
|
||||
/**
|
||||
* The `candidates` a C++ call through a qualifier written with template
|
||||
* arguments can mean: fmt's `formatter<int>::format(…)` is never the
|
||||
* `format` a test defines for `formatter<incomplete_type>`, which the index
|
||||
* names `fmt::formatter::format` like every other specialization's. A member
|
||||
* defined out of line for a full specialization of its class stays only when
|
||||
* that specialization's arguments are the call's; a template's own members,
|
||||
* a partial specialization's and those defined in their class all stay. The
|
||||
* candidates as they are when the qualifier is written without arguments.
|
||||
*/
|
||||
function cppQualifierSpecializationPool(candidates: Node[], ref: UnresolvedRef, context: ResolutionContext): Node[] {
|
||||
if (ref.language !== 'cpp' || candidates.length === 0) return candidates;
|
||||
const args = cppQualifierArguments(ref, context);
|
||||
if (args === null) return candidates;
|
||||
return candidates.filter((n) => {
|
||||
const owner = cppSpecializationOwnerArguments(n, context);
|
||||
return owner === null || cppTemplateArgumentsMatch(owner, args);
|
||||
});
|
||||
}
|
||||
|
||||
/** The template arguments a C++ call writes on the qualifier before its name: `int` for `formatter<int>::format(…)`, null for none. */
|
||||
function cppQualifierArguments(ref: UnresolvedRef, context: ResolutionContext): string[] | null {
|
||||
const segments = ref.referenceName.split('::');
|
||||
if (segments.length < 2) return null;
|
||||
const lines = context.getFileLines?.(ref.filePath) ?? context.readFile(ref.filePath)?.split(/\r?\n/) ?? [];
|
||||
return cppTemplateArgumentsAfter(lines.slice(ref.line - 1, ref.line + 3).join('\n'), segments[segments.length - 2]!, Math.max(0, ref.column));
|
||||
}
|
||||
|
||||
/** Per context: a C++ member's id → the arguments of the full specialization it is defined out of line for (null: none). */
|
||||
const CPP_SPECIALIZATION_OWNER_ARGUMENTS = new WeakMap<ResolutionContext, Map<string, string[] | null>>();
|
||||
|
||||
/**
|
||||
* The arguments of the full specialization of its class a C++ member is
|
||||
* defined out of line for: `incomplete_type` for fmt's `auto
|
||||
* fmt::formatter<incomplete_type>::format(…)`. Null for a member defined in
|
||||
* its class, and for one whose arguments name the template parameters around
|
||||
* it, a template's own (`Foo<T>::bar`) or a partial specialization's
|
||||
* (`Foo<T*>::bar`).
|
||||
*/
|
||||
function cppSpecializationOwnerArguments(n: Node, context: ResolutionContext): string[] | null {
|
||||
if (n.language !== 'cpp' || (n.kind !== 'method' && n.kind !== 'function')) return null;
|
||||
const segments = n.qualifiedName.split('::');
|
||||
if (segments.length < 2) return null;
|
||||
let memo = CPP_SPECIALIZATION_OWNER_ARGUMENTS.get(context);
|
||||
if (!memo) CPP_SPECIALIZATION_OWNER_ARGUMENTS.set(context, (memo = new Map()));
|
||||
const hit = memo.get(n.id);
|
||||
if (hit !== undefined) return hit;
|
||||
const owner = segments[segments.length - 2]!.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
|
||||
const name = n.name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
|
||||
const lines = context.getFileLines?.(n.filePath) ?? context.readFile(n.filePath)?.split(/\r?\n/) ?? [];
|
||||
const head = lines.slice(n.startLine - 1, n.startLine + 11).join('\n').slice(n.startColumn);
|
||||
let found: string[] | null = null;
|
||||
// `Owner<args>::name`, the declarator's own qualifier.
|
||||
for (const m of head.matchAll(new RegExp(`(?<![\\w$])${owner}<`, 'g'))) {
|
||||
const open = m.index! + m[0].length - 1;
|
||||
const end = cppTemplateArgumentsEnd(head, open);
|
||||
if (end < 0 || !new RegExp(`^\\s*::\\s*~?${name}(?![\\w$])`).test(head.slice(end + 1))) continue;
|
||||
const args = splitCppTopLevel(head.slice(open + 1, end)).map(cppTemplateArgumentKey);
|
||||
// A `template <…>` with parameters on the definition makes it a template's
|
||||
// own member or a partial specialization's, whatever the arguments say:
|
||||
// abseil's `template <int max_words> … BigUnsigned<max_words>::FiveToTheNth(`.
|
||||
const before = `${lines.slice(Math.max(0, n.startLine - 3), n.startLine - 1).join('\n')}\n${head.slice(0, m.index)}`;
|
||||
const header = /\btemplate\s*<\s*(>)?(?![\s\S]*\btemplate\s*<)/.exec(before);
|
||||
const parameters = [...cppTemplateParameters(n, context)];
|
||||
const dependent = (header !== null && header[1] === undefined) ||
|
||||
args.some((a) => parameters.some((p) => new RegExp(`(?<![\\w$])${p}(?![\\w$])`).test(a)));
|
||||
found = dependent || args.length === 0 ? null : args;
|
||||
break;
|
||||
}
|
||||
memo.set(n.id, found);
|
||||
return found;
|
||||
}
|
||||
|
||||
/** Per context: a C++ function's id → the template arguments it is an explicit specialization for (null: none). */
|
||||
const CPP_SPECIALIZATION_ARGUMENTS = new WeakMap<ResolutionContext, Map<string, string[] | null>>();
|
||||
|
||||
@@ -18842,6 +18915,12 @@ export function matchMethodCall(
|
||||
return null;
|
||||
});
|
||||
if (strat1) return strat1;
|
||||
// A C++ call through a project class template's specialization written
|
||||
// with its arguments, fmt's `formatter<std::tm, Char>::format(…)`, is a
|
||||
// member of that specialization: when the template's members gave none, a
|
||||
// method picked below by the receiver's name is another class's.
|
||||
if (ref.language === 'cpp' && match === colonMatch && cppQualifierArguments(ref, context) !== null &&
|
||||
context.getNodesByName(objectOrClass!).some((n) => n.language === 'cpp' && isMethodOwnerKind(n))) return null;
|
||||
|
||||
// Built-in method names need a validated receiver (#1987). Typed, imported,
|
||||
// object-literal and direct class receivers have had their chance above;
|
||||
|
||||
Reference in New Issue
Block a user