Skip to content

fix(cpp): a range-based for loop's variable has the type the loop declares - #2439

Merged
colbymchenry merged 8 commits into
mainfrom
claude/serene-perlman-696753
Oct 10, 2026
Merged

colbymchenry merged 8 commits into
mainfrom
claude/serene-perlman-696753

Conversation

@colbymchenry

@colbymchenry colbymchenry commented Oct 7, 2026 •

Copy link
Copy Markdown
Owner

Summary

C++ receiver inference now reads a range-based for loop's declaration, for (ConformanceTestSuite *suite : suites), for (const Foo& x : xs), for (Foo* x : xs). Before, it never did. buildDeclaratorRegex takes a declared name only when ;, =, ,, ), [, { or ( follows it. A loop variable is followed by its :, so x->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):

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

  • What is read. A line the declarator regex doesn't match is read as a range-based for when its 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.
  • Scope. The loop's variable counts only when the call is inside the loop's body (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. A for inside a literal (commented-out code, a code generator's template string) has no body, so it declares nothing. Without this check, protobuf/rocksdb's field, cfd and file loop variables would type same-named receivers in later functions.
  • auto. A loop over auto elements has no initializer to read. The scan goes on, as it does for an auto local 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's shadowed mark is set as before.
  • Bit-fields and the scope operator. Neither is ever read as a loop's declaration. Only a for ( header is read, and only its single :. unsigned car : 1; declares nothing new (and no method is called on a bit-field), nor does void parts::Install().
  • The header scan (the same-stem .h read 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 on 3de5bef0 before the last four C++ PRs landed. Both were built from the same tree with the same native kernel and indexed with codegraph init -y on 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:

  • three find removals dropped out, because main no longer guesses Map::find there;
  • two right adds appeared: FindAllFileNames, and Any::UnpackTo, now that Any is a class.
repo removed added retargeted same edge, new metadata
protocolbuffers/protobuf 10 (all wrong guesses) 26 (25 right, 1 wrong) 3 (wrong → right) 24 (confidence up to 0.9)
facebook/rocksdb 25 (all wrong guesses) 38 (all right) 18 (wrong → right) 24 (16 up to 0.9; 8 down 0.9 → 0.8, same target)
google/leveldb, fmtlib/fmt 0 0 0 0 (dumps byte-identical)

protobuf.

  • Added, right (25):
    • the 13 conformance runner calls;
    • upb::MessageDefPtr / EnumDefPtr / FieldDefPtr::full_name in the Rust generator's for (upb::MessageDefPtr m : …) loops;
    • source->FindAllExtensionNumbers / FindAllFileNames on for (DescriptorDatabase* source : sources_);
    • prototype->GetDescriptor(), ->New() and the chained ->GetDescriptor()->FindFieldByName(…) on for (const Message* prototype : …);
    • the chained option.value().UnpackTo(…) → Any::UnpackTo on for (const Option& option : options).
  • Added, wrong (1): file->name() in for (const FileDescriptor* file : parsed_files) (command_line_interface.cc:3037) now goes to CodeGeneratorResponse_File::name at 0.65.
    • The loop is now read correctly, but FileDescriptor::name is a macro-generated accessor that isn't indexed. FileDescriptor is a project class, so the existing fallback guesses, exactly as it does for a local const FileDescriptor* file = …; today.
    • On main the call had no edge only by accident. The scan read an unrelated const FileDescriptorProto& file 33 lines up, in another block. FileDescriptorProto isn't indexed as a class, so it looks like an outside type.
  • Retargeted (3):
    • value->name() on for (const EnumValueDescriptor* value : canonical_values_): from EnumValue::name to EnumValueDescriptor::name (×2);
    • source->FindFileByName on a DescriptorDatabase* loop variable: from SourceTreeDescriptorDatabase::FindFileByName to the declared DescriptorDatabase::FindFileByName.
  • Removed (10), all wrong guesses:
    • file_descriptor.name() / file_proto.name() / descriptor.name() on FileDescriptorProto / DescriptorProto loop variables. These had gone to EnumValueDescriptor::name, CodeGeneratorResponse_File::name and internal::DescriptorNames::name. Those classes aren't indexed as classes, so they read as outside types.
    • message->name() on a MessageAccessInfo (a generated message not in the repo), which had gone to upb::MessageDefPtr::name.
    • field->is_extension() through using Field = const FieldDescriptor*, which had gone to upb::FieldDefPtr.
    • 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, now that the declaration is read: fragment.remove_prefix(…) on absl::string_view, which had gone to LeftoverBuffer::remove_prefix.

rocksdb.

  • Added (38), all right. Examples:
    • entry->key() / Free on for (LRUHandle* entry : …);
    • cfd->GetName() / current()->storage_info() on ColumnFamilyData*;
    • log->file()->Sync(…) / Close / SyncWithoutFlush / reset_seen_error through log::Writer::file();
    • ag.column_family() / columns() on AttributeGroup;
    • Slice / PinnableSlice / Status / WideColumn methods;
    • be->AsBackupEngine().
  • Retargeted (18), all wrong → right. Examples:
    • log->file() / get_log_number() on for (log::Writer* log : wals_to_sync): from BlobLogWriter to log::Writer (12 sites);
    • value.Reset() on PinnableSlice&: from DBIter::ValueColumnsState::Reset;
    • column.name() on const WideColumn&: from LazyWideColumn::name;
    • child_iter->SetPinnedItersMgr(…) on InternalIterator* / ForwardLevelIterator*: from BlockIter's override to the declared types' methods.
  • Removed (25), all wrong guesses:
  • Same target, confidence 0.9 → 0.8 (8). The loop variable is a smart pointer: for (const std::shared_ptr<Cache>& cache : …) (7) and for (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 other cache / stats declaration.

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.

  • protobuf: 1,746 such loops, of which 683 enclose the call;
  • rocksdb: 3,837, of which 1,297 enclose it;
  • most loops judged not enclosing are auto loops 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'.

repo receiver inferences time inferring (main / this branch) range-for check loop-body check
rocksdb 112,210 51.4 s / 47.1 s 2.18 M lines, 598 ms 4,284 loops, 50 ms
protobuf 52,487 19.8 s / 20.5 s 647 k lines, 226 ms 1,870 loops, 12 ms

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 for substring 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):

Not in this PR

🤖 Generated with Claude Code

colbymchenry and others added 3 commits October 7, 2026 05:29
…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>
@colbymchenry
colbymchenry merged commit b8b3eb9 into main Oct 10, 2026
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
colbymchenry deleted the claude/serene-perlman-696753 branch October 10, 2026 06:15
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>
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