Repository navigation
fix(cpp): -> on an absl::StatusOr reaches the class it holds; a using-declared member of a dependent base is found - #2539
Merged
Conversation
…xture's method The index holds a gtest `TEST_F(Fixture, Name)` / `TEST_P` body as a function named `TEST_F`; gtest makes it a method of a class derived from `Fixture`. #2533 gave such a body the fixture's data members (cppTestFixture), not its methods. rocksdb's tests call `DBTestBase`'s `Put(…)`, `Flush()`, `Key(i)` and `NewIterator(…)` bare, and those calls were guessed by name, often reaching a namesake in another class at 0.4 (`ColumnFamilyTestBase::Put`); `auto iter = NewIterator(…)` gave `iter` no type, so `iter->SeekToFirst()` reached the nested `MemTableRep::Iterator`. - cppImplicitThisMethod is #2479's lookup, taken out of matchCppImplicitThisCall: the caller's class, its bases, the classes around it, with cppTestFixture as the owner of a gtest body. matchCppImplicitThisCall keeps the local-shadowing check for the link. - cppExpressionType types a bare call by the same lookup, as on `this`, instead of the class's first declaration. Of two classes sharing a name in different test files it reads the caller's own, and in a template a dependent base a using-declaration names still counts, as before (protobuf's `using RepeatedFieldWrapper<T>::MutableRepeatedField;`). `this` in a gtest body points to the fixture. - A `->` call on a value of a type from outside the project no longer takes the type's last name for the class it is in (isCppArrowThroughOutsideType); the name strategies decide. matchCppDeclaredTypeCall steps aside for `->` on a value before #2532's outside-type check, so typing JsonTest's `auto m = ToProto<Any>(…)` (an `absl::StatusOr<Any>`) had sent `m->value()` to protobuf's own `hpb::StatusOr::value` at 0.9. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ng-declared member of a dependent base is found Two gaps in C++ member lookup. In protobuf, `absl::StatusOr<FeatureResolver> resolver = …; resolver->MergeFeatures(…)` was guessed by the method's name: the declared type's last name, `StatusOr`, is no class of the project's, and only std's smart pointers and optional were followed to what they hold. In rocksdb's typed_cache.h, `using BasicTypedCacheHelperFns<TValue>::DownCastValue;` makes a bare `DownCastValue(v)` the base's, but a bare name skipped every base that depends on the class's template parameters. - cppHeldType (was cppStdHeldType) also reads `absl::StatusOr`, `absl::optional` and `absl::NoDestructor` as holders, and matchCppHeldTypeCall takes one whose name no project type has (isCppLibraryHolder) — std's holders as before. - cppUsingDeclares reads the `using Base::name;` declarations a class body writes. cppScopeMethod and cppInheritedDataMember look a bare name up in a dependent base when the class brings it in from that base. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md # src/resolution/name-matcher.ts
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
1 participant
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.
Stacked on #2537 (base branch
claude/cpp-test-fixture-calls). Merge #2537 first; GitHub then retargets this PR tomain.Summary
Two gaps in C++ member lookup:
->on an abseil holder. In protocolbuffers/protobuf,absl::StatusOr<FeatureResolver> resolver = FeatureResolver::Create(…); resolver->MergeFeatures(…)was guessed by the method's name. The declared type's last name,StatusOr, is no class of the project's, and onlystd::unique_ptr/std::shared_ptr/std::optionalwere followed to what they hold.typed_cache.h,using BasicTypedCacheHelperFns<TValue>::DownCastValue;makes a bareDownCastValue(v)the base's. But a bare name skipped every base that depends on the class's template parameters, which is C++'s rule only without a using-declaration.What changed (name-matcher.ts)
cppHeldType(wascppStdHeldType) also readsabsl::StatusOr,absl::optionalandabsl::NoDestructoras holders.matchCppHeldTypeCalltakes a holder whose qualified name no project type has (isCppLibraryHolder). That is the same rule std's holders had throughisCppLibraryType, which also wanted a lowercase name.cppUsingDeclaresreads theusing Base<…>::name;declarations a class body writes. A bare call (cppScopeMethod) and a bare data member (cppInheritedDataMember) are looked up in a dependent base when the class brings that name in from that base.Validation
Site-keyed edge diffs (
diff-sites.mjs) of #2537's headb0ed652cagainst this branch. Both arms were built from the same tree, kernel included.Every site was read:
loc->file_name()/loc->line()on anabsl::optional<SourceLocation>.taken->AsView(),type_url->AsView()andtype_url->ToString()on anabsl::StatusOr<MaybeOwnedString>.tmp_container->field()/temp_container->field()on anabsl::NoDestructor<FieldWithArena<…>>.msg_ptr->get()on anabsl::StatusOr<PythonConstMessagePointer>(wasScopedPythonPtr::getat 0.65);MutableRepeatedField(…)calls inRepeatedFieldPrimitiveAccessor, which writesusing RepeatedFieldWrapper<T>::MutableRepeatedField;(wasRepeatedPtrFieldWrapper's at 0.7, nowRepeatedFieldWrapper's at 0.9).has_value()on anabsl::optional(directly, or held by anabsl::StatusOr): it is the library's, notOption::has_value;m->string_value()/m2->struct_value()on anabsl::StatusOr<Proto>whoseProtois a template parameter (a generated test message): notgoogle::protobuf::Value's.resolver->MergeFeatures(…)/status_or_mutator->get()calls keep their target, now at 0.9 by lookup instead of a 0.7 / 0.65 guess;typed_cache.h(DownCastValue,GetBasicHelper, …) keep their target, now placed by class lookup instead of name match.Tests
__tests__/cpp-absl-holder-receiver.test.tshas 4 cases checking targets and confidence:->onabsl::StatusOr,absl::optionalandabsl::NoDestructor, beside a namesake method;absl::StatusOris no holder;The red phase on #2537's code fails all but the project's-own-
StatusOrguard.Mutation check. 7 guards were each disabled alone (
mutate-sor.mjs), and each one fails a test:I dropped one check I'd first written, the
</>balance check inisCppLibraryHolder. Its mutation survived, because a type with unbalanced brackets is already turned away earlier (normalizeCppTypeName→cppTypeBeganAbove).Full suite: 6,978 passed, 82 skipped. 6 tests failed in 3 files (git index currency, the MCP daemon PID-reuse takeover, the daemon rebuild), all in areas this change does not touch. The run overlapped a mutation run, and those 3 files pass when rerun alone (17 passed).
🤖 Generated with Claude Code