Repository navigation
fix(cpp): a call through a namespace links no class's method by its name - #2730
Merged
Merged
Conversation
A C++ call written through a namespace, `x::m()`, reached matchMethodCall's name strategies when no declaration of `x::m` was found by qualified name. They can only return a method of some class, which a namespace never holds: the capitalized-receiver class (`internal` → a class `Internal`), the one method of the name in the project, or the best receiver-word guess. So protocolbuffers/protobuf's `absl::StripPrefix(name, …)` linked to its Rust generator's `MultiCasePrefixStripper::StripPrefix` and `absl::Status(…)` to `upb::Status`'s constructor. fmtlib/fmt's `fmt::ptr(p)` linked to a test scanner's `ptr()` method, and `fmt::day(…)` to a test's `date::day`, ahead of the step that reads the namespaces a macro opens and finds fmt's own `ptr` function and `day` alias in `FMT_BEGIN_NAMESPACE`. Those strategies now return nothing when the qualifier is a namespace (isCppNamespaceQualifier): a lowercase name that no C or C++ type or alias node has, and that the calling file declares as neither a template parameter nor an alias (cppNamesBroughtIn, now split into type names and namespace names, since a namespace the file opens confirms it is one). rocksdb's `hash_table::Insert(…)` through `typedef HashTable<…> hash_table;` keeps its edge. So does the constructor of a class the call names in that namespace (isCppConstructorThrough): protobuf's `internal::SourceWrapper<absl::Cord>(&data)` constructs `struct SourceWrapper<absl::Cord>`, a specialization the index holds without its name, whose template is only declared. A method of another template's specialization is no constructor: fmt's `fmt::scan<int>(…)` is fmt's `scan` function, not `scanner<num>::scan`. The check runs only where one of those strategies would return. 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.
Follow-up to #2729.
What was wrong
A C++ call written through a namespace,
x::m(), reachedmatchMethodCall's name strategies whenever no declaration ofx::mwas found by qualified name. Those strategies can only return a method of some class:internal→ a classInternal);A namespace holds functions and types, never methods, so for these calls every such pick is another class's member. Worse, they run before
matchCppMacroNamespaced, the step that reads the namespaces a macro opens or an alias names, so a guess there hid the right declaration. On main 009e437:absl::StripPrefix(…)×12 → the Rust generator'sMultiCasePrefixStripper::StripPrefix;absl::Status(…)×8 →upb::Status's constructor;absl::Cleanup(…)→internal::cleanup::ChunkList::Cleanup; hpb'sbackend::CreateMessage/backend::ClearMessage→hpb::internal::PrivateAccess::CreateMessageandDynamicExtensionInfoHelper::ClearMessage.fmt::ptr(p)×9 → a test scanner'sdetail::scan_buffer::ptr();fmt::scan<…>(…)×21 → a method of the test'sscanner<num>specialization;fmt::day/month/year(…)×10 → a test'sdate::classes;fmt::underlying(…)→ a ranges formatter's method. fmt's ownptr,scan,underlyingand itsday/month/yearaliases sit inFMT_BEGIN_NAMESPACE.The change
isCppNamespaceQualifier(name-matcher.ts) holds when the qualifier is lowercase, no C or C++ type or alias node has its name (cppTypesNamed), and the calling file declares it as neither a template parameter nor an alias. That last part readscppNamesBroughtInfrom #2729, now split into type names and namespace names, since a namespace the file opens confirms it is one rather than exempting it. When it holds, Strategy 2 and both Strategy 3 returns give no edge, and the later steps run as before.One exception: the constructor of a class the call names in that namespace (
isCppConstructorThrough). Its owner, right inside the written namespace, ismitself, or a specialization of the class templatem, which the index holds without a name. protobuf'sinternal::SourceWrapper<absl::Cord>(&data)constructsstruct SourceWrapper<absl::Cord>in message_lite.cc, whose template is only declared, and keeps its edge. A method of another template's specialization is no constructor: fmt'stemplate <> struct scanner<num>has ascan, butfmt::scan<int>(…)is fmt'sscanfunction.The check runs only where one of those strategies would return.
Validation
Arms on main 009e437:
main3andns3(this branch). They share one kernel, built from main's kernel sources, which this branch doesn't touch. Diffs by natural keys over nodes, every edge and every unresolved ref, plus a call-site diff:Nodes are identical everywhere. leveldb, rocksdb and redis were indexed with this change before the constructor exception was added, and were byte-identical then. The exception only applies where the guard fires, which it never did there.
Every changed site checked:
absl::StripPrefix×12,absl::Status×8 andabsl::Cleanup×1. These call abseil, which isn't in the repo; they now link to nothing.backend::CreateMessage<T>(arena)andbackend::ClearMessage(message)now reachhpb::internal::backend::upb::CreateMessage/ClearMessage. They go through the alias hpb/multibackend.h declares in its first#ifarm,namespace backend = hpb::internal::backend::upb;(the other arm names the cpp backend). That is qualified-name 0.8, from the step that follows namespace aliases, which the guess used to pre-empt.fmt::scan<…>(…)×21 → thescanfunction in test/scan.h;fmt::ptr(p)×6 →ptrin format.h, and ×3 on smart pointers (std-test.cc) → the overload in std.h;fmt::day/month/year(…)×10 → fmt'sday,month,yearaliases in chrono.h. chrono.h declares them asusing day = std::chrono::day;when the standard library has them, and as fmt's ownclass dayin the#elsearm, which the index doesn't hold;fmt::underlying(e)→underlyingin format.h.instantiates:fmt::bytes(…)andfmt::string_buffer(…)went to the constructorsbytes::bytesandstring_buffer::string_buffer, and now reach fmt'sstruct bytesandclass string_buffer, as a construction does.Kept by the constructor exception: protobuf's
internal::SourceWrapper<absl::Cord>(&data)×4, and gmock'sinternal::RefMatcher<T&>(x)in fmt's vendored googletest, which constructsclass RefMatcher<T&>. Kept because the qualifier is a type: rocksdb'shash_table::Insert/Find/Erase/GetMutex×4 throughtypedef HashTable<T*, Hash, Equal> hash_table;.Tests
__tests__/cpp-namespace-qualifier.test.ts, 8 cases. 5 fail on main:ptr;dayalias;internal::Fill→ a classInternal, the word guesslog::Flush→LogWriter, andpython::Escapethrough a namespace the calling file opens;fmt::scan<int>besidescanner<num>::scan.3 guards pass on main: the
SourceWrapperspecialization's constructor, rocksdb'shash_tabletypedef, and one case that combines a function's alias, an alias a header declares, a template parameter's default and a lowercase class.A mutation run (runtime-false edits, so the setup build still compiles) pins every part:
scanptr,day, abseil, each strategy's pick,scanSourceWrapperscanFull suite on 009e437 plus this change, with the kernel built,
CODEGRAPH_KERNEL_EXPECT=1and--maxWorkers=4: 682 files (6 all-skipped), 8,110 tests passed, 82 skipped, 0 failed. That includes #2729'scpp-undeclared-qualifier.test.ts, whosecppNamesBroughtInthis change splits.Cost
None measurable. Index wall times, main3 / this branch: protobuf 174 s / 150 s, rocksdb 56 s / 54 s, the small repos within a second. The check runs only where Strategy 2 or 3 is about to return a pick for a C++
x::m()call. Its scan of the calling file is #2729's, memoized per file, and a specialization's head is read only for an unnamed owner.🤖 Generated with Claude Code