mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-11-14 16:52:09 +08:00
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 <noreply@anthropic.com>
This commit is contained in:
committed by
Colby Mchenry
co-authored by
Claude Opus 5.5
parent
fa03fa6bb4
commit
0d7480f83b
@@ -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<const char*, KeyComparator> 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<std::pair<int, FileMetaData*>>` or `Striped<CacheAlignedWrapper<port::Mutex>>`, 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 CacheShard> 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 <typename Parser = TcParser>`. 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.
|
||||
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
@@ -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<string>();
|
||||
|
||||
/**
|
||||
* 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<CppArgumentType | null>;
|
||||
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);
|
||||
|
||||
Reference in New Issue
Block a user