Repository navigation
fix(cpp): a call on an inherited member reached through a chain, like options.statistics->getTickerCount(…) on a RocksDB Options, links to the member's class - #2628
Merged
Conversation
… `options.statistics->getTickerCount(…)` on a RocksDB `Options`, links to the member's class 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 e8ec7fd: | 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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Step 3 of C++ member-chain calls, after #2626 (outside types) and #2627 (project classes). The type of
x.memberwas read fromx's class alone, so a member that class inherits was never found, and the call fell back to its bare method name. The commonest case is RocksDB'sOptions, which declares no members of its own and inherits all of them fromDBOptionsandColumnFamilyOptions:options.statistics->getTickerCount(…)was guessed;options.table_factory.reset(…)on astd::shared_ptrlinked toFSWritableFilePtr::reset.Why this step
A temporary diagnostic on main recorded why each chain call that still fell through couldn't be typed. Across leveldb, fmt, protobuf and rocksdb:
Options'statistics648,table_factory552,listeners169,prefix_extractor153, …)_impl_)How
cppExpressionType's member branch now usescppInheritedDataMember, the lookup the calling class's own members already use (#2533):Steps 1 and 2 then apply as before: an inherited library-typed member links to no project method, and an inherited project-typed member links to its class's method.
Validation
Kit A/B against main
e8ec7fd8:std::shared_ptrandstd::vectormembers:options.table_factory.reset(…)/prefix_extractor/merge_operator→FSWritableFilePtr::reset(654);options_.db_paths.emplace_back(…)→autovector::emplace_back(256).options.statistics->getTickerCount(…)→Statistics(432);options_.rate_limiter->GetTotalRequests()→RateLimiter(51);options.comparator->Name()→Comparator(26).DummyOldStats→Statistics,DB→Env,ClockCacheShard→Cache, andtracked_locks_->GetPointLockStatus()→LockTracker.this_._internal_metadata_.Clear<…>(),MergeFrom<…>andunknown_fields<…>now reachinternal::InternalMetadata, the member every message inherits fromMessageLite. Before, they reached the message's ownClearorAny::MergeFrom.All 50 random sites read by hand were correct: 12 removals, 12 additions and 12 retargets in rocksdb, plus 12 retargets and both additions in protobuf.
Tests.
__tests__/cpp-member-chain-base-members.test.ts(5 tests):std::shared_ptr's ownreset();4 of the 5 fail on main; the control passes on both. All 3 mutations are killed: own class only, bases before own, and one level of bases only.
Full suite: 588 files and 7,229 tests pass, apart from 2 timeouts in
git-index-currency.test.tsunder load. That file passes 13/13 twice on its own and also timed out on main earlier today.The CHANGELOG gets sentences in the existing unreleased C++ data-member bullet. Re-index C++ projects after upgrading.
🤖 Generated with Claude Code