Repository navigation
fix(cpp): a call through a class the index doesn't hold gets no guess by a shared word - #2729
Merged
Merged
Conversation
… by a shared word A C++ call written through a class, `X::m()`, whose class has no node in the index fell through to the last strategy of matchMethodCall, which scores every method named `m` by the words its class shares with `X` and picks one. protocolbuffers/protobuf declares its descriptor messages in descriptor.pb.h, which is over the indexer's size cap, so `FeatureSet::default_instance()` linked to `UnknownFieldSet::default_instance`, `FeatureSetDefaults::GetDescriptor()` to `JavaFeatures_NestInFileClassFeature::GetDescriptor`, and the options messages' `default_instance()` to `pb::enumvalue::JsonEnumValueOptions`'. A class whose name only shares a word with `X` is another class: one named `X` was looked for first, by qualified name and by class name. That guess is now dropped when `X` is a capitalized name (isUndeclaredTypeName) that no C or C++ class, struct, union, enum, alias or namespace node has, and that the calling file makes nothing of itself (cppNamesBroughtIn): not a template parameter of any `template <…>` head in it, not an alias it declares (a function-local `using FD = FieldDescriptor;`, a typedef, or one a macro it expands declares, like rocksdb's IMPORT_RIBBON_IMPL_TYPES), not a namespace it opens, aliases or uses, and not googletest's TypeParam or TestFixture in a typed test. The one method of a name in the project is still taken as before. The scans read the file once (cppTypeNamesDeclared in cpp-type-aliases.ts, shared with the template-head reader) and the project's headers once for macro bodies, only when a guess would otherwise be returned. 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.
What was wrong
A C++ call written through a class,
X::m(), whose class has no node in the index reached the last strategy ofmatchMethodCall. That strategy scores every method namedmby the words its class's name shares withXand returns the best one at instance-method 0.65. A class whose name only shares a word withXis another class: one namedXwas already looked for, by qualified name and by class name.protocolbuffers/protobuf declares its descriptor messages (
FeatureSet,FileDescriptorProto,FileOptions,OneofOptions, …) indescriptor.pb.h, which is over the indexer's 1 MB file cap. So those classes have no nodes, and neither do their inline staticsdefault_instance(),descriptor()andGetDescriptor(); only the methodsdescriptor.pb.ccdefines out of line are indexed. On main f66d064, 35 such calls take a guess:FeatureSet::default_instance()×20 (descriptor.cc, descriptor_unittest.cc) →google::protobuf::UnknownFieldSet::default_instance("Set");FeatureSetDefaults::GetDescriptor()(pyext/descriptor_pool.cc:598) →pb::JavaFeatures_NestInFileClassFeature::GetDescriptor("Feature");FileOptions×2,ExtensionRangeOptions×2,OneofOptions,EnumOptions,EnumValueOptions,ServiceOptions,MethodOptions::default_instance()(descriptor.cc) →pb::enumvalue::JsonEnumValueOptions::default_instance("Options");FileDescriptorProto::descriptor()×3 andFileOptions::descriptor()(compiler tests) →compiler::CodeGeneratorResponse_File::descriptor("File");InteropTestMessage::GetDescriptor()(rust/test/cpp/interop/test_utils.cc, a generated test message the repo doesn't hold, named by a using-declaration) →Message::GetDescriptor. The generated staticGetDescriptor()hidesMessage's member.Some of the issue's own examples changed on main since it was written. Strategy 3 now scores only the candidates the calling file's includes can see, when there are any (
withinCppTranslationUnit, #2684). SoFeatureSet::descriptor()in code_generator.cc:71 and the Python extension'sOneofOptions/ServiceOptions::default_instance()already link to nothing on main. The 35 above remain because a namesake is in view there.The change
isCppUndeclaredQualifier(name-matcher.ts) holds when the qualifier is a capitalized name (isUndeclaredTypeName, as for inferred receiver types) that no C or C++ class, struct, union, enum, alias, namespace or module node has (cppTypesNamed), and that the calling file makes nothing of itself (cppNamesBroughtIn):template <…>head in the file. fmt'sTraits::count()insidetemplate <…, typename Traits = buffer_traits> class iterator_bufferkeeps its guess,buffer_traits::count, which is right for the parameter's default. The scan reads the file's text, because the index holds that class without its name.usingortypedef, in a function body too (protobuf'susing FD = FieldDescriptor;), or one a project macro the file expands declares. rocksdb'sIMPORT_RIBBON_IMPL_TYPES(TypeParam)declaresusing SimpleSoln = …;, and its 10 right guesses stay.namespace X {,namespace X = …;,using namespace …;), as the issue asked.TypeParamandTestFixturein aTYPED_TESTbody.When it holds, the 0.65 receiver-word guess returns null instead. The other strategies run as before: qualified-name and class lookup before it, exact name, fuzzy and macro-namespace matching after it. The unique-name pick (0.7, the one method of a name in the project) is untouched. On protobuf, its picks for undeclared qualifiers are right or one hop off:
TestFixture::CheckRequired×10, andMapTestPeer::…×8, a class declared inmap_test.inc(not read as C++) whose calls reach theKeyMapBasemethods it forwards to.cppTypeNamesDeclared(cpp-type-aliases.ts) reads template-head parameters and alias names from source text. It shares the per-parameter reader withcollectTemplateHeader(addTemplateParameter, factored out unchanged). The file scan runs once per calling file that reaches the guard. The macro scan runs once per resolver context and reads only#definelines: about 10 ms over protobuf's 641 headers.Validation
Arms on main f66d064:
mainandfix(this branch). They share one kernel, built from main's kernel sources, which this branch doesn't touch. Every repo was diffed by natural keys over nodes, every edge and every unresolved ref, plus a call-site diff.Nodes are identical everywhere. Each removed edge's ref is now an unresolved
failedref, and nothing else moved.Removed, every one checked wrong:
gmock.hcallsDefaultValue<Result>::Exists()and::Get(), which went totesting::internal::BuiltInDefaultValue::Exists/::Get("Default", "Value"). gmock's primarytemplate <typename T> class DefaultValue(gmock.h:1203) has no node, and the call is to its own static, which then callsBuiltInDefaultValue.Exempt, as asked. Every other 0.65 guess through a qualifier with no class node on the five repos stays:
Traits);Printer,Policyin gtest.h);A second fix arm, built after the macro scan was rewritten to read
#definelines first, reproduces the first byte for byte on protobuf, fmt and rocksdb. On all five repos, both versions of the scan find the same macro → names map.Tests
__tests__/cpp-undeclared-qualifier.test.ts, 10 cases. 3 fail on main: the reproduction, the Python extension's calls plus one through a using-declaration, and an undeclared class's out-of-line method beside a guessed call. 7 guards pass on main: a project class and a nested class, namespaces (a project one, an alias, abseil), a macro's alias, a function'susingandtypedefplus an alias node, a template parameter's default,TestFixture, and the unique-name pick, pinned toinstance-method. A mutation run (runtime-false edits, so the setup build still compiles) pins every part but one:TestFixtureTestFixturecppTypeNamesDeclared)usingaliases not readusingtypedefThe last one survives because the carve-out only keeps main's behavior: no fixture or repo has a call through a capitalized namespace the calling file names that Strategy 3 would guess for. Asserting that would mean asserting a guess.
Full suite on f66d064 plus this change, with the kernel built,
CODEGRAPH_KERNEL_EXPECT=1and--maxWorkers=4: 681 files (6 all-skipped), 8,101 tests passed, 82 skipped, 0 failed. The branch was then rebased onto accf600 (#2728, TypeScriptdeclare moduleresolution only, nothing on the C++ path), with a keep-both CHANGELOG resolution. On the rebased tree, tsc passes, and so do this file and #2728'sambient-module-imports.test.ts(41 tests).Cost
None measurable. Index wall times on this machine, which was busy with other sessions' jobs, main / fix / fix2: protobuf 258 s / 267 s / 210 s, rocksdb 92 s / 61 s / 69 s, the small repos within a second. The guard runs only where Strategy 3 is about to return a 0.65 guess for a C++
X::m()call. It reads each such calling file once, and the project's#definelines once per resolver context.Not changed
absl::StripPrefix(…)×12 →rust::MultiCasePrefixStripper::StripPrefix,absl::Status(…)×8 →upb::Status::Status, and fmt'sfmt::ptr(p)×9 →detail::scan_buffer::ptrandfmt::day/month/year(…)×10 → the test'sdate::classes, where fmt's ownptrfunction anddayclass exist. rocksdb'shash_table::…×4 through a lowercase typedef are right, and so are calls of a specialization's constructor. That is the follow-up PR.🤖 Generated with Claude Code