trending/codegraph
1
0
Fork 0
mirror of https://github.com/colbymchenry/codegraph.git synced 2026-10-23 06:50:54 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(cpp): a member declared in an anonymous union, like protobuf's union { Impl_ _impl_; };, is found as a member of the class around it (#2731)

Browse Source 
A class's data members are read from its own lines, skipping those of
the functions and classes nested in it. The index keeps an anonymous
union or struct as a nested `<anonymous>` node, so its lines were skipped
too, though C++ looks its members up as the enclosing class's
([class.union.anon]).

Every generated protobuf message keeps its fields in `union { Impl_
_impl_; };`, so a call through `_impl_` was guessed by the method's name:
- `this_._impl_.type_url_.Destroy()` reached `Arena::Destroy`;
- `_impl_._cached_size_.Get()` reached `FieldArenaRep::Get`;
- `_impl_._has_bits_.Clear()` reached the message's own `Clear`.

fmt's `value` keeps its argument in a union of `string`, `custom` and the
rest in the same way.

cppNestedSpans now leaves out an anonymous union or struct with no name
and no declarator (isCppAnonymousMembers):
- a bare `union {` or `struct {` head;
- `};` after the closing brace.
A union with a declarator, `union { … } u;`, keeps its members to itself,
and so does a partial specialization, which the index also keeps as
`<anonymous>` but whose head names its template.

A member-chain diagnostic on main 81fd0078 found the anonymous union
behind most of protobuf's untyped chains. Kit A/B against 81fd0078:

| repo | removed | added | retargeted | same target, now typed |
|---|---|---|---|---|
| protobuf | 2 | 54 | 349 | 231 |
| fmt | 0 | 0 | 1 | 1 |
| rocksdb | 0 | 0 | 2 | 0 |
| leveldb, redis | identical | | | |

- Retargets go from bare-name guesses to the member's class:
  ArenaStringPtr (Set, Get, IsDefault, Destroy), CachedSize::Get,
  HasBits::Clear, RepeatedPtrField, SourceContext and MessageLite.
- The 2 removals are `ptr.string_value->clear()` on a `std::string*`, which
  had reached `Map::clear`.
- fmt's and rocksdb's retargets pick the overload their argument fits now
  that a union member has a type: `map(value_.uint128_value)` takes
  `native_uint128`, and `Add(…, props.oldest_ancestor_time)` the `uint64_t`
  overload.
- About 45 changed sites read by hand were all right.

Mutations: dropping the nested-span exclusion, the declarator check or
the head check each fail the new tests. Dropping the name check survives,
because the head check covers named types; it stays as a cheap early exit.
A matching change to the caller's-declaration check left the kit A/B
byte-identical, so it was not kept.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Colby Mchenry
2026-10-21 06:55:34 +00:00
committed by GitHub
co-authored by Claude Opus 5.5
parent 81fd007850
commit 5994edb34e
3 changed files with 195 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGELOG.md
+1
View File
@@ -130,6 +130,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- In C++, text inside a string literal is no longer taken for a variable's declaration. That covers the C code protocolbuffers/protobuf's upb generator writes in a raw string (`R"cc( … upb_MiniTableField field = $field_init$; … )cc"`) and log messages like facebook/rocksdb's `"… in WriteBatch handler; …"`. Before, a later call like `field.ctype()` on the function's own `upb::FieldDefPtr field` parameter, or `handler->PutCF(…)` on a `WriteBatch::Handler* handler`, was matched against the type that text seemed to name, so it linked to nothing or to another class's method. Re-index C++ projects after upgrading.
- In C++, a call on an `auto` variable now links to the method of the class C++ finds for the type its initializer gives it, as a call on a variable declared with that type does. The type is read from a cast or smart pointer written out, like facebook/rocksdb's `auto iter = std::unique_ptr<Iterator>(…)` or `auto* mutex = static_cast<InstrumentedMutex*>(arg)`, from `new` or `std::make_unique` of a class, and from what a function or method is declared to return, like the `Iterator*` of `txn->GetIterator(…)`, read to the end of an initializer that goes on to the next line. Before, such a variable took the type of an earlier variable of the same name, or none: in RocksDB, `iter->SeekToFirst()` reached `MemTableRep::Iterator` and `mutex->Lock()` reached `port::Mutex`. A chained call like `cfd->current()->storage_info()` now looks up the class the inner call is declared to return the same way, through a smart pointer too, so protocolbuffers/protobuf's Java generator no longer reaches the C# generator's `EnumGenerator`. A type from outside the project no longer links to a class of the project that shares its name: protobuf's `status.ok()` on an `absl::Status` reached `upb::Status::ok`, and fmtlib/fmt's `os.flush()` on a `std::ostream&` reached fmt's own `ostream::flush`. The same holds for a variable declared in a macro body, or typed by what a function in another file returns, and for a class the index doesn't hold whose name only another class shares: protobuf's `_status.ok()` in its `RETURN_IF_ERROR` macro reached `upb::Status::ok`, and `range.options()` on its generated `DescriptorProto::ExtensionRange` reached `Descriptor::ExtensionRange::options`. A namespace a macro stands for is the project's under either name: facebook/rocksdb declares its classes in `namespace ROCKSDB_NAMESPACE` with `#define ROCKSDB_NAMESPACE rocksdb`, so `db->Put(…)` on a `rocksdb::DB*` links to RocksDB's own `DB::Put`. Re-index C++ projects after upgrading.
- In C++, a call on a data member of the calling method's class, like `versions_->GetColumnFamilySet()` in facebook/rocksdb's `db_impl_write.cc`, now links to the method of the member's type wherever the class is declared. Before, only the call's own file and the header of the same name were read for the member, and RocksDB declares `DBImpl` in `db_impl.h` while its methods are spread over `db_impl_write.cc`, `db_impl_compaction_flush.cc`, `db_impl_open.cc` and more, so such calls were guessed by the member's name. A member declared below an inline method that uses it, one a base class declares, and, in a gtest `TEST_F` or `TEST_P` body, one of the test's fixture, like RocksDB's `db_`, are found the same way, and a member of a class nested in the caller's class is no longer taken for the caller's own. A call on a member of a standard-library type, like `std::atomic<bool> shutting_down_`, no longer links to a project method that only shares the method's name. A call on a member reached through other members now links the same way, like RocksDB's `thread->rand.Next()` on a `Random64` or `meta->largest.user_key()` on an `InternalKey`. Before, it went to whichever class had a method of that name, like `ArenaWrappedDBIter::Next` or `ClippingIterator::user_key`, or, inside a method, to that method's own class. When the member's class has no such method, the call no longer links to another class's. The member is read from the class the calling file sees: RocksDB's `db_bench_tool.cc` and its stress tool each declare their own `ThreadState`, whose `rand` is a `Random64` in one and a `Random` in the other. It is also found in the classes that class derives from. RocksDB's `Options` inherits its members from `DBOptions` and `ColumnFamilyOptions`, so `options.statistics->getTickerCount(…)` now reaches `Statistics`, and `options.table_factory.reset(…)` on a `std::shared_ptr` no longer links to a project method of that name. Re-index C++ projects after upgrading.
- In C++, a member declared inside an anonymous union or struct is now found as a member of the class around it, as C++ looks it up. Every protocolbuffers/protobuf generated message keeps its fields in `union { Impl_ _impl_; };`, so calls like `_impl_.name_.Set(…)` now link to `ArenaStringPtr::Set`. Before, they were guessed by the method's name, and `this_._impl_.type_url_.Destroy()` linked to `Arena::Destroy`. The same goes for fmtlib/fmt's `value`, which keeps its argument in a union. Re-index C++ projects after upgrading.
- In C++, a variable, parameter or member whose declaration puts a qualifier or an annotation around its name is now read for its type, so calls on it link to the declared class's methods: a `const` after the `*` or `&`, like facebook/rocksdb's `Iterator* const iter = iter_guard.get();` or google/leveldb's `Env* const env_;`, a type written before its `const`, like `Slice const& key`, a nullability annotation after the `*`, like protocolbuffers/protobuf's `io::EpsCopyOutputStream* PROTOBUF_NONNULL stream`, and a thread-safety annotation after the name, like LevelDB's `MemTable* imm_ GUARDED_BY(mutex_);`. Before, such a declaration was skipped, or the annotation was taken for its type, so the call took the type of an earlier variable of the same name or was guessed by the method's name: RocksDB's `iter->SeekToFirst()` reached the unrelated `ArenaWrappedDBIter`, and LevelDB's `env_->NewWritableFile(…)` reached `InMemoryEnv`. Re-index C++ projects after upgrading.
- In C++, a call on a name a structured binding declares now links to the method of the type C++ gives it. That covers a range-based `for` over pairs or a map, like facebook/rocksdb's `for (auto& [cfh, iter] : cfh_iter_pairs_) { iter->Valid(); }`, and a declaration like protocolbuffers/protobuf's `auto [dst_ext, is_new] = Insert(…);`. A call on what a `std::function` or another function wrapper returns, like `create_iterator()->Valid()` on a `std::function<Iterator*()>`, links the same way, and so does a call on a class member whose type is written on the line above its name. Before, these calls were guessed by the method's name, and calls on a library container declared that way, like a `std::priority_queue` member's `pop()`, reached a project class's method of that name. Re-index C++ projects after upgrading.
- In C++, a call written without a receiver in a gtest `TEST_F` or `TEST_P` body now links to the test fixture's method, its own or one it inherits, as C++ finds it: facebook/rocksdb's tests call `DBTestBase`'s `Put(…)`, `Flush()` and `NewIterator(…)` this way. A variable such a call initializes, like `auto iter = NewIterator(…)`, takes the type the method returns, so `iter->SeekToFirst()` reaches the iterator class. Before, those calls were guessed by name and often reached a namesake in another class, like RocksDB's `ColumnFamilyTestBase::Put` or protocolbuffers/protobuf's `EncodeDecodeTest::ExpectNoErrors`, and `iter->SeekToFirst()` reached the unrelated `MemTableRep::Iterator`. A `->` call on a value of a type from outside the project, like protobuf's `m->value()` on an `absl::StatusOr<Any>`, no longer links to a class of the project that shares the type's name, like protobuf's own `hpb::StatusOr`. Re-index C++ projects after upgrading.
__tests__/cpp-anonymous-union-members.test.ts
+171
View File
@@ -0,0 +1,171 @@
/**
* The members of a C++ anonymous union are the members of the class it is
* declared in: protobuf's generated messages hold their fields in `union {
* Impl_ _impl_; };`, and fmt's `value` its argument in a union of `string`,
* `custom` and the rest. The union's lines were read as a nested class's, so
* a call through `_impl_.type_url_` or `value_.string` was guessed by the
* method's name: `this_._impl_.type_url_.Destroy()` reached `Arena::Destroy`.
*/
import { describe, it, expect, beforeAll, afterAll } 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 lines = (...l: string[]): string => [...l, ''].join('\n');
describe('a C++ call through a member of an anonymous union', () => {
let root: string;
let cg: CodeGraph;
beforeAll(async () => {
root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cpp-anonymous-union-'));
const write = (file: string, text: string): void => {
fs.mkdirSync(path.dirname(path.join(root, file)), { recursive: true });
fs.writeFileSync(path.join(root, file), text);
};
// Each namesake sits where a call matched by its bare name lands on it.
write('src/arenastring.h', lines(
'namespace google {',
'namespace protobuf {',
'class StringNamesake {',
' public:',
' void ClearToEmpty() {}',
'};',
'class Arena {',
' public:',
' static void Destroy() {}',
'};',
'class SizeNamesake {',
' public:',
' int Get() const { return 0; }',
'};',
'namespace internal {',
'class ArenaStringPtr {',
' public:',
' void ClearToEmpty() {}',
' void Destroy() {}',
'};',
'class CachedSize {',
' public:',
' int Get() const { return 1; }',
'};',
'} // namespace internal',
'} // namespace protobuf',
'} // namespace google',
));
write('src/any.pb.h', lines(
'#include "src/arenastring.h"',
'namespace google {',
'namespace protobuf {',
'class Any final {',
' public:',
' void clear_type_url();',
' int GetCachedSize() const { return _impl_._cached_size_.Get(); }',
' static void SharedDtor(Any& this_);',
' private:',
' struct Impl_ {',
' ::google::protobuf::internal::ArenaStringPtr type_url_;',
' ::google::protobuf::internal::CachedSize _cached_size_;',
' };',
' union { Impl_ _impl_; };',
'};',
'inline void Any::clear_type_url() { _impl_.type_url_.ClearToEmpty(); }',
'} // namespace protobuf',
'} // namespace google',
));
write('src/any.pb.cc', lines(
'#include "src/any.pb.h"',
'namespace google {',
'namespace protobuf {',
'void Any::SharedDtor(Any& this_) { this_._impl_.type_url_.Destroy(); }',
'} // namespace protobuf',
'} // namespace google',
));
write('src/value.h', lines(
'class Inner {',
' public:',
' void Flush() {}',
'};',
'class Other {',
' public:',
' void Flush() {}',
'};',
'class string_value {',
' public:',
' const char* data() const { return ""; }',
'};',
'class StrNamesake {',
' public:',
' const char* data() const { return ""; }',
'};',
'class value {',
' public:',
' union {',
' int int_value;',
' string_value text;',
' };',
'};',
'struct Arg {',
' value value_;',
' const char* Data() { return value_.text.data(); }',
'};',
'struct Tagged {',
' union { Inner a; } u;',
' Other a;',
'};',
'void UseTagged(Tagged& t) { t.a.Flush(); }',
'struct Outer {',
' struct Named { Inner b; } named;',
' Other b;',
'};',
'void UseOuter(Outer& o) { o.b.Flush(); }',
// The index keeps a partial specialization as `<anonymous>` too.
'struct Box {',
' template <typename T> struct Helper { Other h; };',
' template <typename T> struct Helper<T*> {',
' Inner h;',
' };',
' Other h;',
'};',
'void UseBox(Box& b) { b.h.Flush(); }',
));
cg = await CodeGraph.init(root, { index: true });
}, 120_000);
afterAll(() => {
cg?.close();
if (root) fs.rmSync(root, { recursive: true, force: true });
});
/** `calls` callees of the function or method `qualifiedName`, as `qualifiedName`. */
const calls = (qualifiedName: string): string[] => {
const caller = [...cg.getNodesByKind('function'), ...cg.getNodesByKind('method')]
.find((n: Node) => n.qualifiedName === qualifiedName);
expect(caller, qualifiedName).toBeDefined();
return cg.getCallees(caller!.id).filter((r) => r.edge.kind === 'calls').map((r) => r.node.qualifiedName).sort();
};
it("a method defined outside its class reaches the class's union member", () => {
expect(calls('google::protobuf::Any::clear_type_url')).toEqual(['google::protobuf::internal::ArenaStringPtr::ClearToEmpty']);
});
it('a method inside the class reaches a union member declared below it', () => {
expect(calls('google::protobuf::Any::GetCachedSize')).toEqual(['google::protobuf::internal::CachedSize::Get']);
});
it("a chain through an object's union member", () => {
expect(calls('google::protobuf::Any::SharedDtor')).toEqual(['google::protobuf::internal::ArenaStringPtr::Destroy']);
});
it("a union member of a class-typed member of the caller's class", () => {
expect(calls('Arg::Data')).toEqual(['string_value::data']);
});
it('a union with a declarator, a named nested struct or a specialization keeps its members to itself', () => {
expect(calls('UseTagged')).toEqual(['Other::Flush']);
expect(calls('UseOuter')).toEqual(['Other::Flush']);
expect(calls('UseBox')).toEqual(['Other::Flush']);
});
});
src/resolution/name-matcher.ts
+23 -2
View File
@@ -13651,7 +13651,28 @@ function cppClassParenDepths(
return depths;
}
/** The lines of the functions and classes nested in the C++ class declaration `decl`, which ends on line `end`. */
/**
* Is the C or C++ declaration `n` an anonymous union or struct, with no name
* and no declarator, whose members are those of the class it is declared in?
* C++ looks them up as that class's own: protobuf's generated messages hold
* their fields in `union { Impl_ _impl_; };`, and fmt's `value` its argument
* in a union of `string`, `custom` and the rest. Not one with a declarator,
* `union { … } u;`, whose members are `u`'s, nor a specialization the index
* keeps as `<anonymous>`, whose head names its template.
*/
function isCppAnonymousMembers(n: Node, context: ResolutionContext): boolean {
if (n.name !== '<anonymous>' || (n.kind !== 'union' && n.kind !== 'struct')) return false;
const lines = context.getFileLines?.(n.filePath) ?? context.readFile(n.filePath)?.split(/\r?\n/);
const head = lines?.[n.startLine - 1]?.slice(n.startColumn ?? 0);
const foot = lines?.[(n.endLine ?? n.startLine) - 1];
return !!head && !!foot && /^\s*(?:union|struct)\s*\{/.test(head) && /\}\s*;\s*(?:\/\/.*)?$/.test(foot);
}
/**
* The lines of the functions and classes nested in the C++ class declaration
* `decl`, which ends on line `end` — not an anonymous union's or struct's,
* whose members are the class's own (isCppAnonymousMembers).
*/
function cppNestedSpans(decl: Node, end: number, context: ResolutionContext): Array<[number, number]> {
let memo = CPP_NESTED_SPANS.get(context);
if (!memo) CPP_NESTED_SPANS.set(context, (memo = new Map()));
@@ -13659,7 +13680,7 @@ function cppNestedSpans(decl: Node, end: number, context: ResolutionContext): Ar
if (!spans) {
spans = context.getNodesInFile(decl.filePath)
.filter((n) => n.id !== decl.id && n.startLine >= decl.startLine && (n.endLine ?? n.startLine) <= end &&
(isCppFunctionNode(n) || CPP_CLASS_KINDS.has(n.kind)))
(isCppFunctionNode(n) || (CPP_CLASS_KINDS.has(n.kind) && !isCppAnonymousMembers(n, context))))
.map((n): [number, number] => [n.startLine, n.endLine ?? n.startLine]);
memo.set(decl.id, spans);
}