Skip to content

fix(cpp,rust): a qualified call like Status::OK() links the class it names, not one whose name ends the same way - #2723

Merged
colbymchenry merged 1 commit into
mainfrom
claude/strange-greider-bd9f76
Oct 21, 2026
Merged

colbymchenry merged 1 commit into
mainfrom
claude/strange-greider-bd9f76

Conversation

@colbymchenry

Copy link
Copy Markdown
Owner

What was wrong

matchByQualifiedName's partial match (the fallback after the exact qualified-name lookup) took any declaration whose qualified name ended with the written text: candidate.qualifiedName.endsWith(ref.referenceName), with no scope boundary. So Status::OK matched ROCKSDB_NAMESPACE::IOStatus::OK, and CacheShard::ComputeHash matched clock_cache::ClockCacheShard::ComputeHash, and the pick among several matches was DB order.

On main, in facebook/rocksdb alone, 4,331 of 15,233 partial-match decisions were not at a boundary. About 4,100 of those were Status::OK(), Status::InvalidArgument(…), Status::Corruption(…) and their siblings landing on IOStatus's methods (IOStatus::OK sorts first). Others were base-class calls from an override (return FileSystemWrapper::NewWritableFile(…), leveldb's return DB::Put(…) in ModelDB::Put) linking to a namesake subclass or to the calling method itself.

The change

  1. Scope boundary (endsAtScopeBoundary, name-matcher.ts). A candidate's qualified name must be the written name, or end with it right after :: or .. A name written from the global scope (::Foo::bar) is never the tail of a nested one: fix(cpp): a call written from the global scope links the declaration of exactly that name #2445 already resolves C/C++ :: names before this match, and this covers the rest (Ruby ::TypeError and the like).
  2. The call's own language first. Among the boundary candidates, those in the call's language family come first. protobuf's C++ DescriptorPool::generated_pool() (41 calls) and WireFormat::MakeTag(…) (5) met a Ruby or C# namesake first in DB order, and the language gate then dropped the edge, so they linked to nothing.
  3. A C++ name qualified by a template parameter (type-parameters.ts, gateTypeParameter, so it applies to every strategy). CacheShard::ComputeHash(…) inside template <class CacheShard> class ShardedCache gets no edge, except to a member of the parameter's default (template <typename Parser = TcParser> keeps protobuf's Parser::ParseLoop → TcParser::ParseLoop, right for every caller that passes no parser). This part is needed because the boundary alone moved rocksdb's CacheShard::ComputeHash to matchMethodCall's receiver-word guess (instance-method 0.7), with the same wrong target.
  4. C++ template heads read the C++ way. A parameter is named by its last word, with its default kept: int N = 0, detail::fixed_string S, const PrefetchOpts& kOpts, std::integral T. A <…> after a C++ declaration's name is a specialization's arguments, not parameters. Before, the reader took the first word, so template <…, WireFormatLite::FieldType kKeyFieldType> declared a "parameter" WireFormatLite (the new gate would have dropped protobuf's WireFormatLite::WriteTagToArray and internal::StrongReference calls), and const PrefetchOpts& kOpts hid the existing type-reference gate's PrefetchOpts (one protobuf instantiates edge now links).

Which languages reach this match

A probe arm (main plus a log of every partial-match decision; its graphs are byte-identical to main's) over the 14 repos below: C++ 20,853 decisions (4,441 not at a boundary), Rust 200 (44: ripgrep, protobuf's rust/), Ruby 59 (all at a boundary). Java, Kotlin, C#, Python, JS/TS and Go never reach it with a candidate: their qualified names join type and member with :: (com.example::Foo::bar, Serilog.Core::Logger::Write), so a dotted ref like Foo.bar never suffix-matches one. An earlier census over 66 repos (on an older main) found the same three languages.

The file-path suffix match (~line 94): checked, unchanged

matchByFilePath's endsWith only ever compares file nodes that have exactly the ref's basename, so the suffix is already anchored unless a parent directory's name ends with the ref's first directory (mysnippets/ for snippets/…). The probe logged 5,765 of its decisions (C, C++, Objective-C, Ruby, Erlang) and none had a non-boundary candidate, so it's left alone.

Validation

Arms on main bd62d9c: main, bound (parts 1–2), fix (all four parts), plus the probe arm. They're one build with only name-matcher.js and type-parameters.js swapped, sharing the same kernel. Every graph is diffed site by site (kind, file, line, column, ref name). Re-checked on 17f1453 (#2722, a Rust declared-type reader): rocksdb, protobuf, fmt, leveldb and sidekiq are byte-identical across the two bases on both arms, and ripgrep's 38 changed sites are the same.

repo retargeted added removed metadata only
facebook/rocksdb 4,219 83 30 12
protocolbuffers/protobuf 24 47 40 28
fmtlib/fmt 7 0 21 14
google/leveldb 2 0 0 0
BurntSushi/ripgrep (Rust) 0 0 38 0
sidekiq (Ruby), gson, spring-petclinic (Java), serilog, MediatR (C#), redis (C), express (JS), requests (Python), gin (Go) byte-identical

Retargeted, all checked:

  • rocksdb: Status::* → Status::* (about 4,100 calls); TransactionDB::Open (41) away from OptimisticTransactionDB::Open; BlobFileMetaData::Create; DBImpl::Open. Base calls from an override now reach the base, or the method it inherits (fix(resolution): bind Class.method() to the class itself, not a class whose name contains it #1935): Customizable::IsInstanceOf, Configurable::PrepareOptions, DB::MultiGet. The only exception is CacheShard::…, which part 3 removes.
  • protobuf: 18 wrong → right (TestUtil::ExpectClear ×11 away from MapTestUtil, Value::default_instance, Option::MergeImpl, five FieldValuePrinter::Print* self-loops → TextFormat::FieldValuePrinter). 6 wrong → wrong: calls on the unindexed FileOptions (descriptor.pb.h is over the size cap) and on the dependent MessageTraits<T> are now guessed by receiver words instead of by suffix.
  • leveldb: ModelDB::Put / ModelDB::Delete self-loops → DB::Put / DB::Delete.
  • fmt: see the costs below.

Added, all right: rocksdb's 83 are base-class calls that main matched to a namesake subclass declared only in a .cc file, which a later rule then threw away (FileSystemWrapper::* ×77, SkipListFactory::kClassName, EventListener::OnSubcompaction*, FileSystem::SyncFile, PointLockManagerTest::SetUp). protobuf's 47 are the 46 own-language calls above plus PrefetchOpts.

Removed, every one triaged:

  • rocksdb 30: all calls through template parameters: Txn::* ×10 (SecondaryIndexMixin<Txn> : public Txn), Traits::* ×8 (GetFromBatchImpl<Traits>), CacheShard::* ×6, BitFieldT::* ×3, TCreateContext, gtest's TestClass, TComparator.
  • protobuf 40: 30 through template parameters (MessageType ×15, TypeHandler ×11, MessageT, ExtensionType, DescriptorProtoClass); 4 wrong (FileOptions::* / EnumValueOptions::* → pb::file::CppFileOptions, pb::enumvalue::JsonEnumValueOptions); 6 Rust, see the costs below.
  • fmt 21: 10 wrong (formatter<std::tm, Char>::format ×7, sys_time, std::exception → native_formatter::format, and an ostream self-loop), 5 right by coincidence, 5 dependent on T, 1 gtest TestClass.
  • ripgrep 38: 36 wrong (Path::new ×25 on std's Path → PrinterPath::new; Seq::* ×9 on regex-syntax's Seq → the file's own wrapper TSeq; 2 Separator::new under use …::FieldContextSeparator as Separator and use …::FieldMatchSeparator as Separator → ContextSeparator::new), 2 right by coincidence (Separator::disabled and Separator::new under use …::ContextSeparator as Separator).

Known costs (right edges lost):

  • fmt formatter<X>::format. The index has one fmt::formatter for every specialization. Main's suffix match landed on detail::native_formatter::format, which is right for native X (5 production calls in std.h: long long, ullong, bool ×2, unsigned) only because native types' formatter inherits from it. 7 test calls formatter<int>::format(…) now reach formatter<incomplete_type>::format, a specialization defined in another test .cc file, which is wrong. Telling formatter<int> from formatter<incomplete_type> needs specialization-aware lookup of a written qualifier (as fix(cpp): an explicit specialization, like protobuf's WireFormatLite::ReadPrimitive<int32_t, WireFormatLite::TYPE_INT32>, is named after its template #2609 did for functions), in this match and in matchMethodCall's class strategy alike. Follow-up.
  • protobuf Rust Key::get/insert/remove/iter_get/to_view (6). Key is fn map_get<Key: MapKey> and trait MapKey: … + FfiMapKey, so FfiMapKey::get was right, but only through a suffix coincidence (Key::get_prototype stays unresolved on both arms). Follow-up: resolve a call through a trait-bounded generic to the bound trait's method.
  • Template-parameter calls with a dominant instantiation (protobuf TypeHandler::* → GenericTypeHandler, rocksdb Traits::* → one of the two local Traits structs) now get no edge: silent rather than one guessed instantiation.
  • Aliases whose name ends the aliased class's (rocksdb's function-local using Phaser = PeriodicCompactionPhaser;, ConfigHelper, Data) keep the same target, now through the receiver-word strategy at 0.65–0.7 instead of qualified-name 0.85 (the metadata-only rows).

Tests

__tests__/qualified-name-boundary.test.ts, 10 cases. 7 fail on main: the rocksdb reproduction, Status::OK vs IOStatus::OK, DB::Put from ModelDB::Put, the Ruby namesake, Rust Path::new, a function template's Hasher::Hash, the mixin's Txn::Put. 3 guards pass on main: the parameter default, a non-type parameter's qualified type, an explicit specialization's arguments. A mutation run pins every part:

mutation fails
main's sources the 7 red cases
boundary only (main's type-parameters.ts) reproduction, Hasher::Hash, Txn::Put
old unanchored suffix (gate kept) Status::OK, DB::Put, Rust Path::new
no own-language preference the Ruby namesake case
no template-parameter gate reproduction, Hasher::Hash, Txn::Put
no default exception the Parser = TcParser guard
first-word C++ parameter names the non-type parameter guard
C++ after-name arguments read as parameters the specialization guard

Full suite on the merged tree (this branch on 17f1453), with the kernel built and CODEGRAPH_KERNEL_EXPECT=1, --maxWorkers=4: 670 files passed (6 skipped); 8,049 tests passed, 82 skipped, 0 failed.

Cost

None measurable. rocksdb on 17f1453, interleaved main / fix / main / fix: 61 s, 59 s, 62 s, 56 s. Single runs on bd62d9c: protobuf 181 s / 162 s; the small repos are within a second. The boundary check is a string slice, and the gate runs only for resolved C++ refs whose name starts with X::, memoized per enclosing declaration.

Overlap

🤖 Generated with Claude Code

…t names, not one whose name ends the same way

The partial qualified-name match took any declaration whose qualified
name ended with the written text, with no scope boundary. In
facebook/rocksdb about 4,100 calls to `Status::OK()`,
`Status::InvalidArgument(…)` and their siblings went to `IOStatus`'s
methods of those names; base-class calls from an override, like
`return FileSystemWrapper::NewWritableFile(…)` or leveldb's
`return DB::Put(…)`, went to a namesake subclass or to the calling
method itself; and ripgrep's `Path::new(…)` on std's `Path` went to its
own `PrinterPath::new`.

- The written name must start a scope of the candidate's: the
  candidate's qualified name is the name, or ends with it right after
  `::` or `.`.
- Among those candidates, the ones in the call's own language come
  first. protobuf's C++ `DescriptorPool::generated_pool()` met a Ruby
  namesake first, which the language gate then dropped.
- A C++ name qualified by a template parameter, like
  `CacheShard::ComputeHash` in `template <class CacheShard> class
  ShardedCache`, gets no edge from any strategy, except to a member of
  the parameter's default (`typename Parser = TcParser`). The boundary
  alone moved rocksdb's call to the receiver-word guess, with the same
  wrong target.
- The C++ template-head reader names each parameter by its last word,
  `int N`, `detail::fixed_string S`, `const PrefetchOpts& kOpts`, and
  keeps its default. A `<…>` after a C++ declaration's name is a
  specialization's arguments, not parameters. Before, `WireFormatLite::
  FieldType kKeyFieldType` made `WireFormatLite` a type parameter.

Measured on main bd62d9c and re-checked on 17f1453: rocksdb 4,219
retargeted, 83 added, 30 removed; protobuf 24, 47, 40; fmt 7, 0, 21;
leveldb 2, 0, 0; ripgrep 0, 0, 38. Nine Ruby, Java, C#, C, JS, Python
and Go controls are byte-identical. Only C++, Rust and Ruby reach this
match; the file-path suffix match is already anchored and is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant