Repository navigation
fix(cpp): a range-based for loop's variable has the type the loop declares - #2439
Merged
Merged
Conversation
…lares
C++ receiver inference read a declaration only when the declared name was
followed by `;`, `=`, `,`, `)`, `[`, `{` or `(`, so a range-based for's
`Type name :` was never read. In protocolbuffers/protobuf's conformance
runner, `suite->SetVerbose(…)`, `suite->RunSuite(…)` and
`suite->GetFailureListFlagName()` inside `for (ConformanceTestSuite *suite :
suites)` fell through to Strategy 3's guess by the receiver's name, and in
RocksDB `log->file()` on a `log::Writer` loop variable went to
`BlobLogWriter::file`.
A line the declarator regex doesn't match is now read as a range-based for
when its `for (` header declares the receiver before a single `:` (not
`::`), and the loop's variable is taken only when the call is inside the
loop's body: past the header, up to the `}` closing a braced body or the end
of a single-statement body, with comments and literals skipped. After the
loop, or in a later function, the same name is another variable, and the
scan goes on as before. A loop over `auto` elements has nothing to read and
keeps the existing fallback. Bit-fields and the scope operator are never
read as a loop's declaration.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-696753 # Conflicts: # CHANGELOG.md # src/resolution/name-matcher.ts
…s not end the loop Receiver inference now reads comment lines blanked (#2413), so the commented-out loops in the range-for tests no longer reach the loop-body scan; this case keeps its own comment and literal handling covered. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-696753 # Conflicts: # CHANGELOG.md
bompus
added a commit
to bompus/codegraph
that referenced
this pull request
Oct 10, 2026
…eceivers (#430) * fix(context): call paths label synthesized hops like explore, not as events (colbymchenry#2428) Two surfaces label a synthesized (dynamic-dispatch) `calls` hop: codegraph_explore's Flow and dynamic-dispatch links plus the codegraph_node trail (`dynamic: interface → impl @file:line`), and ContextBuilder's "## Call paths", which `codegraph context` prints (`→[…]`). Each kept its own list. The call paths' list stopped at callback, react-render, jsx-render, vue-handler, http-client, queue-job and socket messages, so every other synthesizer fell through to `event ${m.event ?? ''}`: an interface-impl hop read `→[event @src/pdf-exporter.ts:5]`, and so did a C++ override, a redux thunk, a gin middleware chain or a closure collection. The wording now lives once, in src/graph/synthesized-hop.ts (`describeSynthesizedHop`: a summary, the detail only the call paths print, and the wiring site), as AGENTS.md asks of a derivation two surfaces render. ToolHandler renders `dynamic: <summary> @site`, the call paths `<summary><detail> @site`. Explore and the node trail print exactly what they printed before; the call paths keep their wording for the kinds they already labelled, including the queue name, a socket's direction and the HTTP clause, and now name the rest the way explore does. A bus event reads `bus event` there too, and the section's footnote no longer says hops are marked `[callback/event …]`. The helper also holds the wording colbymchenry#2414 gives a Go method a struct gets by embedding (`metadata.promotedInto`), so that arm moves here too. The unused long `label` form of ToolHandler.synthEdgeNote (last read by the removed codegraph_trace) is dropped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(sync): a route rendering a same-file lazy value follows its module, and a new component gets its JSX edges (colbymchenry#2452) A route that renders `const Docs = lazy(() => import('./pages/Docs'))` binds to the component that module exports, and to the declaration while the module is missing or exports none (colbymchenry#2400's declaredComponent). The reference had resolved, so no failed-ref retry saw it, and CG-33's rebind reached it only when the module's component shared the declaration's name: a sync that added the module, gave it its default export or moved that export left the route on its old answer. A data router's `lazy` route kept a moved default export the same way. FrameworkResolver.lazyModules names the modules a route's answer reads, including the files a barrel forwards it through (colbymchenry#2436); React implements it for lazy-import references and the same-file lazy values Pattern 1 reads. After the CG-33 rebind, sync puts back the route references whose modules it added or changed (reopenRouteModuleReaders), in the order their edges were written, for the orphan sweep. The jsx-render refresh read only each changed file's own content, so a new component without JSX of its own never got the edge from `<Team />` in an unchanged file. A sync whose definition delta holds a JSX-renderable node now refreshes synthesis. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(cpp): a range-based for loop's variable has the type the loop declares (colbymchenry#2439) C++ receiver inference read a declaration only when the declared name was followed by `;`, `=`, `,`, `)`, `[`, `{` or `(`, so a range-based for's `Type name :` was never read. In protocolbuffers/protobuf's conformance runner, `suite->SetVerbose(…)`, `suite->RunSuite(…)` and `suite->GetFailureListFlagName()` inside `for (ConformanceTestSuite *suite : suites)` fell through to Strategy 3's guess by the receiver's name, and in RocksDB `log->file()` on a `log::Writer` loop variable went to `BlobLogWriter::file`. A line the declarator regex doesn't match is now read as a range-based for when its `for (` header declares the receiver before a single `:` (not `::`), and the loop's variable is taken only when the call is inside the loop's body: past the header, up to the `}` closing a braced body or the end of a single-statement body, with comments and literals skipped. After the loop, or in a later function, the same name is another variable, and the scan goes on as before. A loop over `auto` elements has nothing to read and keeps the existing fallback. Bit-fields and the scope operator are never read as a loop's declaration. A loop's declaration is recorded like any other (colbymchenry#2413's noteCppDeclaration), so the std-type gate applies to `for (const std::string& s : …)` too, and a loop that doesn't enclose the call still marks the receiver as shadowed. With colbymchenry#2421, the 13 conformance runner calls reach ConformanceTestSuite's methods at 0.9. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(js,ts): a default import is what `export default` declares, not the first exported function (colbymchenry#2432) A default import resolved to the module's first exported function or component whenever the `export default` statement wrote a declaration (`export default function Vans()`, `export default class Store`): only a separate `export default NAME` statement was read. A React Router 6.4+ data-router page exports its `loader` or `action` above the page component, so `import Vans, { loader as vansLoader } from './Vans'` bound the route `/vans` (and every call through `Vans`) to `loader`. The resolver now reads the file's own `export default` statement from comment-stripped code. A declaration it writes is found by position, a binding it names by name, and an anonymous function, class or arrow function is no node at all (the guess took an exported function beside it, or one nested inside it). The statement also beats an exported styled/memo component above it. Every line-initial statement is read and the first one that names a node of the file wins, so template text that only reads like a statement names nothing. Strings are not masked: a masker loses step on a template whose `${…}` holds another template or a regex with a backtick, and on outline it blanked the real statement below one. Only an expression default (`memo(Card)`, `new Service()`), a binding the file doesn't declare, or no statement keeps the old guess. `require('./x').default` (colbymchenry#2433) reads the same statement first, so it reaches the declared default too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(cpp): a `->` call on a smart pointer or optional reaches the type it holds (colbymchenry#2440) A C++ call made with `->` on a receiver the calling function or its class declares as a `std::unique_ptr<T>`, `std::shared_ptr<T>` or `std::optional<T>` now resolves on `T`: on the class C++ name lookup finds for it where the call is written, or on a class that class derives from. Before, receiver inference read the type as `unique_ptr`, found no method, and the call fell through to the guess by method name: rocksdb's `std::unique_ptr<Iterator> iter; iter->Valid()` reached `ArenaWrappedDBIter::Valid`. The held type goes through colbymchenry#2407's lookup (`cppClassWritten`, extracted from `matchCppSupertype`), not `resolveMethodOnType` on its last name: rocksdb has 22 classes named `Iterator`, and the one the calls mean inherits its methods from `IteratorBase`, so a last-name lookup reaches `MemTableRep::Iterator`. The method is taken from that class or its bases through the class's own extends edges (`getSupertypeNodes`). A template parameter, a library type or an alias of one gets no edge; a type the project doesn't declare, or an alias the lookup can't follow, leaves the call as it was. The namespace-macro helpers move from name-matcher.ts to cpp-namespaces.ts unchanged, so name-matcher can import cpp-supertypes without a cycle. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(sync): a barrel edited to forward another page moves the routes that lazily load it --------- Co-authored-by: Colby Mchenry <me@colbymchenry.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
colbymchenry
added a commit
that referenced
this pull request
Oct 11, 2026
… type of its range (#2492) #2439 typed a range-for variable when the loop names its type. A loop over `auto` elements names none, so receiver inference kept scanning as for an `auto` local with an unreadable initializer: it reached an earlier declaration of the same name, often another variable's, or fell through to the guess by the receiver's name. facebook/rocksdb's db_impl_write.cc loops `for (auto cfd : cfds)` over a `const autovector<ColumnFamilyData*>&`. The loop's range is now read from its header and typed as written, template arguments and all, since the index keeps only a return type's last name: - a name the calling function declares, or a data member of its class read from the class's own lines (`versions_` from db_impl_write.cc); - an `auto` local's initializer; - `*x`, `x.m`, `x->m`, `this`, and calls whose declaration writes their return type, free or on the object's class (own or inherited); - aliases followed with their template arguments (cppAliasedTypeWritten); - the element: a standard or abseil sequence container's first template argument, protobuf's RepeatedPtrField and rocksdb's autovector's, or what a project class's begin() returns, dereferenced (ColumnFamilySet's iterator's operator* gives ColumnFamilyData*). Nullability annotations are dropped. The element goes through the same declaration record as any declared receiver, so the typed path, #2413's gate for a standard-library element and #2440's held-type lookup for `->` on a smart-pointer element apply unchanged. Classes are looked up with #2440's cppClassWritten, methods through its cppMethodOf, and matchCppHeldTypeCall now reads its held type through the shared cppStdHeldType. A/B on main 8a5ba3d, site-keyed: leveldb identical; fmt +1 / 3 retargeted; rocksdb -68 / +173 / 104 retargeted / 231 confidence only; protobuf -13 / +25 / 7 / 59. Every removal and retarget was a wrong guess; 4 of rocksdb's additions reach MemTableRep::Iterator through the typed path's last-name lookup, as a declared `Iterator* it` does on main. 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
C++ receiver inference now reads a range-based
forloop's declaration,for (ConformanceTestSuite *suite : suites),for (const Foo& x : xs),for (Foo* x : xs). Before, it never did.buildDeclaratorRegextakes a declared name only when;,=,,,),[,{or(follows it. A loop variable is followed by its:, sox->Method()/x.Method()fell through to Strategy 3's guess by the receiver's name, or the scan read some other declaration of the same name further up.The motivating case is protocolbuffers/protobuf's conformance runner (
conformance/conformance_test_runner.cc, lines 219–291):ed199e60, with fix(c,cpp): a file named like a test is visible to the code that includes it #2421). 13 calls in twofor (ConformanceTestSuite *suite : suites)loops stay unresolved:suite->SetVerbose(verbose),suite->RunSuite(runner, &output, …),suite->GetFailureListFlagName()and 10 more.inferCppReceiverTypereturns null. The guess dropsConformanceTestSuitebecauseconformance_test.his a test path, and the guess's own test filter is stricter than fix(c,cpp): a file named like a test is visible to the code that includes it #2421's include rule.ConformanceTestSuite's methods through the typed path, at 0.9.A small fixture can hide the bug. With a decoy class, the guess picks the right method by word overlap, at 0.65 or 0.7. So the new tests check the edge's confidence, not only its target.
The rule
for (header declares the receiver before a single:(not::). The declared type is taken the same way the declarator regex takes it (CPP_DECLARED_TYPE_TAIL), so normalization, alias following (fix(cpp): a receiver declared through a typedef or using alias calls the type it names #2399) and fix(cpp): a receiver declared as a std or other outside type with a lowercase name calls that type's own member #2413's std-type gate all apply as they do to any other declaration. A line the declarator regex does match is read exactly as before.cppForBodyEncloses). That means past the header, and up to the}that closes a braced body or the end of a single-statement body. After the loop, or in a later function, the same name is another variable, and the scan goes on as before. The body is found by a small lexer that skips comments and string and character literals. Aforinside a literal (commented-out code, a code generator's template string) has no body, so it declares nothing. Without this check, protobuf/rocksdb'sfield,cfdandfileloop variables would type same-named receivers in later functions.auto. A loop overautoelements has no initializer to read. The scan goes on, as it does for anautolocal whose initializer can't be read, and fix(cpp): a receiver declared as a std or other outside type with a lowercase name calls that type's own member #2413'sshadowedmark is set as before.for (header is read, and only its single:.unsigned car : 1;declares nothing new (and no method is called on a bit-field), nor doesvoid parts::Install()..hread top-down for members) is unchanged: a loop in another file never encloses the call.Validation
Arms are
7a796541(main, after #2413, #2421, #2426, #2449, #2443, #2445, #2446 and #2434) and this branch. The changed-site list is identical, edge for edge, to the run on3de5bef0before the last four C++ PRs landed. Both were built from the same tree with the same native kernel and indexed withcodegraph init -yon this Windows box. Edges are diffed per call site (kind, source node, line, col, refName), and every removed, added and retargeted site was read against the source.I first ran the A/B on
ed199e60. Re-running it after #2449 (C++ attribute macros) left rocksdb, leveldb and fmt unchanged. On protobuf:findremovals dropped out, because main no longer guessesMap::findthere;FindAllFileNames, andAny::UnpackTo, now thatAnyis a class.protobuf.
upb::MessageDefPtr/EnumDefPtr/FieldDefPtr::full_namein the Rust generator'sfor (upb::MessageDefPtr m : …)loops;source->FindAllExtensionNumbers/FindAllFileNamesonfor (DescriptorDatabase* source : sources_);prototype->GetDescriptor(),->New()and the chained->GetDescriptor()->FindFieldByName(…)onfor (const Message* prototype : …);option.value().UnpackTo(…)→Any::UnpackToonfor (const Option& option : options).file->name()infor (const FileDescriptor* file : parsed_files)(command_line_interface.cc:3037) now goes toCodeGeneratorResponse_File::nameat 0.65.FileDescriptor::nameis a macro-generated accessor that isn't indexed.FileDescriptoris a project class, so the existing fallback guesses, exactly as it does for a localconst FileDescriptor* file = …;today.const FileDescriptorProto& file33 lines up, in another block.FileDescriptorProtoisn't indexed as a class, so it looks like an outside type.value->name()onfor (const EnumValueDescriptor* value : canonical_values_): fromEnumValue::nametoEnumValueDescriptor::name(×2);source->FindFileByNameon aDescriptorDatabase*loop variable: fromSourceTreeDescriptorDatabase::FindFileByNameto the declaredDescriptorDatabase::FindFileByName.file_descriptor.name()/file_proto.name()/descriptor.name()onFileDescriptorProto/DescriptorProtoloop variables. These had gone toEnumValueDescriptor::name,CodeGeneratorResponse_File::nameandinternal::DescriptorNames::name. Those classes aren't indexed as classes, so they read as outside types.message->name()on aMessageAccessInfo(a generated message not in the repo), which had gone toupb::MessageDefPtr::name.field->is_extension()throughusing Field = const FieldDescriptor*, which had gone toupb::FieldDefPtr.fragment.remove_prefix(…)onabsl::string_view, which had gone toLeftoverBuffer::remove_prefix.rocksdb.
entry->key()/Freeonfor (LRUHandle* entry : …);cfd->GetName()/current()->storage_info()onColumnFamilyData*;log->file()->Sync(…)/Close/SyncWithoutFlush/reset_seen_errorthroughlog::Writer::file();ag.column_family()/columns()onAttributeGroup;Slice/PinnableSlice/Status/WideColumnmethods;be->AsBackupEngine().log->file()/get_log_number()onfor (log::Writer* log : wals_to_sync): fromBlobLogWritertolog::Writer(12 sites);value.Reset()onPinnableSlice&: fromDBIter::ValueColumnsState::Reset;column.name()onconst WideColumn&: fromLazyWideColumn::name;child_iter->SetPinnedItersMgr(…)onInternalIterator*/ForwardLevelIterator*: fromBlockIter's override to the declared types' methods.op.type()on the fuzzers'const DBOperation&(a generated proto), which had gone to gtest'sTestPartResult::type;std::string/std::shared_ptrloop variables:file.c_str()→ gtest'sFilePath::c_str,fname.find(…)→toku::omt::find,cache.get()→BaseCacheInterface::get.for (const std::shared_ptr<Cache>& cache : …)(7) andfor (UnownedPtr<Stats> stats : stats_).->through a smart pointer isn't followed to its element type (the follow-up fix(cpp): a receiver declared as a std or other outside type with a lowercase name calls that type's own member #2413 left), so these now resolve by the receiver's capitalized name at 0.8. On main, 0.9 came from reading some othercache/statsdeclaration.The scope check on real code. An instrumented copy logged every range-for the scan met that declares the receiver's name, and whether its body encloses the call.
autoloops in other functions.Every typed "not enclosed" verdict within 60 lines of the call, and every "enclosed" verdict more than 60 lines from it, was read against the source, and all were right. The instrumented runs' dumps are byte-identical to the fix arm's.
Cost. Measured with resolution on the main thread (
CODEGRAPH_NO_PARALLEL_RESOLVE=1), using instrumented copies of both arms with hrtime counters, on a box at 100% CPU from other work. Both instrumented runs' dumps are byte-identical to the plain arms'.The new code is about 1–1.5% of the time spent inferring receivers, and the hrtime wrapper is a large share of that. Most lines return at the
forsubstring check. The main/branch gap is noise: it goes opposite ways on the two repos.Tests
New
__tests__/cpp-range-for-receiver.test.ts(9 tests):SetVerbosethe guess avoids: the calls resolve, but at 0.65 / 0.7, not 0.9;*on the name, global-scope::and a header split over two lines;for.forin a comment or string literal; a bit-field above a member of the same name; a name followed by::.:(or:(?!:)) to the declarator regex's lookahead instead.CODEGRAPH_KERNEL_EXPECT=1).EBUSY-style filesystem errors, and 2 were daemon/writer-lock lifecycle assertions. A serial rerun of those 50 files with 120 s timeouts passed 1,685 of their 1,702 tests and failed 2. Both pass when run alone:function-ref.test.ts's Caller/impact graph misses methods passed as first-class references (callbacks), e.g. executor.submit(obj.method, …) #1820 shadow case, which took 44 s of its own 60 s limit;mcp-writer-lock.test.ts's two-proxies case.Not in this PR
autoloops (for (const auto& x : xs)) still fall back. Typing them means reading the range's element type: from the container's declaration, or from a call's return type. That is roughly 1,570 calls in protobuf and rocksdb alone.->through a smart-pointer loop variable reaches the element type only by the receiver's name. This is the same gap fix(cpp): a receiver declared as a std or other outside type with a lowercase name calls that type's own member #2413 left for any declaration.🤖 Generated with Claude Code