From e2b0cf5eea53fa5fb2dc5990b798e66223ae6280 Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Sat, 17 Oct 2026 02:05:39 +0000 Subject: [PATCH] =?UTF-8?q?fix(cpp):=20a=20call=20on=20an=20inherited=20me?= =?UTF-8?q?mber=20reached=20through=20a=20chain,=20like=20`options.statist?= =?UTF-8?q?ics->getTickerCount(=E2=80=A6)`=20on=20a=20RocksDB=20`Options`,?= =?UTF-8?q?=20links=20to=20the=20member's=20class=20(#2628)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 3 of member chains, after #2626 and #2627. The type of `x.member` was read from x's class alone, so a member that class inherits was never found and the call fell back to its bare name. A temporary diagnostic on main measured why chain calls still fell through: 2,801 of 4,884 were a member declared in a base class. Nearly all were rocksdb's `Options`, which declares no members of its own and inherits `statistics`, `table_factory`, `listeners`, `prefix_extractor` and the rest from `DBOptions` and `ColumnFamilyOptions`. The other causes are left for later: - a chain whose base can't be typed: 875; - a class the index lacks, e.g. protobuf's generated `_impl_`: 626; - a member found nowhere: 337. cppExpressionType's member branch now uses cppInheritedDataMember, the lookup the calling class's own members already use (#2533): - the class's own declaration first; - then the classes it derives from, through their base edges, to depth 4; - a base that depends on the class's template parameters is skipped unless a using-declaration brings the name in. Kit A/B against main e8ec7fd8: | repo | removed | added | retargeted | same target, now typed | |---|---|---|---|---| | rocksdb | 1,410 | 634 | 392 | 184 | | protobuf | 0 | 2 | 149 | 55 | | leveldb, fmt, redis | identical | | | | - Removed: the bare-name guesses on inherited std::shared_ptr and std::vector members, such as `options.table_factory.reset(…)` → FSWritableFilePtr::reset (654) and `options_.db_paths.emplace_back(…)` → autovector::emplace_back (256). - Added: `options.statistics->getTickerCount(…)` → Statistics (432), `rate_limiter` → RateLimiter (51), `comparator` → Comparator (26). - Retargeted: DummyOldStats -> Statistics, DB -> Env, ClockCacheShard -> Cache. - protobuf's generated `this_._internal_metadata_.Clear<…>()`, `MergeFrom<…>` and `unknown_fields<…>` now reach InternalMetadata, a member every message inherits from MessageLite. Before, they reached the message's own `Clear` or `Any::MergeFrom`. All 50 random sites read by hand (36 rocksdb, 14 protobuf) were correct. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- .../cpp-member-chain-base-members.test.ts | 131 ++++++++++++++++++ src/resolution/name-matcher.ts | 9 +- 3 files changed, 138 insertions(+), 4 deletions(-) create mode 100644 __tests__/cpp-member-chain-base-members.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 17eefc2b5..f418d584f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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(…)` or `auto* mutex = static_cast(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 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 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 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 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`, 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`, 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. diff --git a/__tests__/cpp-member-chain-base-members.test.ts b/__tests__/cpp-member-chain-base-members.test.ts new file mode 100644 index 000000000..c2a2d5299 --- /dev/null +++ b/__tests__/cpp-member-chain-base-members.test.ts @@ -0,0 +1,131 @@ +/** + * A C++ call on a data member that the object's class inherits, reached + * through a member chain, calls a method of the member's class. rocksdb's + * `Options` declares no members of its own: `options.statistics` and + * `options.table_factory` are `DBOptions`' and `ColumnFamilyOptions`'. Only + * the object's own class was read for the member, so such calls were guessed + * by the method's name. + */ +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 through an inherited member', () => { + let root: string; + let cg: CodeGraph; + + beforeAll(async () => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cpp-member-chain-base-')); + 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 ArenaStats {', + ' public:', + ' long getTickerCount(int t) const { return 0; }', + '};', + 'class Statistics {', + ' public:', + ' virtual long getTickerCount(int t) const { return 1; }', + '};', + 'class BlockNamesake {', + ' public:', + ' const char* Name() const { return "block"; }', + '};', + 'class TableFactory {', + ' public:', + ' virtual const char* Name() const { return "table"; }', + '};', + 'class MemTableNamesake {', + ' public:', + ' void CreateMemTableRep() {}', + '};', + 'class MemTableRepFactory {', + ' public:', + ' virtual void CreateMemTableRep() {}', + '};', + 'class FilePtr {', + ' public:', + ' void reset() {}', + '};', + 'class Other {', + ' public:', + ' void Flush() {}', + '};', + 'class Inner {', + ' public:', + ' void Flush() {}', + '};', + )); + write('src/options.h', lines( + '#include ', + '#include "src/types.h"', + 'struct DBOptions {', + ' Statistics* statistics = nullptr;', + '};', + 'struct AdvancedColumnFamilyOptions {', + ' MemTableRepFactory* memtable_factory = nullptr;', + '};', + 'struct ColumnFamilyOptions : public AdvancedColumnFamilyOptions {', + ' std::shared_ptr table_factory;', + '};', + 'struct Options : public DBOptions, public ColumnFamilyOptions {};', + 'struct Shadowing : public DBOptions {', + ' Inner statistics;', + '};', + )); + write('src/use.cc', lines( + '#include "src/options.h"', + 'void Stats(Options& options) { options.statistics->getTickerCount(0); }', + 'void Table(Options* options) { options->table_factory->Name(); }', + 'void Memtable(const Options& opts) { opts.memtable_factory->CreateMemTableRep(); }', + 'void HolderOwn(Options& options) { options.table_factory.reset(); }', + 'void Shadowed(Shadowing& s) { s.statistics.Flush(); }', + 'struct Holder {', + ' Options opts;', + '};', + 'void Deep(Holder* h) { h->opts.statistics->getTickerCount(1); }', + )); + 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 `name`, as `qualifiedName`. */ + const calls = (name: string): string[] => { + const caller = cg.getNodesByKind('function').find((n: Node) => n.name === name); + expect(caller, name).toBeDefined(); + return cg.getCallees(caller!.id).filter((r) => r.edge.kind === 'calls').map((r) => r.node.qualifiedName).sort(); + }; + + it("a member the object's class inherits from either base", () => { + expect(calls('Stats')).toEqual(['Statistics::getTickerCount']); + expect(calls('Table')).toEqual(['TableFactory::Name']); + }); + + it('a member inherited through two levels', () => { + expect(calls('Memtable')).toEqual(['MemTableRepFactory::CreateMemTableRep']); + }); + + it("an inherited library member's own method calls none of the project's", () => { + expect(calls('HolderOwn')).toEqual([]); + }); + + it("the class's own member before a base's of the same name", () => { + expect(calls('Shadowed')).toEqual(['Inner::Flush']); + }); + + it('an inherited member further down a chain', () => { + expect(calls('Deep')).toEqual(['Statistics::getTickerCount']); + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index d87d84adf..d3cf05b88 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -10016,14 +10016,17 @@ function cppExpressionType(expr: string, at: UnresolvedRef, context: ResolutionC return followed && cppSubscriptElement(followed, at, context); } if (!text.endsWith(')')) { - // `compact_->sub_compact_states`: a data member of the object's class; - // `it->second` through a map's iterator: the member of the `std::pair`. + // `compact_->sub_compact_states`: a data member of the object's class, or + // of a class it derives from — rocksdb's `Options` declares none of its + // own, and inherits `table_factory`, `statistics` and the rest from + // `DBOptions` and `ColumnFamilyOptions`; `it->second` through a map's + // iterator: the member of the `std::pair`. const field = /^(.+?)\s*(\.|->)\s*([A-Za-z_]\w*)$/.exec(text); const object = field && cppMemberObject(field[1]!, field[2]!, at, context, depth); const element = object && cppPairMember(object, field![3]!, at, context); if (element) return element; const cls = object && cppWrittenClass(object, at, context); - return cls ? cppDataMemberIn(cls, field![3]!, context) : null; + return cls ? cppInheritedDataMember([cls], field![3]!, context, 0, new Set([cls.qualifiedName])) : null; } const open = cppOpeningParen(text, text.length - 1); const callee = open > 0 ? text.slice(0, open).trim() : '';