fix(cpp): a call on a member reached through other members, like thread->rand.Next() on a Random64, links to the method of the member's class (#2627)

Step 2 of member chains; #2626 was step 1. matchCppMemberChainCall (formerly
matchCppOutsideMemberChainCall) now also resolves a chain whose member is
of a project class, in the same steps matchCppUnspelledReceiverCall takes
for `it->second.Flush()`:
- the class C++ lookup finds for the member's type, where that type is
  written;
- then its method, or one it inherits (cppMethodOf, with overloads picked
  by argument types);
- or no edge when the class surely lacks it (cppLacksMethod).

Before, such a call went by its bare name to whichever class had a method
of that name. Inside a method it could also be taken for an implicit
`this` call on the method's own class.

A member is now read from the class node C++ lookup found (cppDataMemberIn),
not from the first class of that qualified name that declares it
(cppDataMemberType, removed). rocksdb's db_bench_tool.cc and
db_stress_shared_state.h each declare a ROCKSDB_NAMESPACE::ThreadState,
whose `rand` is a Random64 in one and a Random in the other.

Kit A/B against main 6e884d7b:

| repo | added | retargeted | same target, now typed | removed |
|---|---|---|---|---|
| rocksdb | 571 | 2,008 | 1,738 | 2 |
| protobuf | 42 | 95 | 142 | 0 |
| leveldb | 42 | 30 | 141 | 0 |
| fmt | 0 | 56 | 20 | 0 |
| redis | 0 | 0 | 0 | 0 (identical) |

The most common rocksdb retargets are DBImpl -> DB (calls on a `DB*`),
BlobContents -> Slice, BlobFileReader -> FileDescriptor and
EnvWrapper -> Env. Hand-reading random samples found one wrong
retarget (db_bench's `thread->rand.Next()`, the ThreadState namesake),
which led to the member-lookup fix above. After that fix, all 76 sites
sampled across the four repos were correct:
- 30 retargets and additions, plus both removals, in rocksdb;
- 20 in protobuf, 14 in leveldb and 10 in fmt.

The two removed calls are `task_reader->rep_->GetCoroutine(…)` on a
`SstFileReader::Rep`, which has no such method. They had linked to
`CoroDB`'s.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Colby Mchenry
2026-10-16 23:13:48 +00:00
committed by GitHub
parent 6e884d7bce
commit e8ec7fd847
3 changed files with 223 additions and 44 deletions
+1 -1
View File
@@ -105,7 +105,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- In C, a call now links to the function of that name its own file can see: its own, else one a header it includes defines, like a `static inline` helper, else one another source file defines without `static`. Before, the function nearest the call was taken, and when that was another source file's `static` function, which no other file can call, the call linked to nothing: in redis, the calls to `server.c`'s `mstime()` and `ustime()` from the rest of the server were lost to the `static` copies other files keep for themselves, and jemalloc's tests' calls to the `do_arena_create` their shared header defines to a neighbouring test's own. A call to a `static inline` function that several headers define reaches the copy in a header the file includes, so protocolbuffers/protobuf's upb code no longer links to the copies in its PHP extension's combined `php-upb.h`. A source file that `#include`s another, like redis's `ae.c` with its event-loop backend or hiredis's `async.c` with `dict.c`, now calls that file's `static` functions, and a header a file includes through an include directory, like jemalloc's `#include "jemalloc/internal/bit_util.h"`, counts as included. A function a header defines only for a build that turns on a flag, like lua-cjson's `fpconv_init` under `#ifdef USE_INTERNAL_FPCONV`, gives way to another source file's definition when nothing in the project's code or build files sets that flag, so redis's calls to `fpconv_init()` reach the `fpconv.c` its build compiles. Re-index C projects after upgrading.
- 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. 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. 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.
@@ -0,0 +1,176 @@
/**
* A C++ call on a data member reached through a member chain, like rocksdb's
* `thread->rnd.Next()` on a `Random rnd` or `meta.smallest.user_key()` on an
* `InternalKey smallest`, calls the method of the class the member is
* declared as, or of a class it derives from. The extractor records such a
* call by its bare name, so it went to whichever class had a method of that
* name: `ArenaWrappedDBIter::Next`, `ClippingIterator::user_key`, or the
* calling method's own class.
*/
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 on a member chain of a project class', () => {
let root: string;
let cg: CodeGraph;
beforeAll(async () => {
root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cpp-member-chain-project-'));
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 comes first: a call matched by its bare name lands on it.
write('src/types.h', lines(
'class ArenaWrappedDBIter {',
' public:',
' void Next() {}',
'};',
'class Random {',
' public:',
' unsigned Next() { return 0; }',
'};',
'class ClippingIterator {',
' public:',
' int user_key() const { return 1; }',
'};',
'class InternalKey {',
' public:',
' int user_key() const { return 0; }',
'};',
'class InMemoryEnv {',
' public:',
' void NewWritableFile() {}',
'};',
'class Env {',
' public:',
' virtual void NewWritableFile() {}',
'};',
'class Other {',
' public:',
' void Shared() {}',
'};',
'class Base {',
' public:',
' void Shared() {}',
'};',
'class Derived : public Base {};',
'class Free {',
' public:',
' void Missing() {}',
'};',
'class Lacking {',
' public:',
' void Something() {}',
'};',
'class Cache {',
' public:',
' void Put(int k, int v) {}',
'};',
'class Table {',
' public:',
' void Put(int k) {}',
' void Put(int k, int v) {}',
'};',
'class Random64 {',
' public:',
' unsigned Next() { return 1; }',
'};',
));
// Two files each declare a `SharedState` of their own, with members of the same name.
write('src/a_shared.h', lines(
'#include "src/types.h"',
'struct SharedState {',
' Random rand;',
'};',
));
write('src/bench.cc', lines(
'#include "src/types.h"',
'struct SharedState {',
' Random64 rand;',
'};',
'void Bench(SharedState* shared) { shared->rand.Next(); }',
));
write('src/use.cc', lines(
'#include "src/types.h"',
'struct ThreadState {',
' Random rnd;',
'};',
'struct FileMeta {',
' InternalKey smallest;',
'};',
'struct Options {',
' Env* env;',
'};',
'struct Holder {',
' Derived d;',
' Lacking l;',
' Table table;',
'};',
'struct Outer {',
' ThreadState inner;',
'};',
'void Rand(ThreadState* thread) { thread->rnd.Next(); }',
'void Key(FileMeta& meta) { meta.smallest.user_key(); }',
'void EnvCall(Options& options) { options.env->NewWritableFile(); }',
'void TwoHops(Outer* o) { o->inner.rnd.Next(); }',
'void Inherited(Holder& h) { h.d.Shared(); }',
'void Lacks(Holder& h) { h.l.Missing(); }',
'void Overload(Holder& h) { h.table.Put(1, 2); }',
'class Worker {',
' public:',
' void Next() {}',
' void Step() { this->state_.rnd.Next(); }',
' private:',
' ThreadState state_;',
'};',
));
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 `name`, as `qualifiedName` (`:line` with `withLine`). */
const calls = (name: string, withLine = false): string[] => {
const caller = [...cg.getNodesByKind('function'), ...cg.getNodesByKind('method')].find((n: Node) => n.name === name);
expect(caller, name).toBeDefined();
return cg.getCallees(caller!.id).filter((r) => r.edge.kind === 'calls')
.map((r) => (withLine ? `${r.node.qualifiedName}:${r.node.startLine}` : r.node.qualifiedName)).sort();
};
it("calls the method of the member's class, not a namesake's", () => {
expect(calls('Rand')).toEqual(['Random::Next']);
expect(calls('Key')).toEqual(['InternalKey::user_key']);
expect(calls('EnvCall')).toEqual(['Env::NewWritableFile']);
});
it('through two members', () => {
expect(calls('TwoHops')).toEqual(['Random::Next']);
});
it("a method the member's class inherits, and none when the class lacks it", () => {
expect(calls('Inherited')).toEqual(['Base::Shared']);
expect(calls('Lacks')).toEqual([]);
});
it("of the class's overloads, the one the arguments fit", () => {
expect(calls('Overload', true)).toEqual(['Table::Put:49']);
});
it("a chain through `this` is no call on the calling method's own class", () => {
expect(calls('Step')).toEqual(['Random::Next']);
});
it("a member of the file's own class, not of another file's class of that name", () => {
expect(calls('Bench')).toEqual(['Random64::Next']);
});
});
+46 -43
View File
@@ -9620,31 +9620,23 @@ function cppWrittenClass(type: CppWrittenType, at: UnresolvedRef, context: Resol
return found && CPP_CLASS_KINDS.has(found.node.kind) ? found.node : null;
}
/**
* The type the C++ class `cls` declares its data member `name` with, as
* written, in the class's scope: `std::vector<SubcompactionState>` for
* rocksdb's `CompactionState::sub_compact_states`. Data members are not
* indexed, so the class's own lines are read — one inside none of the
* functions or classes nested in it. Null when none of them declares it
* (a base class may).
*/
function cppDataMemberType(cls: string, name: string, context: ResolutionContext): CppWrittenType | null {
for (const decl of context.getNodesByQualifiedName(cls)) {
const member = cppDataMemberIn(decl, name, context);
if (member) return member;
}
return null;
}
const CPP_DATA_MEMBERS = new WeakMap<ResolutionContext, Map<string, CppWrittenType | null>>();
const CPP_NESTED_SPANS = new WeakMap<ResolutionContext, Map<string, Array<[number, number]>>>();
const CPP_CLASS_PAREN_DEPTHS = new WeakMap<ResolutionContext, Map<string, number[]>>();
/**
* cppDataMemberType for one declaration of the class, `decl`. A declarator
* inside parentheses is a parameter of a method the class declares — which
* has no node of its own, so its lines are the class's — not a member:
* rocksdb's `DB` declares `CreateColumnFamilies(const
* The type the C++ class `decl` declares its data member `name` with, as
* written, in the class's scope: `std::vector<SubcompactionState>` for
* rocksdb's `CompactionState::sub_compact_states`. Data members are not
* indexed, so the class's own lines are read — one inside none of the
* functions or classes nested in it. Null when none of them declares it
* (a base class may). Only `decl`'s own: a class of the same qualified name
* another file declares for itself is another class — rocksdb's
* `db_bench_tool.cc` and `db_stress_shared_state.h` each declare a
* `ThreadState`, whose `rand` is a `Random64` in one and a `Random` in the
* other. A declarator inside parentheses is a parameter of a method the
* class declares — which has no node of its own, so its lines are the
* class's — not a member: rocksdb's `DB` declares `CreateColumnFamilies(const
* std::vector<ColumnFamilyDescriptor>& column_families, …)`. Nor is a name
* an argument list follows, a method.
*/
@@ -10031,7 +10023,7 @@ function cppExpressionType(expr: string, at: UnresolvedRef, context: ResolutionC
const element = object && cppPairMember(object, field![3]!, at, context);
if (element) return element;
const cls = object && cppWrittenClass(object, at, context);
return cls ? cppDataMemberType(cls.qualifiedName, field![3]!, context) : null;
return cls ? cppDataMemberIn(cls, field![3]!, context) : null;
}
const open = cppOpeningParen(text, text.length - 1);
const callee = open > 0 ? text.slice(0, open).trim() : '';
@@ -11055,30 +11047,42 @@ function cppMemberChainReceiver(ref: UnresolvedRef, context: ResolutionContext):
/**
* A C++ call on a data member reached through a member chain, which the
* extractor records by its bare name, calls no project method when that
* member's type is from outside the project: a library type
* (isCppLibraryType) or one under a namespace the project declares nothing
* in (isCppOutsideQualifiedType). By its bare name the call went to any
* project method of that name: rocksdb's `options.cf_paths.emplace_back(…)`
* on a `std::vector<DbPath>` reached `autovector::emplace_back`,
* `props->rep.db_id.size()` on a `std::string` `BlobContents::size`, and
* protobuf's `state_->child_pid.has_value()` on a `std::optional<pid_t>`
* `Option::has_value`. Unlike a call chain's, a member chain's operator is
* read where it is written, so a library template of a template parameter is
* no matter: `.` calls a member of `std::vector<T>` itself, and `->` on a
* `std::unique_ptr<T>` one of `T`, which is none of the project's to tell.
* - null for such a member: no project method is called.
* - undefined for any other call, or a member whose type can't be read or is
* the project's: the name strategies decide, as before.
* extractor records by its bare name, calls a member of the class that
* member's type names, looked up as C++ looks it up where the type is
* written (cppMemberObject reads each member's declared type in its class).
* By its bare name the call went to whichever class had a method of that
* name, or, in a method, to the method's own class: rocksdb's
* `thread->rnd.Next()` on a `Random rnd` reached `ArenaWrappedDBIter::Next`,
* `meta.smallest.user_key()` on an `InternalKey` `ClippingIterator::user_key`,
* and `options.cf_paths.emplace_back(…)` on a `std::vector<DbPath>`
* `autovector::emplace_back`. Unlike a call chain's, a member chain's
* operator is read where it is written, so a library template of a template
* parameter is no matter: `.` calls a member of `std::vector<T>` itself, and
* `->` on a `std::unique_ptr<T>` one of `T`.
* - The member of that class, or of a class it derives from.
* - null when no project method is called: a type from outside the project,
* a library type (isCppLibraryType) or one under a namespace the project
* declares nothing in (isCppOutsideQualifiedType); a template parameter's
* type the calling function writes; or a class that surely lacks the
* method (cppLacksMethod).
* - undefined for any other call, or a member whose type can't be read or
* placed: the name strategies decide, as before.
*/
function matchCppOutsideMemberChainCall(ref: UnresolvedRef, context: ResolutionContext): null | undefined {
function matchCppMemberChainCall(ref: UnresolvedRef, context: ResolutionContext): ResolvedRef | null | undefined {
const receiver = cppMemberChainReceiver(ref, context);
if (!receiver) return undefined;
const object = cppMemberObject(receiver.expression, receiver.op, ref, context, 0);
if (!object || isCppPointerType(object.text)) return undefined;
if (isCppLibraryType(object.text, context)) return null;
return cppClassWritten(object.text, object, ref, context) === undefined &&
isCppOutsideQualifiedType(object.text, object, context) ? null : undefined;
const found = cppClassWritten(object.text, object, ref, context);
if (found === undefined) return isCppOutsideQualifiedType(object.text, object, context) ? null : undefined;
// (As for an unspelled receiver: a template parameter another template
// writes is what the call's specialization makes it.)
if (found === null) return isCppTemplateParameterType(object) && object.scope !== context.getNodeById?.(ref.fromNodeId)?.qualifiedName ? undefined : null;
if (!found.exact || !CPP_CLASS_KINDS.has(found.node.kind)) return undefined;
const method = cppMethodOf(found.node, ref.referenceName, ref, context);
if (method) return { original: ref, targetNodeId: method.id, confidence: 0.9, resolvedBy: 'instance-method' };
return cppLacksMethod(found.node, context) ? null : undefined;
}
/**
@@ -17026,10 +17030,9 @@ function matchReferenceInner(
if (ref.language === 'cpp' && ref.referenceKind === 'calls' && /^[A-Za-z_]\w*$/.test(ref.referenceName)) {
const unspelled = nmTimedT('cppUnspelled', ref, () => matchCppUnspelledReceiverCall(ref, context));
if (unspelled !== undefined) return unspelled;
// So is one on a member reached through a chain, `options.cf_paths.emplace_back(…)`,
// when that member's type is from outside the project.
const outside = nmTimedT('cppOutsideMemberChain', ref, () => matchCppOutsideMemberChainCall(ref, context));
if (outside !== undefined) return outside;
// So is one on a member reached through a chain, `thread->rnd.Next()`.
const chained = nmTimedT('cppMemberChain', ref, () => matchCppMemberChainCall(ref, context));
if (chained !== undefined) return chained;
}
// Try strategies in order of confidence