Skip to content

fix(c,cpp): a name in C or C++ code outside tests links to what the file's own includes declare - #2684

Merged
colbymchenry merged 1 commit into
mainfrom
claude/cpp-prod-translation-unit
Oct 19, 2026
Merged

colbymchenry merged 1 commit into
mainfrom
claude/cpp-prod-translation-unit

Conversation

@colbymchenry

Copy link
Copy Markdown
Owner

What

#2675 made a C or C++ test link a name that several files declare to the one its translation unit declares. This applies the same rule to every C and C++ reference. Of the same-named candidates, the ones the file's unit declares come first:

  • a definition in the file itself;
  • one in a file it includes, directly or through other includes;
  • one in the source that implements such a header (the same-stem source that includes it), unless that source keeps it to itself: a static, an unnamed namespace, or a class only it declares.

When the unit declares none of them, the candidates stay as they were, so it never guesses outside the unit.

On main f43432c, production code took whichever namesake the ranking put first:

  • upb's C API from its bindings. protocolbuffers/protobuf's Python extension (python/*.c), its Lua binding (lua/*.c), the hpb backend and the benchmarks called the copies of upb's C API that the PHP extension bundles in php-upb.c, or left them unlinked. Example: upb_Encode → upb/wire/encode.c, upb_Arena_New → upb/mem/arena.h.
  • protobuf's Java generator. It calls the free HasHasbit(descriptor) and IsRealOneof(field) that java/helpers.h declares. Main linked those calls to ImmutableFieldGenerator::HasHasbit / IsRealOneof, member functions in the folder beside the callers.
  • rocksdb.
    • The stress tool's StringToHex(…) (58 calls) reaches the function its db_stress_common.h declares, not LDBCommand::StringToHex.
    • CompressionTypeToString, EncodeKey and NewWritableFile calls that main left unlinked now link.
    • Roundup / TruncateToPageBoundary reach util/aligned_buffer.h, not PosixMmapFile's methods.
    • Random(123) reaches rocksdb's Random, not vendored gtest's.
  • glog. GetStackTrace() reaches utilities.cc's. AddLogSink, RemoveLogSink and FlushLogFilesUnsafe now link to logging.cc.

What the wider scope needed

Each item was measured with its own arm.

  1. A source implements the header beside it. A source can include two headers named like it: protobuf's python/google/protobuf/pyext/message.cc includes google/protobuf/pyext/message.h and google/protobuf/message.h. Read as implementing both, its free cmessage::MergeFrom joined every unit that includes message.h, and 33 generated CopyFrom bodies' MergeFrom(from) went there. Now, when a source includes a same-stem header in its own folder, only that one counts. With none beside it, all count, as in fix(c,cpp): a name in a C or C++ test links to what the test's own includes declare #2675: fmt's src/format.cc implements include/fmt/format.h.

  2. A header's own overloads. A header's inline overload that calls the one its source defines is in the header's own unit. Example: protobuf's conformance/binary_wireformat.h has template <typename T> Wire Varint(T x) { return Varint(static_cast<uint64_t>(x)); }. Without this, 30 such calls narrowed to the header's own overload, in json/json.h, the cpp generator's helpers.h, binary_wireformat.h and others.

  3. Argument count. Narrowing never leaves a call with only functions that can't take its arguments when a candidate outside the unit can. Without this, 4 production calls went wrong:

    • glog's three 3-argument GetStackTrace(stack, ARRAYSIZE(stack), 1) calls went to utilities.cc's GetStackTrace(). They mean the function stacktrace.h declares, which the stacktrace_*-inl.h headers that stacktrace.cc includes define.
    • breakpad's safe_readlink.h template's 3-argument SafeReadLink(…) linked to the 2-parameter template itself. safe_readlink.cc, which defines the 3-parameter function, never includes its header.

    The guard also fixes 9 breakpad unittest calls that fix(c,cpp): a name in a C or C++ test links to what the test's own includes declare #2675 had narrowed the same wrong way. One limit: the check reads the definition's parameter list, which may not show what a call relies on, such as a default written only on the header's declaration. 35 calls like rocksdb's CancelAllBackgroundWork(db) therefore keep main's target at main's confidence instead of 0.9.

  4. fix(c): a header's definition for a flag nothing sets, like lua-cjson's #ifdef USE_INTERNAL_FPCONV fpconv_init, gives way to the source file the build compiles #2615's flag-only definitions. For a C call, narrowing doesn't leave only definitions that a flag nothing sets compiles, when the build without it compiles another. That choice stays fix(c): a header's definition for a flag nothing sets, like lua-cjson's #ifdef USE_INTERNAL_FPCONV fpconv_init, gives way to the source file the build compiles #2615's: fixture fpx/use.c calls the fpx.c that never includes fpx.h, not the header's #ifdef FPX_FAST definition. The full suite's c-feature-macro-variants.test.ts caught this. It changes nothing in the 9 repos.

  5. A file-level "call" is left alone. A call outside any function is a macro, or a declaration read as a call, like the prototypes in rocksdb's c.h.

A/B (main f43432c vs this branch)

Full indexes with the same kernel. A site is (kind, source node, line, col, refName). I checked that main f43432c indexes these 9 repos byte-identically to main 24ea3c5, where most of the triage below was done. The branch was then rebased onto 22e9f7b. The three commits in between (#2680–#2682) change Go extraction and which build folders under src/ are scanned for TypeScript and friends; nothing on the C/C++ resolution path.

Repo Added Retargeted Removed Confidence only
protocolbuffers/protobuf 170 335 29 21,386
facebook/rocksdb 90 255 29 1,156
redis/redis 0 0 0 1,384
karpathy/llm.c 0 59 0 667
chromium/breakpad 3 11 7 157
fmtlib/fmt 2 5 0 56
google/leveldb 0 0 0 25
jqlang/jq 0 0 0 8
google/glog 3 1 0 4
tokio, excalidraw, okhttp, typeorm (controls) byte-identical

Confidence only. The target is the same, but it is now the only candidate the unit declares, so exact-match 0.4/0.7 → 0.9. Of protobuf's sites, 15,492 went 0.7 → 0.9 and 5,901 went 0.4 → 0.9. rocksdb has 87 at 0.4 → 0.5: a C++ caller and a header indexed as C (see Notes).

Triage

I read every (before → after target) group and sampled the large ones by file pair. I also checked argument counts mechanically: for each retargeted or added call, the number of arguments against the parameters of its new target. That check found the glog and breakpad cases the guard now handles. What it still flags is receiver guesses that were wrong before too, defaults declared on a header, macro wrappers, and calls whose arguments my audit script misread.

Right, or main's wrong target replaced by the right one:

  • protobuf:
    • 378 upb calls into upb's own files: from the Python extension 152, hpb 55, upb itself 55, upb_generator 47, the benchmarks 37, the Lua binding 29;
    • the Java generator's HasHasbit ×22 and IsRealOneof ×15;
    • ReadTag(ptr, &tag) → internal::ReadTag, not CodedInputStream::ReadTag;
    • GetClassData(*lhs) → internal::GetClassData;
    • UnknownFieldParse(tag, std::string*, …) → parse_context.cc's overload;
    • QualifiedClassName(d, Options()) → the cpp generator's own Options, not the C# generator's.
  • rocksdb:
    • StringToHex ×58, Roundup ×9, TruncateToPageBoundary ×8, Info(logger_, …) ×6 → the free functions;
    • Random ×14;
    • db_full->WaitForCompact(…) → DB::;
    • the rocksdb_* C API from examples/*.c → db/c.cc;
    • CopyFile(fs, …) → file/file_util.
  • llm.c: cublasCheck(…) ×34 → the llmc/cublas_common.h the programs include, and other functions from the sources they include.
  • breakpad: SafeReadLink ×9; CreateGUID → guid_creator.cc.
  • glog: as listed above.

Wrong → wrong:

  • ~70 rocksdb receiver-word guesses whose receiver's type isn't read, for example:
    • fs_->DeleteFile(…) was FileTemperatureTestFS and is now FaultInjectionTestFS;
    • debug_iter->Valid() was ArenaWrappedDBIter and is now SequenceIterWrapper;
    • writable_file_->Close(…) was the enclosing wrapper and is now WritableFile; the receiver is an FSWritableFile.
  • rocksdb's Hash()(t) ×7, a functor built from a template parameter, was a struct and is now util/hash.cc's Hash function.
  • A few protobuf X::default_instance() / X::descriptor() guesses.
  • fmt's using ptr = void*; callback(ptr());, which is no call.

Right → wrong:

  • rocksdb's block_cache_tracer_->is_tracing_enabled() ×4 in a header: BlockCacheTracer → IOTracer. Both headers are in the unit, and the receiver-word guess picks the other one.

Removed, all wrong guesses:

  • rocksdb ×29:
    • debug_iter->GetProperty(…) → ArenaWrappedDBIter;
    • FLAGS_block_cache_trace_file.empty() → BlockBuilder::empty;
    • gtest's thread_.Get() → ThreadStatusUpdater::Get;
    • the 7 Hash()(t) constructions.
  • protobuf ×29:
    • Options() → the C# generator's Options, replaced by the cpp generator's;
    • SourceCodeInfo::default_instance() → CodeGeneratorResponse_File;
    • map->end() / erase() on a std::map → protobuf's Map.
  • breakpad ×7: memory_blocks_.size() → MemoryMappedFile::size; line_map_.end() → StaticMap::end.

Cost

Full indexes, interleaved main/branch, 3 rounds, same kernel:

  • rocksdb: median 52.8s here vs 50.3s on main (runs 50.3–54.3s vs 49.8–52.2s), perhaps 2–5% more;
  • protobuf: median 116.7s vs 118.1s (109.7–120.8s vs 109.1–130.1s), within the noise.

The check runs for each C/C++ reference with two or more candidates, and now for production references too, not only tests'. It reuses the memoized include graph and same-stem header lists. The argument count is read only when narrowing would actually drop a candidate for a call.

Tests

__tests__/cpp-translation-unit.test.ts, 5 cases:

  • Fail on main (3):
    • upb_Encode from a Python-extension-shaped file reaches upb/wire/encode.c, not a PHP-bundled copy;
    • the Java generator's HasHasbit(descriptor) reaches helpers.h's free function, not the field generator's method beside it;
    • glog's GetStackTrace() reaches utilities.cc's (the 3-argument call beside it keeps the stacktrace definition).
  • Guards that pass on main (2):
    • a header's template Varint calls its source's Varint;
    • pyext/message.cc is no implementation of google/protobuf/message.h.

Each refinement was checked by mutation: without the argument-count guard, without the same-folder rule, and without the own-header rule, exactly the matching test fails. #2675's cpp-test-translation-unit.test.ts still passes.

The full suite runs on this change on main 22e9f7b, with the kernel built, CODEGRAPH_KERNEL_EXPECT=1 and --maxWorkers=4 --minWorkers=1: 7,528 passed, 0 failed, 80 skipped. An earlier run, before refinement 4, failed c-feature-macro-variants.test.ts twice; those two tests are what pin it.

Notes

  • What's left of the upb case. 343 calls from protobuf's Python extension still go to php-upb.c. The upb source that defines them includes only its internal header, for example upb/reflection/field_def.c → upb/reflection/internal/field_def.h, which then includes the public one the Python code includes. So the unit doesn't see that source as the implementation. Following a source's same-stem includes through such an internal header would close it. That's its own change and needs its own A/B, so I'll take it as the next follow-up.
  • A C++ header indexed as C. rocksdb's db_stress_tool/db_stress_common.h has its first C++ construct past the 8 KB that the language check samples, so it is indexed as C. That is why its StringToHex lost to LDBCommand::StringToHex on main: the ranking skips a C candidate for a C++ caller. In the 9 repos, it's the only header indexed as C for that reason, so I'm not changing the language check here.
  • Qualified names are still unchanged, as in fix(c,cpp): a name in a C or C++ test links to what the test's own includes declare #2675; the :: boundary fix lives on another branch.

🤖 Generated with Claude Code

…ile's own includes declare

#2675 narrowed a C or C++ test's same-named candidates to the ones its
translation unit declares. Production code now gets the same rule:
protobuf's Python extension called the copies of upb's C API that the PHP
extension bundles in php-upb.c, and its Java generator's calls of the free
HasHasbit() went to ImmutableFieldGenerator::HasHasbit, a method beside the
caller.

Refinements the wider scope needed:
- a source implements the same-stem header beside it, when it includes one:
  pyext/message.cc is no implementation of google/protobuf/message.h;
- a header's own overloads reach the definitions its source holds:
  conformance/binary_wireformat.h's template Varint calls the .cc's;
- narrowing never leaves a call with only functions its arguments can't
  fill, past one outside the unit that they can: glog's three-argument
  GetStackTrace(...) is not utilities.h's GetStackTrace(). This also mends
  9 breakpad unittest calls #2675 had narrowed the same way;
- nor, for a C call, to definitions only a flag nothing sets compiles,
  past one the build without it does: that stays #2615's choice.
A file-level "call" (a macro, or a declaration read as a call) is left as
it was.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@colbymchenry
colbymchenry merged commit 32b839a into main Oct 19, 2026
@colbymchenry
colbymchenry deleted the claude/cpp-prod-translation-unit branch October 19, 2026 22:12
danusha2345 pushed a commit to danusha2345/codegraph that referenced this pull request Oct 20, 2026
…he name strategies; colbymchenry#1938/colbymchenry#1954 tests follow upstream's colbymchenry#2684/colbymchenry#2664

The same changes went into the rebased PR branches.

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