Files
codegraph/__tests__/cpp-pimpl-member-chains.test.ts
T
Colby MchenryandClaude Opus 5.5 bdf288a76f fix(cpp): a pimpl's Rep, which a header names and its source file defines, is found for the code in that file (#2733)
A C++ name is looked up from where it is written, and a class that a
source file defines belongs to that file's translation unit, so only that
file sees it (#2538). A header that declares `struct Rep;` and
`std::unique_ptr<Rep> rep_;` writes the name for a `Table::Rep` that its
source file defines, and code in that file reads the member through it.
The lookup ran from the header, saw no source file's classes and found
nothing. The call was then guessed by the method's name, often onto the
calling class's own method:
- leveldb's `rep_->index_block->NewIterator(…)` reached
  `Table::NewIterator`;
- rocksdb's `rep->write_batch.GetDataSize()` reached
  `WriteBatchWithIndex::GetDataSize`;
- rocksdb's `level_iters_[i]->status()` reached `ForwardIterator::status`.

cppClassWritten's lookup now also sees the classes of the file the
reference is in, the translation unit that includes the header
(visibleFrom(file, unit)). classNode filters by that same visibility
instead of recomputing the header's.

A member-chain diagnostic on main 81fd0078 found 99 rocksdb and 7 leveldb
chain calls through a `Rep` whose class couldn't be placed.

Kit A/B against main 5994edb3:

| repo | removed | added | retargeted | same target, now typed |
|---|---|---|---|---|
| rocksdb | 34 | 40 | 43 | 56 |
| fmt | 0 | 27 | 0 | 1 |
| protobuf | 25 | 1 | 2 | 2 |
| leveldb | 0 | 1 | 3 | 3 |
| redis | identical | | | |

- Retargeted: from the calling class's own method or a namesake to the
  member's class:
  - rocksdb: WriteBatch, ForwardLevelIterator, IndexBuilder and
    PartitionedIndexBuilder, RangeDelBlockBuilder, Env, MemFile;
  - leveldb: Block::NewIterator;
  - protobuf: DescriptorPool::Tables::FindSymbol.
- Removed: calls on library members reached this way, which had linked to
  namesakes:
  - `std::stack::pop` (BinaryHeap::pop);
  - `std::vector::size` (Block::size);
  - `std::thread::join` (WindowsThread::join);
  - absl::flat_hash_set's `clear` (Map::clear);
  - `std::optional`'s `has_value` and `reset` (Option,
    ScopedPythonPtr).
- Added:
  - WriteBatch's Put, Delete, Merge and the rest, through
    WriteBatchWithIndex's `rep->write_batch`;
  - gtest's UnitTestImpl through `impl()`, in fmt's and rocksdb's vendored
    gtest;
  - leveldb's FilterPolicy::Name.
- About 35 changed sites read by hand were all right.

Mutations: dropping the unit from visibleFrom, or from the lookup's call,
fails the new tests. classNode's own filter only chooses among several
declarations of one class, and nothing on the kit or in the tests has two,
so dropping it survives.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-21 07:31:38 +00:00

116 lines
3.7 KiB
TypeScript

/**
* A C++ call through a member of a pimpl's `Rep` — rocksdb's
* `rep_->index_builder->IndexSize()`, leveldb's `rep_->options.comparator` —
* calls a method of that member's class. The class declares `struct Rep;` and
* `std::unique_ptr<Rep> rep_;` (or `Rep* rep_;`) in its header, and the source
* file that calls through it defines `struct Table::Rep { … }`. The member's
* type was looked up from the header, which sees no source file's classes, so
* the call was guessed by the method's name.
*/
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import { CodeGraph } from '../src';
import type { Node } from '../src/types';
const lines = (...l: string[]): string => [...l, ''].join('\n');
describe('a C++ call through a member of a pimpl Rep', () => {
let root: string;
let cg: CodeGraph;
beforeAll(async () => {
root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cpp-pimpl-'));
const write = (file: string, text: string): void => {
fs.mkdirSync(path.dirname(path.join(root, file)), { recursive: true });
fs.writeFileSync(path.join(root, file), text);
};
write('include/table.h', lines(
'#include <memory>',
'namespace ns {',
'class Table {',
' public:',
' int Size();',
' void Flush();',
' private:',
' struct Rep;',
' std::unique_ptr<Rep> rep_;',
'};',
'class Builder {',
' public:',
' int Count();',
' private:',
' struct Rep;',
' Rep* rep_;',
'};',
'} // namespace ns',
));
write('src/table.cc', lines(
'#include "include/table.h"',
'namespace ns {',
'class Block {',
' public:',
' int size() const { return 1; }',
' void Flush() {}',
'};',
'struct Table::Rep {',
' Block block;',
' Block* index;',
'};',
// Each namesake sits where a call matched by its bare name lands on it.
'class Columns {',
' public:',
' int size() const { return 0; }',
' void Flush() {}',
'};',
'int Table::Size() { return rep_->block.size(); }',
'void Table::Flush() { rep_->index->Flush(); }',
'} // namespace ns',
));
write('src/builder.cc', lines(
'#include "include/table.h"',
'namespace ns {',
'class Counter {',
' public:',
' int size() const { return 2; }',
'};',
'struct Builder::Rep {',
' Counter counter;',
'};',
'class Tally {',
' public:',
' int size() const { return 3; }',
'};',
'int Builder::Count() { return rep_->counter.size(); }',
'} // namespace ns',
));
cg = await CodeGraph.init(root, { index: true });
}, 120_000);
afterAll(() => {
cg?.close();
if (root) fs.rmSync(root, { recursive: true, force: true });
});
/** `calls` callees of the function or method `qualifiedName`, as `qualifiedName`. */
const calls = (qualifiedName: string): string[] => {
const caller = [...cg.getNodesByKind('function'), ...cg.getNodesByKind('method')]
.find((n: Node) => n.qualifiedName === qualifiedName);
expect(caller, qualifiedName).toBeDefined();
return cg.getCallees(caller!.id).filter((r) => r.edge.kind === 'calls').map((r) => r.node.qualifiedName).sort();
};
it("a member of the Rep a smart pointer holds, defined in the calling file", () => {
expect(calls('ns::Table::Size')).toEqual(['ns::Block::size']);
});
it("a pointer member of the Rep", () => {
expect(calls('ns::Table::Flush')).toEqual(['ns::Block::Flush']);
});
it('a raw Rep pointer', () => {
expect(calls('ns::Builder::Count')).toEqual(['ns::Counter::size']);
});
});