Skip to content

fix(cpp): a call written from the global scope links the declaration of exactly that name - #2445

Merged
colbymchenry merged 7 commits into
mainfrom
claude/nifty-bun-4f5d18
Oct 7, 2026
Merged

colbymchenry merged 7 commits into
mainfrom
claude/nifty-bun-4f5d18

Conversation

@colbymchenry

@colbymchenry colbymchenry commented Oct 7, 2026 •

Copy link
Copy Markdown
Owner

Follow-up to #2415.

What was wrong

The resolver's name-existence pre-filter (hasAnyPossibleMatch in resolveOneInner) never accepted a C/C++ name that starts with ::. Its :: branch needs the separator past the first character, and the single-: loop skips names holding ::. So a global-qualified call like ::leveldb::RepairDB(…) was dropped before any resolution strategy ran, unless a detected framework's claimsReference() let it through.

protobuf's ::_pbi::PrivateAccess::GenerateParseTable(…) calls resolved only because protobuf ships three .swift files. That makes the Swift ↔ Objective-C bridge a detected framework, and the bridge claims any name with a : in it. That dependency is why #2415 left the claims check universal. C++-only repos had no such escape: on main, rocksdb leaves all 729 of its leading-:: calls unresolved.

The change

  1. Pre-filter: a C or C++ name with a leading :: is checked without it.
  2. Claims narrowed: claimsReference() is asked only of this.frameworksFor(ref.language), the frameworks whose resolve() reads the reference since fix(frameworks): a framework resolves only references written in its own languages #2415. fix(frameworks): a framework resolves only references written in its own languages #2415's comment explaining why the claims stayed universal is gone.
  3. matchByQualifiedName treats a global-qualified C/C++ name as exactly that qualified name: C/C++ declarations whose qualified name is the written name without ::, the call site's file first. It never falls through to the suffix match, which takes a namesake nested in another namespace or class. It also skips a declaration inside a namespace that a macro opens: the index can't see fmt's FMT_BEGIN_NAMESPACE, so fmt::pipe looks like a global pipe. Step 5 (matchCppMacroNamespaced) still handles macro namespaces and namespace aliases, which is where ::_pbi::… keeps resolving.
  4. ::std::… and ::memset count as std::… and memset in the built-in check. Step 5 returns early for a single-segment name: it used to slice ::pipe into the head pip and the name ipe.

Why 3 is needed: the pre-filter alone gives wrong edges

Measured on #2415's head with only 1 and 2 applied, on the same dist and kernel as the baseline:

repo added right wrong
protobuf, redis 0 (protobuf byte-for-byte unchanged: the narrowed claims removed nothing)
leveldb 1 0 ::crc32c::Extend (the external crc32c library) → leveldb's own leveldb::crc32c::Extend
fmt 11 1 10: the posix mock wrappers (test::open, test::fopen, test::pipe, …) linked to themselves from the ::open(…) etc. they wrap; gtest's Win32 ::CreateThread → ThreadWithParamSupport::CreateThread
rocksdb 12 0 12: Windows ::Compress/::Decompress → the xpress::Compress wrappers that call them; ::gettid → WinEnvThreads::gettid; gtest's ::GetThreadPriority → rocksdb's <anonymous>::GetThreadPriority

That arm also gained no in-repo ::ns::x call. A correct top-level target ns::x never ends with ::ns::x, so the suffix match can only return nested namesakes. protobuf showed the same thing: its ::google::protobuf::internal::TSanWrite calls (435 of them) already passed the pre-filter on the bridge's claim and still resolved to nothing.

Validation

Final A/B on the squash itself. Both arms use the same dist and the same kernel, rebuilt for main's C/C++ changes. The baseline is 235387e, the squash's parent. The fix arm is the same build with this change overlaid (tsc output of resolution/index.js and resolution/name-matcher.js), which is the tree of 7a79654. Every index is diffed site by site (source node, line, column, ref name, ref kind), plus unresolved statuses and synthesized edges.

repo added removed what
leveldb 1 0 ::leveldb::RepairDB(…) → leveldb::RepairDB
fmt 24 0 23 ::testing::… → fmt's vendored googletest/gmock in test/gtest/; ::fmt::assert_fail → assert_fail inside FMT_BEGIN_NAMESPACE (step 5)
redis 3 0 jemalloc's tests' ::operator new(…) → jemalloc's replacement global operator new (deps/jemalloc/src/jemalloc_cpp.cpp)
rocksdb 610 0 609 ::testing::… → the vendored gtest 1.8.1 in third-party/ (219 of them InitGoogleTest); ::ROCKSDB_NAMESPACE::RepairDB
protobuf 1,778 6 1,641 ::google::… (e.g. ::google::protobuf::internal::TSanWrite, WireFormatLite::StringSize, MessageLite::internal_visibility), 128 ::hpb::…, 4 ::upb::…, 4 ::pb::…, 1 ::upb_Array_DataPtr. Removed: 6 ::operator delete(ptr) → a generated class's operator delete, which were wrong; protobuf declares no global operator delete
27 framework repos (the #2415 kit: Express, Drupal, Laravel, Play, Spring, Django/FastAPI, React/Next/Expo/RN, Vue/Nuxt, SvelteKit, ASP.NET, Vapor, CICS, Go …), measured on main ed199e6 0 0 identical edges, unresolved statuses and synthesized edges

Triage of every added edge:

  • Exact name match. Each target's qualified name is exactly the written name without its ::. The one exception is fmt's ::fmt::assert_fail, resolved through the macro namespace.
  • testing:: targets are in-repo copies. They point to rocksdb's third-party/gtest-1.8.1/fused-src/ and fmt's test/gtest/. Unqualified testing::X calls already resolve there (403 in rocksdb, 10 in fmt), and so do ::testing::Test bases (fix(cpp): a base class links to the class C++ name lookup finds, not a namesake #2407).
  • External names stay unresolved.
    • Where googletest isn't in the repo, ::testing:: stays unresolved: leveldb 3, protobuf 76.
    • ::std::…, ::absl::…, ::crc32c::… and OS calls like ::CloseHandle and ::fcntl stay unresolved. No edge was added for any of them.
  • The ::_pbi::… alias calls (924) are identical on both arms.
  • Known limitation: ::upb_Array_DataPtr. It lands on the first of three identical C copies of upb (php/ext/google/protobuf/php-upb.h), not upb/message/internal/array.h. That is the same pick the qualified-name path already makes among duplicate definitions. Choosing by the include graph (cpp-includers.ts, fix(c,cpp): a file named like a test is visible to the code that includes it #2421) would fix both paths; left as a follow-up.

Earlier rounds, as main moved:

repo #2415's head main ed199e6 final
leveldb +1 +1 +1
fmt +16 +18 +24
redis +3 +3 +3
rocksdb +386 +610 +610
protobuf +1,649/−6 +1,679/−6 +1,778/−6

Each round's added sites are a strict superset of the round before. The extras are exact matches to declarations that later extraction fixes index:

Still unresolved in-repo: a handful of names the index doesn't hold under the written qualified name, such as fmt's ::testing::PrintToString, and protobuf's ::google::protobuf::EqualsProto and a name split across a line break. They stay silent rather than wrong.

Tests

__tests__/cpp-global-scope-names.test.ts:

  • C++-only fixture:
    • ::store::Repair() from inside a namespace that has its own store::Repair reaches the global one.
    • ::_pbi::Prefetch() through namespace _pbi = ::pb::internal; reaches pb::internal::Prefetch.
  • Never a namesake:
    • a wrapper store::posix::close calling ::close(fd) links nothing;
    • ::testing::TempDir() with only a nested store::testing::TempDir in the project links nothing;
    • a mock test::pipe calling ::pipe(fds) does not reach a struct pipe inside a macro-opened namespace;
    • an external ::testing::Test base stays unresolved.
  • C++ beside Swift and Objective-C files, so the bridge is detected: ::_pbi::Prefetch() still resolves with the narrowed claims.

Mutation-checked:

  • main's sources: the ::ns::f() test fails;
  • pre-filter strip reverted: the ::ns::f() test and the Swift/ObjC test fail;
  • global-scope branch disabled: the ::ns::f() and never-a-namesake tests fail (::store::Repair → tools::store::Repair, ::close → store::posix::close);
  • macro-namespace skip removed: the never-a-namesake test fails (::pipe → pipe).

Reverting only the claims narrowing changes no test. In the measurements it removed nothing either: on #2415's head, protobuf with the strip and the narrowing was byte-for-byte the baseline, and the 27 framework repos are identical. A ref no part of whose name is a known symbol can't be resolved by name in any case; the narrowing keeps the claims consistent with resolve() and saves the work.

Full suite on Windows, at 100% CPU from other sessions, with the kernel rebuilt for main's C/C++ changes and CODEGRAPH_KERNEL_EXPECT=1:

The build's tsc and a type-check of the new test file are clean.

Main moved six times while this was being merged. Each sync re-ran tsc, this PR's test, and the test files of the new main commits; C/C++ commits got a hunk review against this change first. The rounds brought in #2430/#2442, #2449, #2433, #2434/#2444, #2437/#2448, and #2431/#2436/#2441/#2443/#2446/#2447/#2453.

🤖 Generated with Claude Code

colbymchenry and others added 7 commits October 7, 2026 07:40
…of exactly that name

The resolver's name-existence pre-filter read no leading `::`: its `::`
branch wanted the separator past the first character, and the single-`:`
loop skips names holding `::`. So `::leveldb::RepairDB(…)`,
`::ROCKSDB_NAMESPACE::RepairDB(…)` or protobuf's `::_pbi::…` calls were
dropped before any strategy ran, unless a detected framework claimed them.
protobuf's got through only because it ships three Swift files: the
Swift ↔ Objective-C bridge claims any name with a `:` in it, which is why

- The pre-filter checks a C or C++ name without its leading `::`.
- claimsReference() is asked only of the frameworks that resolve the
  reference's language (frameworksFor), like resolve() since #2415.
- matchByQualifiedName takes a global-qualified C/C++ name as exactly
  that qualified name. Its suffix match took a namesake nested in another
  namespace or class: with the pre-filter alone, 23 of the 24 edges it
  added on leveldb, fmt and rocksdb were wrong, mostly wrappers like fmt's
  mock `test::open` linked to the `::open(…)` they wrap, and protobuf's
  `::operator delete` went to `DynamicMessage::operator delete`. A
  declaration inside a namespace a macro opens (fmt's `struct pipe`) only
  looks global in the index, so it is skipped; step 5 still reads those.
- `::std::…` and `::memset` count as `std::…` and `memset` for the
  built-in check, and step 5 returns early for a single-segment name.

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