Repository navigation
merge: reconcile upstream through 1753472a #497
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
31 commits
Select commit
Hold shift + click to select a range
fedda29
fix(cpp): a declaration with a qualifier or annotation around its nam…
colbymchenry bb817d4
fix(cpp): structured bindings and function wrappers give their types …
colbymchenry 8d91806
fix(cpp): a bare call in a gtest TEST_F or TEST_P body reaches the fi…
colbymchenry 0cea43e
fix(cpp): `->` on an absl::StatusOr reaches the class it holds; a usi…
colbymchenry 61cb418
fix(cpp): a call or type name never links to what only another source…
colbymchenry a6ceb04
fix(cpp): a call to an overloaded method links to the overload its ar…
colbymchenry 34dd3d2
fix(cpp): a nested class defined out of line is indexed under its nam…
colbymchenry 6df4827
fix(cpp): a call through a class reaches that class's own or inherite…
colbymchenry 754cc45
fix(cpp): an alias naming a class through a derived class leads to th…
colbymchenry d94a92e
fix(cpp): a gtest TEST_F or TEST_P body finds its fixture's types, an…
colbymchenry d4b9c2b
fix(cpp): a line starting with `*` is read as code, and block comment…
colbymchenry 2428621
fix(cpp): a comparison or shift in a call's arguments opens no templa…
colbymchenry 8cba7e5
fix(cpp): a nullptr argument picks the std::nullptr_t overload, and n…
colbymchenry e03ff67
fix(cpp): a receiver declared over more than one line has its type re…
colbymchenry 1753472
fix(cpp): a call of a member template with explicit template argument…
colbymchenry a4a3c40
merge: upstream/main through 1753472a (15 C++ resolution fixes)
bompus dc2289f
fix(cpp): link no call or type name to what only another source file …
bompus 1b30d19
fix(cpp): a call through a class reaches its own or inherited method …
bompus 20bdbed
fix(cpp): read receivers through annotations, bindings and wrappers, …
bompus 72e973a
fix(cpp): a call of a member template with explicit template argument…
bompus 884719f
fix(cpp): a construction makes the class C++ lookup finds for the typ…
bompus b9510f0
fix(cpp): read a receiver declared as a pointer to a class template o…
bompus f92479d
fix(cpp): blank block comments and string contents before reading dec…
bompus 7e5e1ba
test(cpp): keep the fork's no-guess result for receivers whose type c…
bompus b5d1647
test(golden): re-baseline for C++ constructions placed by lookup
bompus 9d5b66d
fix(cpp): place a construction in a test tree's header only from that…
bompus c876f90
docs: record where the fork's C++ resolution differs from upstream's …
bompus 125841c
merge: bring fork/consolidated ce0c73db into the upstream 1753472a re…
bompus 666cd74
fix(cpp): answer review findings on the upstream reconcile
bompus 89a7907
test(cpp): use a valid call in the deleted-overload fixture
bompus baaed0d
perf(cpp): keep the visible-file set of one C++ file at a time
bompus File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,217 @@ | ||
| /** | ||
| * A C++ `->` call on an `absl::StatusOr`, an `absl::optional` or an | ||
| * `absl::NoDestructor` calls a member of the type it holds, as one on a | ||
| * `std::unique_ptr` or `std::optional` does; and a bare call in a class | ||
| * template finds a member its dependent base declares when the class brings | ||
| * the name in with a using-declaration. | ||
| * | ||
| * In protocolbuffers/protobuf, `absl::StatusOr<FeatureResolver> resolver = | ||
| * FeatureResolver::Create(…); resolver->MergeFeatures(…)` was guessed by the | ||
| * method's name: the declared type's last name, `StatusOr`, is no class of | ||
| * the project's. In facebook/rocksdb's typed_cache.h, `using | ||
| * BasicTypedCacheHelperFns<TValue>::DownCastValue;` makes a bare | ||
| * `DownCastValue(v)` the base's, which C++ doesn't look in otherwise: the | ||
| * base depends on the class's template parameters. | ||
| */ | ||
| import { describe, it, expect, afterEach } 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 roots: string[] = []; | ||
| afterEach(() => { | ||
| for (const root of roots.splice(0)) fs.rmSync(root, { recursive: true, force: true }); | ||
| }); | ||
|
|
||
| async function indexed(files: Record<string, string>): Promise<CodeGraph> { | ||
| const root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cpp-absl-held-')); | ||
| roots.push(root); | ||
| for (const [rel, content] of Object.entries(files)) { | ||
| fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); | ||
| fs.writeFileSync(path.join(root, rel), content); | ||
| } | ||
| return CodeGraph.init(root, { index: true }); | ||
| } | ||
|
|
||
| function callable(cg: CodeGraph, qualifiedName: string): Node { | ||
| const node = [...cg.getNodesByKind('method'), ...cg.getNodesByKind('function')].find((n) => n.qualifiedName === qualifiedName); | ||
| if (!node) throw new Error(`no function or method ${qualifiedName}`); | ||
| return node; | ||
| } | ||
|
|
||
| /** `calls` callees of a function or method, as `qualifiedName @confidence`. */ | ||
| function calls(cg: CodeGraph, caller: string): string[] { | ||
| return cg | ||
| .getCallees(callable(cg, caller).id) | ||
| .filter((r) => r.edge.kind === 'calls') | ||
| .map((r) => `${r.node.qualifiedName} @${(r.edge.metadata as { confidence?: number } | undefined)?.confidence}`) | ||
| .sort(); | ||
| } | ||
|
|
||
| const lines = (...text: string[]): string => [...text, ''].join('\n'); | ||
|
|
||
| // Two classes with a method of each name, so a guess by the name can miss. | ||
| const RESOLVERS = lines( | ||
| 'namespace google {', | ||
| 'namespace protobuf {', | ||
| 'class FeatureResolver {', | ||
| ' public:', | ||
| ' int MergeFeatures(int parent, int child) const { return parent; }', | ||
| '};', | ||
| 'class FeatureSetResolver {', | ||
| ' public:', | ||
| ' int MergeFeatures(int parent, int child) const { return child; }', | ||
| '};', | ||
| '} // namespace protobuf', | ||
| '} // namespace google', | ||
| ); | ||
|
|
||
| describe('C++ absl holders and using-declared members of dependent bases', () => { | ||
| it("is the held class's method for `->` on an absl::StatusOr, absl::optional or absl::NoDestructor", async () => { | ||
| const cg = await indexed({ | ||
| 'src/feature_resolver.h': RESOLVERS, | ||
| 'src/feature_resolver_test.cc': lines( | ||
| '#include "src/feature_resolver.h"', | ||
| 'namespace google {', | ||
| 'namespace protobuf {', | ||
| 'int Merge(absl::StatusOr<FeatureResolver> resolver) {', | ||
| ' return resolver->MergeFeatures(1, 2);', | ||
| '}', | ||
| 'int MergeOptional(absl::optional<FeatureResolver> resolver) {', | ||
| ' return resolver->MergeFeatures(1, 2);', | ||
| '}', | ||
| 'int MergeStatic() {', | ||
| ' static absl::NoDestructor<FeatureResolver> resolver;', | ||
| ' return resolver->MergeFeatures(1, 2);', | ||
| '}', | ||
| '} // namespace protobuf', | ||
| '} // namespace google', | ||
| ), | ||
| }); | ||
| try { | ||
| for (const caller of ['Merge', 'MergeOptional', 'MergeStatic']) { | ||
| expect(calls(cg, `google::protobuf::${caller}`)).toEqual(['google::protobuf::FeatureResolver::MergeFeatures @0.9']); | ||
| } | ||
| } finally { | ||
| cg.close(); | ||
| } | ||
| }); | ||
|
|
||
| it("is not a project's own StatusOr, nor what a StatusOr of a pointer holds", async () => { | ||
| const cg = await indexed({ | ||
| 'src/feature_resolver.h': RESOLVERS, | ||
| // protobuf's hpb declares a `StatusOr` of its own. | ||
| 'hpb/status.h': lines( | ||
| 'namespace absl {', | ||
| 'template <typename T>', | ||
| 'class StatusOr {', | ||
| ' public:', | ||
| ' T value() const;', | ||
| '};', | ||
| '} // namespace absl', | ||
| ), | ||
| 'src/use.cc': lines( | ||
| '#include "src/feature_resolver.h"', | ||
| '#include "hpb/status.h"', | ||
| 'namespace google {', | ||
| 'namespace protobuf {', | ||
| 'int Merge(absl::StatusOr<FeatureResolver> resolver) {', | ||
| ' return resolver->MergeFeatures(1, 2);', | ||
| '}', | ||
| 'int MergeThroughPointer(absl::StatusOr<FeatureResolver*> resolver) {', | ||
| ' return resolver->MergeFeatures(1, 2);', | ||
| '}', | ||
| '} // namespace protobuf', | ||
| '} // namespace google', | ||
| ), | ||
| }); | ||
| try { | ||
| expect(calls(cg, 'google::protobuf::Merge')).toEqual([]); | ||
| expect(calls(cg, 'google::protobuf::MergeThroughPointer')).toEqual([]); | ||
| } finally { | ||
| cg.close(); | ||
| } | ||
| }); | ||
|
|
||
| it('is the member of the base the using-declaration names, of two dependent bases', async () => { | ||
| const cg = await indexed({ | ||
| 'util/runner.h': lines( | ||
| 'namespace rocksdb {', | ||
| 'template <class T>', | ||
| 'class Second {', | ||
| ' public:', | ||
| ' void Run() {}', | ||
| '};', | ||
| 'template <class T>', | ||
| 'class First {', | ||
| ' public:', | ||
| ' void Run() {}', | ||
| '};', | ||
| 'template <class T>', | ||
| 'class Both : public Second<T>, public First<T> {', | ||
| ' public:', | ||
| ' using First<T>::Run;', | ||
| ' void Go() { Run(); }', | ||
| '};', | ||
| '} // namespace rocksdb', | ||
| ), | ||
| }); | ||
| try { | ||
| expect(calls(cg, 'rocksdb::Both::Go')).toEqual(['rocksdb::First::Run @0.9']); | ||
| } finally { | ||
| cg.close(); | ||
| } | ||
| }); | ||
|
|
||
| it('is the dependent base member a using-declaration brings in, for a bare call and a data member', async () => { | ||
| const cg = await indexed({ | ||
| // (The base in a header of its own, where the derived class's file doesn't write its member.) | ||
| 'cache/typed_cache_base.h': lines( | ||
| 'namespace rocksdb {', | ||
| 'class Cache {', | ||
| ' public:', | ||
| ' int Insert(int key) { return key; }', | ||
| '};', | ||
| 'class Other {', | ||
| ' public:', | ||
| ' int Insert(int key) { return 0; }', | ||
| '};', | ||
| 'template <class TValue>', | ||
| 'class BasicTypedCacheHelperFns {', | ||
| ' public:', | ||
| ' static TValue* DownCastValue(void* v) { return nullptr; }', | ||
| ' protected:', | ||
| ' Cache* cache_;', | ||
| '};', | ||
| '} // namespace rocksdb', | ||
| ), | ||
| 'cache/typed_cache.h': lines( | ||
| '#include "cache/typed_cache_base.h"', | ||
| 'namespace rocksdb {', | ||
| 'inline int* DownCastValue(void* v) { return nullptr; }', | ||
| 'template <class TValue, class TCreateContext>', | ||
| 'class FullTypedCacheHelperFns : public BasicTypedCacheHelperFns<TValue> {', | ||
| ' public:', | ||
| ' using BasicTypedCacheHelperFns<TValue>::DownCastValue;', | ||
| ' using BasicTypedCacheHelperFns<TValue>::cache_;', | ||
| ' static void Delete(void* v) {', | ||
| ' DownCastValue(v);', | ||
| ' }', | ||
| ' int Store(int key) {', | ||
| ' return cache_->Insert(key);', | ||
| ' }', | ||
| '};', | ||
| '} // namespace rocksdb', | ||
| ), | ||
| }); | ||
| try { | ||
| // (Without the using-declarations C++ finds the namespace's `DownCastValue`, as before.) | ||
| expect(calls(cg, 'rocksdb::FullTypedCacheHelperFns::Delete')).toEqual(['rocksdb::BasicTypedCacheHelperFns::DownCastValue @0.9']); | ||
| expect(calls(cg, 'rocksdb::FullTypedCacheHelperFns::Store')).toEqual(['rocksdb::Cache::Insert @0.9']); | ||
| } finally { | ||
| cg.close(); | ||
| } | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,151 @@ | ||
| /** | ||
| * A C++ alias can name a class one of a class's bases declares: | ||
| * protocolbuffers/protobuf's conformance suites write `using | ||
| * ConformanceRequestSetting = BinaryAndJsonConformanceSuite::ConformanceRequestSetting;`, | ||
| * a class their base `ConformanceTestSuite` declares. Following the alias | ||
| * looked the class up only by the qualified name as spelled, which no class | ||
| * has, so constructions through the alias stopped at the alias and calls on | ||
| * such a variable lost their class. The aliased type is now looked up the way | ||
| * a declared type is, a class's members through its bases too. | ||
| */ | ||
| import { describe, it, expect, afterEach } from 'vitest'; | ||
| import * as fs from 'fs'; | ||
| import * as os from 'os'; | ||
| import * as path from 'path'; | ||
| import { CodeGraph } from '../src'; | ||
|
|
||
| const roots: string[] = []; | ||
| afterEach(() => { | ||
| for (const root of roots.splice(0)) fs.rmSync(root, { recursive: true, force: true }); | ||
| }); | ||
|
|
||
| async function indexed(files: Record<string, string>): Promise<CodeGraph> { | ||
| const root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cpp-alias-base-')); | ||
| roots.push(root); | ||
| for (const [rel, content] of Object.entries(files)) { | ||
| fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); | ||
| fs.writeFileSync(path.join(root, rel), content); | ||
| } | ||
| return CodeGraph.init(root, { index: true }); | ||
| } | ||
|
|
||
| const lines = (...text: string[]): string => [...text, ''].join('\n'); | ||
|
|
||
| /** What a function's or method's `kind` edges reach, as `qualifiedName @confidence`. */ | ||
| function targets(cg: CodeGraph, qualifiedName: string, kind: 'calls' | 'instantiates'): string[] { | ||
| const from = [...cg.getNodesByKind('method'), ...cg.getNodesByKind('function')].find((n) => n.qualifiedName === qualifiedName)!; | ||
| return cg.getOutgoingEdgesFrom([from.id], [kind]) | ||
| .map((e) => `${cg.getNode(e.target)?.qualifiedName} @${(e.metadata as { confidence?: number } | undefined)?.confidence}`) | ||
| .sort(); | ||
| } | ||
|
|
||
| const NS = 'google::protobuf::conformance'; | ||
| const SUITE = lines( | ||
| 'namespace google {', | ||
| 'namespace protobuf {', | ||
| 'namespace conformance {', | ||
| 'class ConformanceTestSuite {', | ||
| ' public:', | ||
| ' class ConformanceRequestSetting {', | ||
| ' public:', | ||
| ' ConformanceRequestSetting(int level, const char* name) {}', | ||
| ' const char* GetTestName() const { return ""; }', | ||
| ' };', | ||
| '};', | ||
| '} // namespace conformance', | ||
| '} // namespace protobuf', | ||
| '} // namespace google', | ||
| ); | ||
|
|
||
| describe('C++ aliases of a class a base declares', () => { | ||
| it('reach the class through an alias spelled through the derived class', async () => { | ||
| const cg = await indexed({ | ||
| 'conformance/conformance_test.h': SUITE, | ||
| 'conformance/binary_json_conformance_suite.h': lines( | ||
| '#include "conformance/conformance_test.h"', | ||
| 'namespace google {', | ||
| 'namespace protobuf {', | ||
| 'namespace conformance {', | ||
| 'class BinaryAndJsonConformanceSuite : public ConformanceTestSuite {};', | ||
| 'template <typename MessageType>', | ||
| 'class BinaryAndJsonConformanceSuiteImpl {', | ||
| ' public:', | ||
| ' void RunTest();', | ||
| ' private:', | ||
| ' using ConformanceRequestSetting =', | ||
| ' BinaryAndJsonConformanceSuite::ConformanceRequestSetting;', | ||
| '};', | ||
| '} // namespace conformance', | ||
| '} // namespace protobuf', | ||
| '} // namespace google', | ||
| ), | ||
| 'conformance/binary_json_conformance_suite.cc': lines( | ||
| '#include "conformance/binary_json_conformance_suite.h"', | ||
| 'namespace google {', | ||
| 'namespace protobuf {', | ||
| 'namespace conformance {', | ||
| 'template <typename MessageType>', | ||
| 'void BinaryAndJsonConformanceSuiteImpl<MessageType>::RunTest() {', | ||
| ' ConformanceRequestSetting setting(1, "x");', | ||
| ' setting.GetTestName();', | ||
| '}', | ||
| '// An alias the function declares itself is not indexed: read from the function.', | ||
| 'void RunLocal() {', | ||
| ' using ConformanceRequestSetting = BinaryAndJsonConformanceSuite::ConformanceRequestSetting;', | ||
| ' ConformanceRequestSetting local(3, "w");', | ||
| ' local.GetTestName();', | ||
| '}', | ||
| '} // namespace conformance', | ||
| '} // namespace protobuf', | ||
| '} // namespace google', | ||
| ), | ||
| }); | ||
| try { | ||
| const run = `${NS}::BinaryAndJsonConformanceSuiteImpl::RunTest`; | ||
| expect(targets(cg, run, 'instantiates')).toEqual([`${NS}::ConformanceTestSuite::ConformanceRequestSetting @0.9`]); | ||
| expect(targets(cg, run, 'calls').filter((t) => t.includes('GetTestName'))) | ||
| .toEqual([`${NS}::ConformanceTestSuite::ConformanceRequestSetting::GetTestName @0.9`]); | ||
| expect(targets(cg, `${NS}::RunLocal`, 'instantiates')).toEqual([`${NS}::ConformanceTestSuite::ConformanceRequestSetting @0.9`]); | ||
| expect(targets(cg, `${NS}::RunLocal`, 'calls').filter((t) => t.includes('GetTestName'))) | ||
| .toEqual([`${NS}::ConformanceTestSuite::ConformanceRequestSetting::GetTestName @0.9`]); | ||
| } finally { | ||
| cg.close(); | ||
| } | ||
| }); | ||
|
|
||
| it("reach the class through an alias a derived class writes with the base's name alone", async () => { | ||
| const cg = await indexed({ | ||
| 'conformance/conformance_test.h': SUITE, | ||
| 'conformance/text_format_suite.h': lines( | ||
| '#include <string>', | ||
| '#include "conformance/conformance_test.h"', | ||
| 'namespace google {', | ||
| 'namespace protobuf {', | ||
| 'namespace conformance {', | ||
| 'class TextFormatSuite : public ConformanceTestSuite {', | ||
| ' public:', | ||
| ' using Setting = ConformanceRequestSetting;', | ||
| ' using Name = std::string;', | ||
| ' void Run() {', | ||
| ' Setting setting(2, "y");', | ||
| ' setting.GetTestName();', | ||
| ' Name name("z");', | ||
| ' }', | ||
| '};', | ||
| '} // namespace conformance', | ||
| '} // namespace protobuf', | ||
| '} // namespace google', | ||
| ), | ||
| }); | ||
| try { | ||
| const run = `${NS}::TextFormatSuite::Run`; | ||
| // (An alias of a type from outside the project still makes the alias.) | ||
| expect(targets(cg, run, 'instantiates')) | ||
| .toEqual([`${NS}::ConformanceTestSuite::ConformanceRequestSetting @0.9`, `${NS}::TextFormatSuite::Name @0.9`]); | ||
| expect(targets(cg, run, 'calls').filter((t) => t.includes('GetTestName'))) | ||
| .toEqual([`${NS}::ConformanceTestSuite::ConformanceRequestSetting::GetTestName @0.9`]); | ||
| } finally { | ||
| cg.close(); | ||
| } | ||
| }); | ||
| }); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.