fix(c,cpp): a file named like a test is visible to the code that includes it by colbymchenry · Pull Request #2421 · colbymchenry/codegraph

colbymchenry · 2026-10-07T06:14:05Z

Summary

A C or C++ file named like a test suite is now visible to the code that #includes it. Before, it was visible to no production code at all. isVisibleAcrossFiles hides every symbol in a test-suite file (*_test.h, test_*.cc, a tests/ directory, …) from non-test files, because a test suite is not linked into the program. For C and C++ that is decided by the name alone. But a header is part of every translation unit that includes it, whatever its name.

The motivating case is protocolbuffers/protobuf's conformance framework:

The code. ConformanceTestSuite lives in conformance/conformance_test.h / .cc, and ConformanceTestRunner in conformance/test_runner.h. The suites (binary_json_conformance_suite.cc, text_format_conformance_suite.cc) and the runners (fork_pipe_runner.h, testee.h) include these headers and derive from or call the classes.
On main (with fix(cpp): a base class links to the class C++ name lookup finds, not a namesake #2407). suite_.ReportFailure(test, level, request, response) at binary_json_conformance_suite.cc:1126 and 8 more sites finds ConformanceTestSuite::ReportFailure through the supertype walk (matchReference returns it at 0.9). resolveOneInner then drops it: isVisibleAcrossFiles → isTestSuitePath('conformance/conformance_test.cc'). So the calls resolve to nothing. Before fix(cpp): a base class links to the class C++ name lookup finds, not a namesake #2407 they reached ForkPipeRunner::ReportFailure by a wrong guess. The base classes themselves no longer go through this rule: since fix(cpp): a base class links to the class C++ name lookup finds, not a namesake #2407, cpp-supertypes.ts resolves them by C++ lookup.
Now. All 9 suite_.ReportFailure sites reach ConformanceTestSuite::ReportFailure at 0.9, along with the rest of the suites' calls into the framework (112 edges in conformance/).

The rule

isVisibleAcrossFiles' test-suite check now lets a C/C++ candidate through when the reference's translation unit includes it:

A header (or any included file) counts when the reference's file includes it, directly or through other headers.
A definition in a source file (ConformanceTestSuite::ReportFailure is defined in conformance_test.cc, and the in-class declaration in the header is not a node) counts through the header it implements. That is the header named like it (same stem) that the source itself includes, when that header is a test suite's too.
- The last condition matters: jemalloc's test test/unit/hash.c includes the library's hash.h to test it, not to implement it.

Everything else is unchanged:

a test suite nothing in the program includes stays out of reach (rocksdb's 240 test-named files);
non-C/C++ languages are untouched (typeorm's Record, tokio's Output);
references from tests already saw test suites.

Why per translation unit, not "a file any non-test file includes". The simpler rule was my first sketch, and the survey ruled it out. protobuf's src/google/protobuf/test_util.h is included by 15 unit-test files that isTestPath doesn't recognize as tests: 14 *_unittest.cc and compiler/cpp/unittest.cc (follow-up filed). A file-level rule would have made TestUtil visible to every production file in protobuf. The per-TU rule is the language's own visibility rule: a unittest that includes test_util.h sees it, and descriptor.cc does not.

Where the include graph comes from (src/resolution/cpp-includers.ts, new):

It is read from the import node extraction mints for every #include. Each include is resolved with context.resolveImport, the same branch that builds the graph's include edges, so "includes" here is the include edge a reader sees. The include resolver can't be imported from name-matcher without a cycle.
The walk runs from the included file up to its includers and is memoized per context. A handful of files are asked about, by references from many files, and a file nothing includes costs one map lookup.
The include nodes come from a new narrow query (getCppIncludeNodes: six columns of C/C++ import nodes). Decoding every import node took about 330 ms on protobuf, against about 45 ms for the narrow select.
Only the includes spelled with the candidate source's stem get resolved.

Validation

All arms ran with codegraph init -y and the native kernel, on this Windows box. Main is 4040eab9 (with #2407). Edges are diffed per call site (kind, source node, line, col, refName), and every changed site was read against the source.

repo	added	same edge, new metadata
protocolbuffers/protobuf	675	0
redis	1	0
google/leveldb, fmtlib/fmt, karpathy/llm.c, jqlang/jq, facebook/rocksdb (C/C++ with many test files)	0	0 (dumps byte-identical)
typeorm, tokio, okhttp, excalidraw (repos that motivated the rule in #2209)	0	0 (dumps byte-identical)

protobuf, conformance (112). Every source file includes the target, directly or through the header the target source implements.
- All 9 suite_.ReportFailure sites;
- suite_.RunTest / ReportSuccess / ReportSkip / RunValidInputTest / RunValidBinaryInputTest;
- the bare inherited ReportFailure(…), ReportSuccess(…) and WireFormatToString(…) calls inside BinaryAndJsonConformanceSuite's own methods;
- setting.GetRequest() and the rest of the nested ConformanceRequestSetting class, plus 6 ConformanceRequestSetting setting(…) constructions;
- TextFormatConformanceTestSuite's SetFailureListFlagName(…);
- testee.cc's runner_->RunTest(…) → ConformanceTestRunner::RunTest.
protobuf, unit tests (563). These are calls from *_unittest.cc files (and wire_format_unittest.h) into the test utilities they include: test_util.h, test_util2.h, test_util_lite.cc, test_textproto.h. Since these files count as production code, the same rule had dropped them. All 563 were checked by script: the written name matches the target's, and every qualifier names the target's namespace.
- 397 TestUtil::X (qualified-name 0.85);
- 86 reflection_tester.X → TestUtil::ReflectionTester (0.9);
- 52 TestUtilLite::X;
- 17 ParseTextOrDie(…);
- 8 TestUtil::ReflectionTester::X;
- 3 bare EqualsToSerialized(…) under using TestUtil::EqualsToSerialized;.
redis (1). modules/vector-sets/expr.c includes fastjson_test.c under #ifdef TEST_MAIN and calls its run_fastjson_test().
Same graph, either resolver mode. Re-indexing with resolution kept on the main thread (CODEGRAPH_NO_PARALLEL_RESOLVE=1) reproduces the parallel dumps byte for byte on protobuf, rocksdb, redis and leveldb.

Cost (resolution on the main thread, instrumented copy):

repo	checks	visible	time in the check
rocksdb (379 s run)	906	0	185 ms (84 ms is the include index)
protobuf (938 s run)	781	675	257 ms (184 ms is the include index)

Before the narrow query and the stem filter, the same checks took 936 ms on rocksdb and 1.2–1.8 s on protobuf, which decoded every import node and resolved every include of each test source. The instrumented runs' dumps are byte-identical to the final arm's.

Tried and not shipped: the same exception for the receiver-name guess

matchMethodCall's Strategy 3 has its own "production never calls into a test" filter (isTestPath on both sides). Giving it the same include exception gained 13 correct edges in protobuf: conformance_test_runner.cc's suite->SetVerbose(…) and its siblings. It cost rocksdb 1 new wrong edge and 3 wrong→wrong retargets. db_stress_listener.cc's fs->FileExists(…) on a std::shared_ptr<FileSystem> moved from FaultInjectionTestFS to test::StringFS, a test double in test_util/testutil.h, which production code includes. That filter exists to keep test doubles out of guesses, so it stays as is. The 13 runner calls fail for another reason: the C++ declarator regex never reads a range-for declaration (for (ConformanceTestSuite *suite : suites)). That is filed separately.

Tests

New __tests__/cpp-included-test-suite.test.ts is a fixture modelled on protobuf's conformance layout.
- On main, 3 of 6 fail: suite_.ReportFailure through the base class; a free function the included header declares; a test-named .c that a production file includes.
- The 3 controls pass on both: a production file that doesn't include the header stays unlinked; a real *_test.c nobody includes stays out of reach; a test that includes the production header it tests (jemalloc's shape) doesn't become that header's implementation.
- It passes on the wasm extraction path and on the native kernel (CODEGRAPH_KERNEL_PATH).
- Mutation check. With the "header is a test suite too" condition removed, the jemalloc-shaped control fails: src/hash.c links hash_selftest in test/unit/hash.c.
prod-to-test-suite.test.ts (fix(resolution): production code never names a test suite's symbol #2209's typeorm/okhttp cases), cross-file-visibility, cpp-base-class-lookup and the other C/C++ resolution suites pass.
tsc --noEmit is clean.
Full suite on this Windows box. Run in parallel alongside the indexing jobs, it failed 70 files, all timeouts, file locks or wall-clock budgets. Re-running those 70 files serially with long timeouts left one failure: function-ref's Caller/impact graph misses methods passed as first-class references (callbacks), e.g. executor.submit(obj.method, …) #1820 case hit its own 60 s limit, and it passes alone.
After merging main (fix(kernel): a Go value like wm.new or raft.None keeps its function ref #2401, fix(sync): a Liquid section or snippet reference links its file when the file appears later #2403, neither touches C/C++ resolution), the touched suites and those two PRs' test files pass.

Follow-ups filed

C++ receiver inference never reads a range-based for declaration (the 13 conformance_test_runner.cc calls).
isTestPath doesn't recognize Google's *_unittest.cc naming (protobuf's 62 files count as production code).

🤖 Generated with Claude Code

…udes it isVisibleAcrossFiles hides every symbol in a test-suite file from non-test code, decided by the file's name. A C or C++ header is part of every translation unit that includes it, whatever its name: protobuf's conformance framework lives in conformance/conformance_test.h/.cc and test_runner.h, and the 9 `suite_.ReportFailure(...)` calls in the suites found ConformanceTestSuite::ReportFailure through the supertype walk only to have it dropped by the test-suite check. The check now lets a C/C++ candidate through when the reference's file includes it, directly or through other headers. A definition in a source file counts through the same-named test-suite header it includes (conformance_test.cc -> conformance_test.h), so a test that includes the production header it tests (jemalloc's test/unit/hash.c and hash.h) does not become its implementation. The include graph (new cpp-includers.ts) is read from the import nodes extraction mints for every #include, each resolved the way the graph's include edge is, walked from the included file up, from a narrow query of six columns. protobuf +675 edges, none removed or retargeted: 112 in conformance/ and 563 from *_unittest.cc files (which isTestPath does not recognize as tests) into the test utilities they include. redis +1 (expr.c includes fastjson_test.c). leveldb, fmt, llm.c, jq, rocksdb, typeorm, tokio, okhttp and excalidraw are byte-identical. The same exception for Strategy 3's receiver-name guess was measured and left out: it moved rocksdb guesses onto test::StringFS, a test double production code includes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…rzakhani-a8363e

…a test (#2434) Google names C++ tests `foo_unittest.cc` (protobuf, Breakpad, glog, Chromium), not `foo_test.cc`. The test-file check wanted a separator right before `test`, so each one read as production code: search and explore ranked it alongside the code it tests, `codegraph affected` never listed it, and the resolver's test rules ran backwards for it. Production calls landed on a unittest's own declarations (`stack_frame_entries_.size()` on a unittest's `StackHelper`, a template's `AddressType()` on a unittest's typedef), and a unittest's calls into test suites outside what it includes (a Python `*_unittest.py` using a `tests/` fake) were treated as production calls. #2421 already links a C/C++ unittest to the test helpers it includes. `unittest` and `unittests` join the separator-delimited test suffixes (`foo_unittest.cc`, `foo-unittest.cpp`, `foo.unittest.js`), and a file named so is a test suite to the resolver, as `foo_test.cc` is. A bare `unittest.go` stays production code: promtool's runs rule tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…lares (#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 (#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 #2421, the 13 conformance runner calls reach ConformanceTestSuite's methods at 0.9. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

…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 and others added 4 commits October 7, 2026 00:37

Merge remote-tracking branch 'origin/main' into claude/intelligent-mi…

e84c7de

…rzakhani-a8363e

Merge remote-tracking branch 'origin/main' into claude/intelligent-mi…

f1aa74f

…rzakhani-a8363e

Merge remote-tracking branch 'origin/main' into claude/intelligent-mi…

3bae9e2

…rzakhani-a8363e

colbymchenry merged commit 22a7ba8 into main Oct 7, 2026

colbymchenry deleted the claude/intelligent-mirzakhani-a8363e branch October 10, 2026 06:15

Navigation Menu

Repository navigation

fix(c,cpp): a file named like a test is visible to the code that includes it - #2421

fix(c,cpp): a file named like a test is visible to the code that includes it#2421
colbymchenry merged 4 commits into
mainfrom
claude/intelligent-mirzakhani-a8363e

colbymchenry commented Oct 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

colbymchenry commented Oct 7, 2026

Summary

The rule

Validation

Tried and not shipped: the same exception for the receiver-name guess

Tests

Follow-ups filed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant