From f66d064279c02f483c33602abec2e5ce9e13522a Mon Sep 17 00:00:00 2001 From: Colby Mchenry Date: Wed, 21 Oct 2026 04:20:15 +0000 Subject: [PATCH] fix(cpp): a function is named by its declarator, never by its noexcept, throw or requires clause (#2727) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both C++ extractors name a function by the first qualified identifier found breadth-first in its declarator, which is how an out-of-line `Foo::bar` is named. The walk skipped only the parameter list and the trailing return type, so a qualified name in the clauses after them became the name of a function whose own name is plain. nlohmann/json's constructor `basic_json(CompatibleType&& val) noexcept(noexcept( JSONSerializer::to_json(…)))` was indexed as a method `JSONSerializer::to_json` that doesn't exist, with no node for the constructor; `throw(std::bad_alloc)` named a method `std::bad_alloc`, and a trailing `requires std::integral` a function `std::integral`. The walk now also skips `noexcept`, `throw_specifier` and `requires_clause`, in the TS extractor and the kernel alike; the parity fixture covers all three. Measured on main 226ac930 with fresh indexes, and byte-identical again after syncing onto e97012eb: json 42, abseil 28 and protobuf 1 phantom nodes become the real functions at the same lines; fmt, leveldb, rocksdb, redis and three controls in other languages are byte-identical. Kernel parity sweeps on json, fmt, abseil, leveldb and rocksdb find no file with diffs. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + __tests__/cpp-declarator-clause-names.test.ts | 137 ++++++++++++++++++ __tests__/fixtures/kernel-parity/torture.cpp | 16 ++ codegraph-kernel/src/ccpp/mod.rs | 11 +- src/extraction/languages/c-cpp.ts | 13 +- 5 files changed, 172 insertions(+), 6 deletions(-) create mode 100644 __tests__/cpp-declarator-clause-names.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 03c6f825..5c8dd8f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -113,6 +113,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In C++, a base class like `Message` in `class DynamicMessage final : public Message` now links to the class C++ itself would pick: the one declared in the namespaces around the class, or brought in by the file's `using` declarations and namespace aliases, and the class a `typedef` or `using` alias names. Before, a base class was matched by its name alone, so it could land on a class of the same name in another namespace or another language: in protocolbuffers/protobuf, `DynamicMessage` derived from the JSON parser's internal `Message` class, and in google/leveldb and RocksDB, iterators derived from an unrelated nested `Iterator` class such as the skip list's. The wrong base showed up in `codegraph_explore`'s type hierarchy and in impact, and tied the base's methods to the wrong overrides. A template parameter used as a base no longer links to a class that shares its name, and a base written from the global scope, like `public ::testing::Test`, now links to its class. Re-index C++ projects after upgrading. - In C++ and Rust, a call written with a qualifier, like `Status::OK()`, now links to a method of the class it names, never to one of a class whose name only ends the same way. Before, facebook/rocksdb's calls to `Status::OK()`, `Status::InvalidArgument(…)` and `Status::Corruption(…)` linked to `IOStatus`'s methods of those names, a subclass calling its base, like `return FileSystemWrapper::NewWritableFile(…)` or google/leveldb's `return DB::Put(…)`, linked to another subclass or to the calling method itself, and BurntSushi/ripgrep's `Path::new(…)` on the standard library's `Path` linked to its own `PrinterPath::new`. When classes in several languages share the name, the one in the call's own language is taken, so protocolbuffers/protobuf's C++ `DescriptorPool::generated_pool()` calls, which met its Ruby namesake first and linked to nothing, now link to the C++ method. A C++ call through a template parameter, like `CacheShard::ComputeHash(key)` in rocksdb's `template class ShardedCache`, no longer links to a class that merely shares words with the parameter's name, unless that class is the parameter's default, like `TcParser` in `template `. A template's value parameter, like `const PrefetchOpts& kOpts`, is no longer mistaken for a type parameter, so references to `PrefetchOpts` inside it now link. Re-index C++ and Rust projects after upgrading. - In C++, a call through a class template's specialization written with its arguments, like fmtlib/fmt's `formatter::format(42, ctx)`, no longer links to a member defined for another specialization, like a test's `formatter::format`, and when the template declares no such member, as for `formatter::parse(ctx)`, the call is no longer matched to another class's method of that name, like `native_formatter::parse`. A call to the specialization a member is defined for, and to a template's own members, links as before. Re-index C++ projects after upgrading. +- In C++, a function is now named by its declarator, never by something written in the clauses after its parameters. nlohmann/json's constructor `basic_json(CompatibleType&& val) noexcept(noexcept(JSONSerializer::to_json(…)))` was missing from the index, replaced by a method `JSONSerializer::to_json` that doesn't exist and that other `JSONSerializer::to_json(…)` calls linked to; a method declared with `throw(std::bad_alloc)` was indexed as `std::bad_alloc`, and a function with a trailing `requires std::integral` as `std::integral`. Re-index C++ projects after upgrading. - In C and C++, a header or source file named like a test, such as protocolbuffers/protobuf's `conformance_test.h` and `test_runner.h`, now counts as part of the code that `#include`s it, directly or through other headers. Before, calls from that code into it were dropped, so protobuf's conformance suites' `suite_.ReportFailure(…)` and `RunValidInputTest(…)` calls, and its unit tests' calls to the shared `TestUtil` helpers, linked to nothing. A test file that only tests include is still kept out of reach of the rest of your code. Re-index C and C++ projects after upgrading. - In C++, a class, struct or method now keeps the namespaces and the class it is declared in when the parser misreads something earlier in the file, such as an unknown macro in front of a member or in a class header. Before, everything after the misread code could lose them, or land inside a class it isn't in: in protocolbuffers/protobuf, `FieldDescriptor` was indexed without `google::protobuf::`, and in RocksDB, the `Opts` struct declared inside a cache table class was indexed outside it, so finding a class's base classes and the methods called on its objects fell back to guessing by name. Methods that had come loose from their class are its members again, and classes the misread code had hidden, like leveldb's POSIX file and environment classes, are now indexed. Re-index C++ projects after upgrading. - In C++, a call on a variable, parameter or member declared as a standard-library or other outside type with a lowercase name, like `std::string`, `std::vector`, `std::set` or `absl::flat_hash_set`, no longer links to a project method that only shares the method's name, even one of a project class in another namespace with the type's name, like your own `util::flat_hash_set`. A variable declared outside the calling function, like another function's or a file's `std::string text`, no longer links its calls to such a class either. In google/leveldb, `saved_key_.clear()` on a `std::string saved_key_;` reached `Slice::clear`, and in protocolbuffers/protobuf, `proto.append(…)` on a `std::string` reached an unrelated `LeftoverBuffer::append`. The same goes for `->` through a raw pointer like `std::string* out`, while `->` through a smart pointer, iterator or optional still links to the type it holds. Comments also no longer pass for a variable's declaration, so more calls now link to the method of the type the variable is really declared with. A call on a member reached through other members, like facebook/rocksdb's `options.cf_paths.emplace_back(…)` on a `std::vector`, `props->rep.db_id.size()` on a `std::string` or `bbto.filter_policy.reset(…)` on a `std::shared_ptr`, no longer links to a project method of that name either: those had reached `autovector::emplace_back`, `BlobContents::size` and `FSWritableFilePtr::reset`. The same goes for a member of an outside type like `absl::Status`. Re-index C++ projects after upgrading. diff --git a/__tests__/cpp-declarator-clause-names.test.ts b/__tests__/cpp-declarator-clause-names.test.ts new file mode 100644 index 00000000..c18a3e2a --- /dev/null +++ b/__tests__/cpp-declarator-clause-names.test.ts @@ -0,0 +1,137 @@ +/** + * A C++ function is named by its declarator, never by something written in + * the clauses after its parameters. nlohmann/json's constructor + * + * basic_json(CompatibleType && val) noexcept(noexcept( + * JSONSerializer::to_json(std::declval(), …))) + * + * had no node: both extractors looked for a qualified name anywhere in the + * declarator except its parameters and trailing return type, found the call + * inside `noexcept(…)`, and indexed a method `JSONSerializer::to_json` that + * doesn't exist, which other `JSONSerializer::to_json(…)` calls then + * linked to. A `throw(std::bad_alloc)` specification and a trailing + * `requires std::integral` clause did the same. Runs against the native + * kernel (when built) and the wasm extractor, which must agree. + */ +import { describe, it, expect, beforeAll, beforeEach, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as path from 'path'; +import { extractFromSource } from '../src/extraction'; +import { initGrammars, loadGrammarsForLanguages } from '../src/extraction/grammars'; +import { tryKernelExtract, resetKernelForTests } from '../src/extraction/kernel'; +import type { ExtractionResult, Language } from '../src/types'; + +const KERNEL_PATH = path.join( + __dirname, + '..', + 'codegraph-kernel', + 'prebuilds', + `${process.platform}-${process.arch}`, + 'codegraph-kernel.node' +); +const kernelAvailable = fs.existsSync(KERNEL_PATH) || process.env.CODEGRAPH_KERNEL_EXPECT === '1'; + +const ENV_KEYS = ['CODEGRAPH_KERNEL', 'CODEGRAPH_KERNEL_LANGS'] as const; + +/** ` ` for every function and method. */ +function callables(result: ExtractionResult): string[] { + return result.nodes + .filter((n) => n.kind === 'function' || n.kind === 'method') + .map((n) => `${n.kind} ${n.qualifiedName}`); +} + +describe('a C++ function named past the clauses after its parameters', () => { + let savedEnv: Record = {}; + + beforeAll(async () => { + await initGrammars(); + await loadGrammarsForLanguages(['cpp']); + }); + + beforeEach(() => { + savedEnv = Object.fromEntries(ENV_KEYS.map((k) => [k, process.env[k]])); + resetKernelForTests(); + }); + + afterEach(() => { + for (const k of ENV_KEYS) { + if (savedEnv[k] === undefined) delete process.env[k]; + else process.env[k] = savedEnv[k]; + } + resetKernelForTests(); + }); + + function extract(backend: 'kernel' | 'wasm', file: string, source: string, language: Language): ExtractionResult { + if (backend === 'wasm') { + process.env.CODEGRAPH_KERNEL = '0'; + return extractFromSource(file, source, language); + } + delete process.env.CODEGRAPH_KERNEL; + process.env.CODEGRAPH_KERNEL_LANGS = 'all'; + const result = tryKernelExtract(file, source, language); + expect(result, `kernel extraction of ${file}`).not.toBeNull(); + return result!; + } + + const backends = kernelAvailable ? (['kernel', 'wasm'] as const) : (['wasm'] as const); + + for (const crlf of [false, true]) { + const eol = (s: string) => (crlf ? s.replace(/\n/g, '\r\n') : s); + const label = crlf ? ' (CRLF)' : ''; + + it.each(backends)(`reproduction: nlohmann/json's constructor with noexcept(noexcept(JSONSerializer::to_json(…))): %s${label}`, (backend) => { + const source = [ + 'namespace nlohmann {', + 'template class JSONSerializer>', + 'class basic_json {', + ' public:', + ' template < typename CompatibleType, typename U = CompatibleType, int = 0 >', + ' basic_json(CompatibleType && val) noexcept(noexcept( // NOLINT(bugprone-forwarding-reference-overload)', + ' JSONSerializer::to_json(*this,', + ' val)))', + ' {', + ' JSONSerializer::to_json(*this, val);', + ' }', + '};', + '} // namespace nlohmann', + '', + ].join('\n'); + const result = extract(backend, 'include/json.hpp', eol(source), 'cpp'); + expect(callables(result)).toEqual(['method nlohmann::basic_json::basic_json']); + }); + + it.each(backends)(`a throw(std::bad_alloc) specification and a trailing requires clause: %s${label}`, (backend) => { + const source = [ + 'namespace util {', + 'class Buffer {', + ' public:', + ' void Reserve(int n) throw(std::bad_alloc) {}', + '};', + 'template ', + 'T Twice(T v) requires std::integral {', + ' return v * 2;', + '}', + '} // namespace util', + '', + ].join('\n'); + const result = extract(backend, 'src/buffer.cpp', eol(source), 'cpp'); + expect(callables(result)).toEqual(['method util::Buffer::Reserve', 'function util::Twice']); + }); + + it.each(backends)(`an out-of-line method keeps its own qualified name past a noexcept clause: %s${label}`, (backend) => { + const source = [ + 'namespace util {', + 'struct Pool {', + ' int Get() const;', + '};', + 'int Pool::Get() const noexcept(noexcept(Other::Make())) {', + ' return 1;', + '}', + '} // namespace util', + '', + ].join('\n'); + const result = extract(backend, 'src/pool.cpp', eol(source), 'cpp'); + expect(callables(result)).toEqual(['method util::Pool::Get']); + }); + } +}); diff --git a/__tests__/fixtures/kernel-parity/torture.cpp b/__tests__/fixtures/kernel-parity/torture.cpp index b2ba1ea0..d4041dc2 100644 --- a/__tests__/fixtures/kernel-parity/torture.cpp +++ b/__tests__/fixtures/kernel-parity/torture.cpp @@ -155,3 +155,19 @@ float drive() { (void)m; return r + f + flags + leg; } +// A function named past its noexcept/throw/requires clauses. +namespace clauses { +template