From 0d7480f83b09098efb4f46556a7de3e98c038688 Mon Sep 17 00:00:00 2001 From: Josh66 Date: Fri, 30 Oct 2026 06:19:24 -0500 Subject: [PATCH] fix(cpp): an overload chosen by arguments that call it again no longer overflows the stack On main, indexing darts-clone failed in "Resolving refs" with "Maximum call stack size exceeded", and the whole run was lost. darts.h writes `extras(extras(id).prev()).set_next(extras(id).next())` on a class with a const and a non-const `extras(id)`. Choosing between them types the arguments; typing `extras(id).prev()` types `extras(id)`, whose overload is chosen by the arguments of the call the reference is at, the outer one, so the same arguments are typed again. cppArgumentType restarts cppExpressionType's depth at 0 on each lap, so the depth bound never trips. cppArgumentTypeChoice now keeps the calls whose arguments it is typing (`file:line:column:name`), and a choice that comes back to the same call takes the first tied overload, the answer it already gives when no argument's type is known. A try/finally clears the entry. Validation on main 5526e33d (Windows 11): - Red/green: the new test fails with main's name-matcher.ts (RangeError: Maximum call stack size exceeded) and passes with the fix. - A/B against main: darts-clone fails on main and indexes with the fix, 503 more linked sites (897 edges against the 394 the failed run left), with `reserve_id` and `expand_units` reaching `extras`, `prev`, `next`, `set_prev` and `set_next`. leveldb, fmt, googletest and nlohmann/json are byte-identical, as are the express and flask controls. - Index time unchanged: median of 3 interleaved runs on nlohmann/json, 15.9 s on main and 16.0 s with the fix. Original PR #2999 by @JosefAschauer. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + .../cpp-overload-argument-reentry.test.ts | 67 +++++++++++++++++++ src/resolution/name-matcher.ts | 17 ++++- 3 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 __tests__/cpp-overload-argument-reentry.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index abbe59c18..c59e9da35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -209,6 +209,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In C, C++ and Objective-C, a struct or class member declared as a pointer, reference, array or function, like `SharedState* shared;`, `jv elements[];` or `virtual Status Put(…) = 0;`, no longer makes the struct look like it inherits from the member's type. Before, `codegraph_explore` and impact listed those types as base classes, every struct holding a pointer to a type as one of its subclasses, and a method of that struct as an override of the type's method with the same name. Real base classes like `class Derived : public Base`, Objective-C superclasses and Go's embedded fields are unchanged. Re-index C, C++ and Objective-C projects after upgrading. - In C and C++, a function-pointer table's handler, like `.read = impl_read` in an ops struct, no longer links to a Python or Rust function that only shares its name, and such a function no longer takes the place of the C function the table names. Re-index C and C++ projects that also hold Python or Rust after upgrading. Thanks @bompus. - In C++, a call on a variable, parameter or member declared through a `typedef` or `using` alias now links to the method of the type the alias names, with the alias looked up the way C++ does it: in the calling function, its class and the classes that class inherits from, then the namespaces around it. In google/leveldb, `table_.Insert(buf)` on a `Table table_;` declared next to `typedef SkipList Table;` now reaches `SkipList::Insert` instead of the unrelated `HandleTable::Insert`, and `Table::Iterator iter(&table_);` reaches `SkipList::Iterator`'s constructor. A class that only shares the alias's name is no longer taken for it, and when the aliased type has no such method, as with an alias of `std::vector` or of a template parameter, no method is guessed from the receiver's name. An alias of a type written from the global scope names that global type, also inside a namespace of the same name: protocolbuffers/protobuf's `using Arena = ::upb::Arena;` in `hpb::internal::backend::upb`, so hpb's `arena_.Fuse(…)` on a `backend::Arena` now reaches `upb::Arena::Fuse` instead of the calling method itself. Re-index C++ projects after upgrading. +- In C++, indexing no longer fails with "Maximum call stack size exceeded" on a call whose arguments call the same overloaded method, like darts-clone's `extras(extras(id).prev()).set_next(…)` on a class with a `const` and a non-`const` `extras(id)`. Choosing the overload read its arguments, which chose the same overload again without end, and the whole index run failed. Such a call now takes the first overload when the choice comes back to itself, as one whose argument types are unknown does. Thanks @JosefAschauer. - In C++, a call on a variable, parameter or member declared with template arguments nested inside its template arguments, like `autovector>` or `Striped>`, now links to the method of the class it is declared as. Before, the nested arguments hid that class: in RocksDB, `files_marked_for_compaction_.clear()` on such an `autovector` linked to an unrelated `CompactionInputFiles::clear` that only shares words with the variable's name, and `mutex_.Get(key)` on a `Striped` linked to nothing. Re-index C++ projects after upgrading. - 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. diff --git a/__tests__/cpp-overload-argument-reentry.test.ts b/__tests__/cpp-overload-argument-reentry.test.ts new file mode 100644 index 000000000..75d989286 --- /dev/null +++ b/__tests__/cpp-overload-argument-reentry.test.ts @@ -0,0 +1,67 @@ +/** + * Choosing among a C++ method's overloads types the call's arguments, and an + * argument can be a call of that same overloaded method: + * `extras(extras(id).prev()).set_next(…)` on a class with a const and a + * non-const `extras(id)`. Typing `extras(id).prev()` types `extras(id)`, whose + * overload is chosen by the arguments of the call the reference is at — the + * outer one — so the choice re-entered itself with a fresh depth on every lap + * until the stack ran out, and the whole index failed ("Maximum call stack size + * exceeded"). A re-entered choice now takes the first overload, as one whose + * arguments' types are unknown does. + */ +import { describe, it, expect, afterAll, beforeAll } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; + +let root = ''; +let cg: CodeGraph | undefined; + +beforeAll(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-cpp-overload-reentry-')); + fs.mkdirSync(path.join(root, 'src')); + fs.writeFileSync(path.join(root, 'src/builder.h'), `#pragma once + +class Builder { + public: + struct Extra { + int prev() const { return prev_; } + int next() const { return next_; } + void set_prev(int prev) { prev_ = prev; } + void set_next(int next) { next_ = next; } + int prev_; + int next_; + }; + + void unlink(int id) { + extras(extras(id).prev()).set_next(extras(id).next()); + extras(extras(id).next()).set_prev(extras(id).prev()); + } + + private: + const Extra &extras(int id) const { return extras_[id]; } + Extra &extras(int id) { return extras_[id]; } + + Extra extras_[16]; +}; +`); +}); + +afterAll(() => { + cg?.close(); + if (root) fs.rmSync(root, { recursive: true, force: true }); +}); + +describe('a C++ overload chosen by arguments that call the same overloaded method', () => { + it('indexes, and links the calls in the method', async () => { + cg = await CodeGraph.init(root, { index: true }); + const unlink = cg.getNodesInFile('src/builder.h').find((n) => n.name === 'unlink')!; + expect(unlink).toBeDefined(); + const called = new Set(cg.getOutgoingEdges(unlink.id).filter((e) => e.kind === 'calls') + .map((e) => cg!.getNode(e.target)!.name)); + expect(called).toContain('extras'); + expect(called).toContain('set_next'); + expect(called).toContain('set_prev'); + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index c01c1b6a6..e2a180ee1 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -17608,6 +17608,9 @@ function cppOverloadFor(candidates: Node[], name: string, ref: UnresolvedRef, co return cppArgumentTypeChoice(overloads.filter((_, i) => fits[i] === top), name, args, ref, context); } +/** The calls (`file:line:column:name`) whose arguments cppArgumentTypeChoice is typing. */ +const CPP_ARGUMENT_CHOICES_IN_FLIGHT = new Set(); + /** * Of overloads the call's arguments `args` fit alike by count (in * preference order), the one the types the arguments have where the call @@ -17616,7 +17619,19 @@ function cppOverloadFor(candidates: Node[], name: string, ref: UnresolvedRef, co */ function cppArgumentTypeChoice(tied: Node[], name: string, args: string[], ref: UnresolvedRef, context: ResolutionContext): Node { if (tied.length === 1 || args.length === 0) return tied[0]!; - const types = args.map((arg) => cppArgumentType(arg, ref, context)); + // Typing an argument can come back to this choice: in `extras(extras(id).prev())` + // on an overloaded `extras`, the inner `extras(id)` is chosen by the arguments + // of the call the reference is at — the outer one — and each lap starts its + // depth afresh. Re-entered, take the first, as when no argument's type is known. + const key = `${ref.filePath}:${ref.line}:${ref.column}:${name}`; + if (CPP_ARGUMENT_CHOICES_IN_FLIGHT.has(key)) return tied[0]!; + CPP_ARGUMENT_CHOICES_IN_FLIGHT.add(key); + let types: Array; + try { + types = args.map((arg) => cppArgumentType(arg, ref, context)); + } finally { + CPP_ARGUMENT_CHOICES_IN_FLIGHT.delete(key); + } if (types.every((t) => t === null)) return tied[0]!; const fits = tied.map((n) => cppArgumentTypeFit(n, name, types, context)); const top = Math.max(...fits);