Skip to content

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

Open
colbymchenry wants to merge 3 commits into
mainfrom
claude/serene-perlman-696753
Open

colbymchenry wants to merge 3 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>

This branch has not been deployed

No deployments
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