mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-17 05:07:37 +08:00
fix(cpp): an alias of a type written from the global scope names that global type (#2568)
protobuf's hpb declares `class Arena { … backend::Arena arena_; }`, where
`namespace backend = hpb::internal::backend::upb;` and that namespace says
`using Arena = ::upb::Arena;`. cppTypeName dropped the leading `::`, so the
alias's target was looked up as `upb::Arena` from the alias's own scope,
whose nearest `upb` is that very namespace: the alias named itself, the
lookup ended on no class, and `arena_.Fuse(other.arena_)` fell back to the
type's last name, linking `hpb::Arena::Fuse` (and `IsFused`, `RefArena`) to
itself instead of `upb::Arena::Fuse`.
TypeName now records a leading `::` (`global`), and a global name is looked
up from the global scope alone: when an alias's target is followed (follow,
and the scope expand looks the target up from and hands on), in a class's
base clause (baseClassesOf), and in cppAliasedTypeWritten, which #2532's
`auto` locals and chains walk alias by alias.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
+1
-1
@@ -87,7 +87,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
- An import in a Vue, Svelte or Astro component now links the component to the file it imports, as an import in a script does, and so does an import of a scoped package that a `tsconfig` path or a workspace package maps to a project file, like `import { debounce } from '@excalidraw/common'`. Before, only the names such an import binds were linked, so a barrel or a module imported for its side effects showed no dependency from those files in `codegraph affected`, and `codegraph sync` could not update them when a barrel they import through was deleted or added. `codegraph sync` and the file watcher also link such an import again, and one written as `.` or `..`, when the file it names comes back. Re-index JavaScript, TypeScript, Vue, Svelte and Astro projects after upgrading.
|
||||
- `codegraph sync` and the file watcher now move an import written without an extension, like `import { sizeOf } from './button'`, to a file added beside the one it found before when the new file comes first, such as `button.ts` next to `button.vue`. Before, the import and the names it binds stayed linked to `button.vue` until the importing file changed or the project was re-indexed. JavaScript, TypeScript, Vue, Svelte and Astro.
|
||||
- In C, C++ and Objective-C, a struct or class member declared as a pointer, reference, array or function, like `SharedState* shared;`, `jv elements[];` or `virtual Status Put(…) = 0;`, no longer makes the struct look like it inherits from the member's type. Before, `codegraph_explore` and impact listed those types as base classes, every struct holding a pointer to a type as one of its subclasses, and a method of that struct as an override of the type's method with the same name. Real base classes like `class Derived : public Base`, Objective-C superclasses and Go's embedded fields are unchanged. Re-index C, C++ and Objective-C projects after upgrading.
|
||||
- In C++, a call on a variable, parameter or member declared through a `typedef` or `using` alias now links to the method of the type the alias names, with the alias looked up the way C++ does it: in the calling function, its class and the classes that class inherits from, then the namespaces around it. In google/leveldb, `table_.Insert(buf)` on a `Table table_;` declared next to `typedef SkipList<const char*, KeyComparator> Table;` now reaches `SkipList::Insert` instead of the unrelated `HandleTable::Insert`, and `Table::Iterator iter(&table_);` reaches `SkipList::Iterator`'s constructor. A class that only shares the alias's name is no longer taken for it, and when the aliased type has no such method, as with an alias of `std::vector` or of a template parameter, no method is guessed from the receiver's name. Re-index C++ projects after upgrading.
|
||||
- In C++, a call on a variable, parameter or member declared through a `typedef` or `using` alias now links to the method of the type the alias names, with the alias looked up the way C++ does it: in the calling function, its class and the classes that class inherits from, then the namespaces around it. In google/leveldb, `table_.Insert(buf)` on a `Table table_;` declared next to `typedef SkipList<const char*, KeyComparator> Table;` now reaches `SkipList::Insert` instead of the unrelated `HandleTable::Insert`, and `Table::Iterator iter(&table_);` reaches `SkipList::Iterator`'s constructor. A class that only shares the alias's name is no longer taken for it, and when the aliased type has no such method, as with an alias of `std::vector` or of a template parameter, no method is guessed from the receiver's name. An alias of a type written from the global scope names that global type, also inside a namespace of the same name: protocolbuffers/protobuf's `using Arena = ::upb::Arena;` in `hpb::internal::backend::upb`, so hpb's `arena_.Fuse(…)` on a `backend::Arena` now reaches `upb::Arena::Fuse` instead of the calling method itself. Re-index C++ projects after upgrading.
|
||||
- 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 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.
|
||||
|
||||
@@ -0,0 +1,216 @@
|
||||
/**
|
||||
* A C++ type written from the global scope (`::upb::Arena`) is looked up there
|
||||
* alone, also where an alias names it and where a class's base clause or a
|
||||
* declaration writes it.
|
||||
*
|
||||
* protocolbuffers/protobuf's hpb declares `class Arena { … backend::Arena
|
||||
* arena_; }`, where `namespace backend = hpb::internal::backend::upb;` and
|
||||
* that namespace says `using Arena = ::upb::Arena;`. The alias was looked up
|
||||
* as if written `upb::Arena`, from its own namespace, whose nearest `upb` is
|
||||
* that namespace itself: the alias named itself, no class was found, and
|
||||
* `arena_.Fuse(other.arena_)` fell back to the type's last name, landing on
|
||||
* the calling method `hpb::Arena::Fuse`.
|
||||
*/
|
||||
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-global-alias-'));
|
||||
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`. */
|
||||
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)
|
||||
.sort();
|
||||
}
|
||||
|
||||
const lines = (...l: string[]): string => [...l, ''].join('\n');
|
||||
|
||||
const UPB_ARENA = lines(
|
||||
'namespace upb {',
|
||||
'class Arena {',
|
||||
' public:',
|
||||
' bool Fuse(Arena& other) { return true; }',
|
||||
' bool IsFused(Arena& other) const { return false; }',
|
||||
' void RefArena(const Arena& to) {}',
|
||||
'};',
|
||||
'} // namespace upb',
|
||||
);
|
||||
|
||||
describe('a C++ alias of a type written from the global scope', () => {
|
||||
it('reproduction: protobuf hpb\'s `backend::Arena` is `upb::Arena`, not the calling method\'s own class', async () => {
|
||||
const cg = await indexed({
|
||||
'upb/mem/arena.hpp': UPB_ARENA,
|
||||
'hpb/multibackend.h': lines(
|
||||
'#define HPB_INTERNAL_BACKEND_UPB 1',
|
||||
'#define HPB_INTERNAL_BACKEND_CPP 2',
|
||||
'namespace hpb {',
|
||||
'namespace internal {',
|
||||
'namespace backend {',
|
||||
'namespace upb {}',
|
||||
'namespace cpp {}',
|
||||
'} // namespace backend',
|
||||
'} // namespace internal',
|
||||
'#if HPB_INTERNAL_BACKEND == HPB_INTERNAL_BACKEND_UPB',
|
||||
'namespace backend = hpb::internal::backend::upb;',
|
||||
'#elif HPB_INTERNAL_BACKEND == HPB_INTERNAL_BACKEND_CPP',
|
||||
'namespace backend = hpb::internal::backend::cpp;',
|
||||
'#endif',
|
||||
'} // namespace hpb',
|
||||
),
|
||||
'hpb/backend/types.h': lines(
|
||||
'#include "hpb/multibackend.h"',
|
||||
'#include "upb/mem/arena.hpp"',
|
||||
'namespace hpb {',
|
||||
'namespace internal {',
|
||||
'namespace backend {',
|
||||
'#if HPB_INTERNAL_BACKEND == HPB_INTERNAL_BACKEND_UPB',
|
||||
'namespace upb {',
|
||||
'using Arena = ::upb::Arena;',
|
||||
'}',
|
||||
'#elif HPB_INTERNAL_BACKEND == HPB_INTERNAL_BACKEND_CPP',
|
||||
'namespace cpp {',
|
||||
'using Arena = google::protobuf::Arena;',
|
||||
'}',
|
||||
'#endif',
|
||||
'} // namespace backend',
|
||||
'} // namespace internal',
|
||||
'} // namespace hpb',
|
||||
),
|
||||
'hpb/arena.h': lines(
|
||||
'#include "hpb/backend/types.h"',
|
||||
'namespace hpb {',
|
||||
'class Arena {',
|
||||
' public:',
|
||||
' bool Fuse(Arena& other) { return arena_.Fuse(other.arena_); };',
|
||||
' bool IsFused(Arena& other) { return arena_.IsFused(other.arena_); };',
|
||||
' void RefArena(const Arena& to) { arena_.RefArena(to.arena_); }',
|
||||
' private:',
|
||||
' backend::Arena arena_;',
|
||||
'};',
|
||||
'} // namespace hpb',
|
||||
),
|
||||
});
|
||||
try {
|
||||
expect(calls(cg, 'hpb::Arena::Fuse')).toEqual(['upb::Arena::Fuse']);
|
||||
expect(calls(cg, 'hpb::Arena::IsFused')).toEqual(['upb::Arena::IsFused']);
|
||||
expect(calls(cg, 'hpb::Arena::RefArena')).toEqual(['upb::Arena::RefArena']);
|
||||
} finally {
|
||||
cg.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('names the global class, not a namesake in the scope around the alias', async () => {
|
||||
const cg = await indexed({
|
||||
'upb/mem/arena.hpp': UPB_ARENA,
|
||||
'hpb/arena.cc': lines(
|
||||
'#include "upb/mem/arena.hpp"',
|
||||
'namespace hpb {',
|
||||
'namespace upb {',
|
||||
'class Arena {',
|
||||
' public:',
|
||||
' bool Fuse(Arena& other) { return false; }',
|
||||
'};',
|
||||
'} // namespace upb',
|
||||
'using GlobalArena = ::upb::Arena;',
|
||||
'bool Join(GlobalArena& a, GlobalArena& b) {',
|
||||
' return a.Fuse(b);',
|
||||
'}',
|
||||
'} // namespace hpb',
|
||||
),
|
||||
});
|
||||
try {
|
||||
expect(calls(cg, 'hpb::Join')).toEqual(['upb::Arena::Fuse']);
|
||||
} finally {
|
||||
cg.close();
|
||||
}
|
||||
});
|
||||
|
||||
it('through an `auto` variable the alias types', async () => {
|
||||
const cg = await indexed({
|
||||
'upb/mem/arena.hpp': UPB_ARENA,
|
||||
'hpb/arena.cc': lines(
|
||||
'#include "upb/mem/arena.hpp"',
|
||||
'namespace hpb {',
|
||||
'namespace upb {',
|
||||
'using Arena = ::upb::Arena;',
|
||||
'} // namespace upb',
|
||||
'class Arena {',
|
||||
' public:',
|
||||
' bool Fuse(Arena& other) { return false; }',
|
||||
'};',
|
||||
'upb::Arena MakeArena() { return upb::Arena(); }',
|
||||
'bool Join(upb::Arena& other) {',
|
||||
' auto made = MakeArena();',
|
||||
' return made.Fuse(other);',
|
||||
'}',
|
||||
'} // namespace hpb',
|
||||
),
|
||||
});
|
||||
try {
|
||||
expect(calls(cg, 'hpb::Join').filter((c) => c.endsWith('::Fuse'))).toEqual(['upb::Arena::Fuse']);
|
||||
} finally {
|
||||
cg.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('other C++ names written from the global scope', () => {
|
||||
it('a base class: `::lib::Options`, not the namespace\'s own `lib::Options`', async () => {
|
||||
const cg = await indexed({
|
||||
'src/config.cc': lines(
|
||||
'namespace app {',
|
||||
'struct Inner {',
|
||||
' void Run() {}',
|
||||
'};',
|
||||
'} // namespace app',
|
||||
'namespace lib {',
|
||||
'struct Options {',
|
||||
' struct Inner {',
|
||||
' void Run() {}',
|
||||
' };',
|
||||
'};',
|
||||
'} // namespace lib',
|
||||
'namespace app {',
|
||||
'namespace lib {',
|
||||
'struct Options {};',
|
||||
'} // namespace lib',
|
||||
'class Config : public ::lib::Options {};',
|
||||
'void Use() {',
|
||||
' Config::Inner inner;',
|
||||
' inner.Run();',
|
||||
'}',
|
||||
'} // namespace app',
|
||||
),
|
||||
});
|
||||
try {
|
||||
expect(calls(cg, 'app::Use')).toEqual(['lib::Options::Inner::Run']);
|
||||
} finally {
|
||||
cg.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -62,6 +62,13 @@ export interface CppAliasedType {
|
||||
interface TypeName {
|
||||
names: string[];
|
||||
templated: boolean[];
|
||||
/**
|
||||
* Written from the global scope (`::upb::Arena`), so looked up there alone:
|
||||
* protobuf's `namespace hpb::internal::backend::upb { using Arena =
|
||||
* ::upb::Arena; }` would otherwise find its own namespace's `upb::Arena`,
|
||||
* the alias itself.
|
||||
*/
|
||||
global?: boolean;
|
||||
}
|
||||
|
||||
/** `SkipList<const char*, Cmp<int>>::Iterator` → `SkipList::Iterator`. */
|
||||
@@ -105,13 +112,19 @@ function cppTypeName(raw: string): TypeName | null {
|
||||
}
|
||||
parts.push(part.trim());
|
||||
templated.push(args);
|
||||
if (parts[0] === '') {
|
||||
// A leading `::` names the global scope.
|
||||
// A leading `::` names the global scope.
|
||||
const global = parts[0] === '';
|
||||
if (global) {
|
||||
parts.shift();
|
||||
templated.shift();
|
||||
}
|
||||
if (parts.length === 0 || !parts.every((p) => /^[A-Za-z_]\w*$/.test(p))) return null;
|
||||
return { names: parts, templated };
|
||||
return global ? { names: parts, templated, global } : { names: parts, templated };
|
||||
}
|
||||
|
||||
/** The scopes a written name is looked up from: the global scope alone for `::Name`. */
|
||||
function scopesFor(name: TypeName, scopes: readonly string[]): readonly string[] {
|
||||
return name.global ? [''] : scopes;
|
||||
}
|
||||
|
||||
/** Is a written type a pointer (`Table*`, `const Foo* const`), not counting its template arguments? */
|
||||
@@ -430,7 +443,7 @@ function baseClassesOf(cls: string, context: ResolutionContext): string[] {
|
||||
for (const part of splitTopLevel(text.slice(colon + 1, brace))) {
|
||||
const name = cppTypeName(part.replace(/\b(?:public|protected|private|virtual)\b/g, ' '));
|
||||
if (!name) continue;
|
||||
const base = classNamed(name.names, scopesWithin(parentScope(cls)), context);
|
||||
const base = classNamed(name.names, scopesFor(name, scopesWithin(parentScope(cls))), context);
|
||||
if (base && base !== cls && !bases.includes(base)) bases.push(base);
|
||||
}
|
||||
}
|
||||
@@ -621,7 +634,7 @@ export function cppAliasedTypeWritten(
|
||||
): { text: string; alias: Node } | null | undefined {
|
||||
const name = cppTypeName(written);
|
||||
if (!name) return undefined;
|
||||
const { found, next } = lookupSegments(name, scopes, ref, context);
|
||||
const { found, next } = lookupSegments(name, scopesFor(name, scopes), ref, context);
|
||||
if (!found || typeof found !== 'object' || 'cls' in found) return undefined;
|
||||
if ('ambiguous' in found || next !== name.names.length || isAliasTemplate(found.alias, context)) return null;
|
||||
const text = declarationText(found.alias, context);
|
||||
@@ -689,7 +702,7 @@ function follow(
|
||||
context: ResolutionContext,
|
||||
hops: number,
|
||||
): CppAliasedType | null | undefined {
|
||||
const { found, next } = lookupSegments(name, scopesWithin(scope), ref, context);
|
||||
const { found, next } = lookupSegments(name, scopesFor(name, scopesWithin(scope)), ref, context);
|
||||
if (!found || typeof found !== 'object' || !('alias' in found)) return undefined;
|
||||
const alias = found.alias;
|
||||
if (hops >= MAX_HOPS) return null;
|
||||
@@ -721,11 +734,13 @@ function expand(
|
||||
if (!target || parameters?.has(target.names[0]!)) return null;
|
||||
const pointer = isCppPointerType(written);
|
||||
const names = [...target.names, ...rest];
|
||||
const further = follow({ names, templated: [...target.templated, ...restTemplated] }, scope, ref, context, hops);
|
||||
const further = follow({ names, templated: [...target.templated, ...restTemplated], global: target.global }, scope, ref, context, hops);
|
||||
if (further === null) return null;
|
||||
if (further) return { ...further, pointer: further.pointer || pointer };
|
||||
const cls = classNamed(names, scopesWithin(scope), context);
|
||||
// `::upb::Arena` is looked up from the global scope, whatever scope writes it.
|
||||
const from = target.global ? '' : scope;
|
||||
const cls = classNamed(names, scopesWithin(from), context);
|
||||
const classTemplate = !!cls && context.getNodesByQualifiedName(cls)
|
||||
.some((n) => isCFamily(n) && CLASS_KINDS.has(n.kind) && templateParametersAround(n, context).size > 0);
|
||||
return { target: target.names, rest: [...rest], scope, pointer, classTemplate };
|
||||
return { target: target.names, rest: [...rest], scope: from, pointer, classTemplate };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user