mirror of
https://github.com/colbymchenry/codegraph.git
synced 2026-10-17 05:07:37 +08:00
fix(c): a C call, like redis's mstime(), reaches the function its own file can see, past another file's static copy (#2613)
A C call now links to the function of its name the call's own file can see:
its own, or one a header it includes defines, else one with external
linkage, the best placed of them (cppFunctionOverload's C branch). Before,
the name strategies took the function nearest the call; when that was
another source file's `static`, isVisibleAcrossFiles dropped it and the
call was left unlinked, so no call reached redis's `server.c` `mstime()`
or `ustime()`.
- A source file's `static` function is visible to a file that `#include`s
that source file: hiredis's async.c includes dict.c, redis's ae.c its
event-loop backend.
- The coordinator's `resolveImport` hook falls back to matchByFilePath for
a C/C++ `#include` the import resolver can't place, as resolveOneInner
already does, so cppIncluders sees includes only the end of their path
names: protobuf's `google/protobuf/descriptor.upb.h`, which two generated
copies end in, and jemalloc's `jemalloc/internal/*.h`. Standard headers
are left out: otherwise `#include <math.h>` makes jemalloc's
test/math.h visible to its own include through isIncludedCppTestSuite.
A/B on the cgstd kit against main 4a591aee: leveldb, fmt and rocksdb
identical; redis +194 calls linked and 115 retargeted (114 to hiredis
alloc.h's POSIX `static inline`s, 1 known miss: lua-cjson's fpconv_init to
an inline only `#ifdef USE_INTERNAL_FPCONV` builds use); protobuf 412 upb
calls retargeted from the unincluded php-upb.h amalgamation to the headers
the calling file includes.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -102,6 +102,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
|
||||
- In C++, a method defined by a macro of the same file is now indexed where the file invokes the macro, so calls to it link. protocolbuffers/protobuf's descriptor classes only declare most of their accessors and define them further down with macros like `PROTOBUF_DEFINE_ACCESSOR(FieldDescriptor, number, int)`, and its reflection defines `GetInt32`, `SetInt32` and the like by pasting names together in a macro. Those methods had no node at all, so calls like `field->number()`, `descriptor->full_name()` or `reflection->GetInt32(…)` linked to nothing. Calls through what such a method returns, like `field->containing_type()->full_name()`, now link too. A macro defined in another file, and a method declared but defined nowhere in the repository, are still not indexed. Re-index C++ projects after upgrading.
|
||||
- In C++, an explicit specialization of a function or member function template, like protocolbuffers/protobuf's `template <> bool WireFormatLite::ReadPrimitive<int32_t, WireFormatLite::TYPE_INT32>(…)` or facebook/rocksdb's `template <> int64_t Deserialize<int64_t>(…)`, is now indexed under its template's name, so search finds it and calls link to it. Before, it kept its template arguments in its name, and one with a `::` among them was named after the last part, like `TYPE_INT32>`. A call that writes template arguments, like `Deserialize<int64_t>(src, offset)`, links to the specialization written with the same ones, and otherwise to the template itself, as fmt's `num_bits<unsigned short>()` does; a call that writes none links to a specialization only when nothing else has that name. Re-index C++ projects after upgrading.
|
||||
- In C++, a call to an overloaded function outside a class now links to the overload its arguments fit, by how many it passes and the types they have where the call writes them, as a call to a method already did: protocolbuffers/protobuf's `ParseWithLimit(limit, data, result)` on an `int32_t result` reaches the overload taking an `int32_t&`, and facebook/rocksdb's `NewLRUCache(cache_opts)` the one taking an `LRUCacheOptions`. Before, the overload nearest the call was taken. Such a call is never linked to an overload only another source file can see, like another `.cc` file's `static` function: it now links to the one its own file sees, where before it often linked to nothing. A call to a name that both a function and a class have reaches the function, which hides the class, and a parameter named like the function it belongs to, like protobuf's `std::string u64(uint64_t u64)`, now has its own type. Re-index C++ projects after upgrading.
|
||||
- In C, a call now links to the function of that name its own file can see: its own, else one a header it includes defines, like a `static inline` helper, else one another source file defines without `static`. Before, the function nearest the call was taken, and when that was another source file's `static` function, which no other file can call, the call linked to nothing: in redis, the calls to `server.c`'s `mstime()` and `ustime()` from the rest of the server were lost to the `static` copies other files keep for themselves, and jemalloc's tests' calls to the `do_arena_create` their shared header defines to a neighbouring test's own. A call to a `static inline` function that several headers define reaches the copy in a header the file includes, so protocolbuffers/protobuf's upb code no longer links to the copies in its PHP extension's combined `php-upb.h`. A source file that `#include`s another, like redis's `ae.c` with its event-loop backend or hiredis's `async.c` with `dict.c`, now calls that file's `static` functions, and a header a file includes through an include directory, like jemalloc's `#include "jemalloc/internal/bit_util.h"`, counts as included. Re-index C projects after upgrading.
|
||||
- In C++, text inside a string literal is no longer taken for a variable's declaration. That covers the C code protocolbuffers/protobuf's upb generator writes in a raw string (`R"cc( … upb_MiniTableField field = $field_init$; … )cc"`) and log messages like facebook/rocksdb's `"… in WriteBatch handler; …"`. Before, a later call like `field.ctype()` on the function's own `upb::FieldDefPtr field` parameter, or `handler->PutCF(…)` on a `WriteBatch::Handler* handler`, was matched against the type that text seemed to name, so it linked to nothing or to another class's method. Re-index C++ projects after upgrading.
|
||||
- In C++, a call on an `auto` variable now links to the method of the class C++ finds for the type its initializer gives it, as a call on a variable declared with that type does. The type is read from a cast or smart pointer written out, like facebook/rocksdb's `auto iter = std::unique_ptr<Iterator>(…)` or `auto* mutex = static_cast<InstrumentedMutex*>(arg)`, from `new` or `std::make_unique` of a class, and from what a function or method is declared to return, like the `Iterator*` of `txn->GetIterator(…)`, read to the end of an initializer that goes on to the next line. Before, such a variable took the type of an earlier variable of the same name, or none: in RocksDB, `iter->SeekToFirst()` reached `MemTableRep::Iterator` and `mutex->Lock()` reached `port::Mutex`. A chained call like `cfd->current()->storage_info()` now looks up the class the inner call is declared to return the same way, through a smart pointer too, so protocolbuffers/protobuf's Java generator no longer reaches the C# generator's `EnumGenerator`. A type from outside the project no longer links to a class of the project that shares its name: protobuf's `status.ok()` on an `absl::Status` reached `upb::Status::ok`, and fmtlib/fmt's `os.flush()` on a `std::ostream&` reached fmt's own `ostream::flush`. The same holds for a variable declared in a macro body, or typed by what a function in another file returns, and for a class the index doesn't hold whose name only another class shares: protobuf's `_status.ok()` in its `RETURN_IF_ERROR` macro reached `upb::Status::ok`, and `range.options()` on its generated `DescriptorProto::ExtensionRange` reached `Descriptor::ExtensionRange::options`. A namespace a macro stands for is the project's under either name: facebook/rocksdb declares its classes in `namespace ROCKSDB_NAMESPACE` with `#define ROCKSDB_NAMESPACE rocksdb`, so `db->Put(…)` on a `rocksdb::DB*` links to RocksDB's own `DB::Put`. Re-index C++ projects after upgrading.
|
||||
- In C++, a call on a data member of the calling method's class, like `versions_->GetColumnFamilySet()` in facebook/rocksdb's `db_impl_write.cc`, now links to the method of the member's type wherever the class is declared. Before, only the call's own file and the header of the same name were read for the member, and RocksDB declares `DBImpl` in `db_impl.h` while its methods are spread over `db_impl_write.cc`, `db_impl_compaction_flush.cc`, `db_impl_open.cc` and more, so such calls were guessed by the member's name. A member declared below an inline method that uses it, one a base class declares, and, in a gtest `TEST_F` or `TEST_P` body, one of the test's fixture, like RocksDB's `db_`, are found the same way, and a member of a class nested in the caller's class is no longer taken for the caller's own. A call on a member of a standard-library type, like `std::atomic<bool> shutting_down_`, no longer links to a project method that only shares the method's name. Re-index C++ projects after upgrading.
|
||||
|
||||
@@ -0,0 +1,129 @@
|
||||
/**
|
||||
* A C call reaches the function of its name the call's own file sees: its
|
||||
* own, else one a header it includes defines, else one with external
|
||||
* linkage. jemalloc's tests call the `static inline do_arena_create` of the
|
||||
* arena_util.h they include; by directory, the name strategies picked a
|
||||
* neighbouring test's own `static do_arena_create`, which no other file can
|
||||
* call, and the call was left unlinked.
|
||||
*/
|
||||
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/index';
|
||||
import type { Node } from '../src/types';
|
||||
|
||||
const lines = (...l: string[]): string => [...l, ''].join('\n');
|
||||
|
||||
describe("a C call reaches the function its file sees, not another file's static", () => {
|
||||
let root: string;
|
||||
let cg: CodeGraph;
|
||||
|
||||
beforeAll(async () => {
|
||||
root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-c-static-rescue-'));
|
||||
const write = (file: string, text: string): void => {
|
||||
fs.mkdirSync(path.dirname(path.join(root, file)), { recursive: true });
|
||||
fs.writeFileSync(path.join(root, file), text);
|
||||
};
|
||||
// jemalloc: a header's static inline helper, and two tests with a static helper of its name.
|
||||
write('test/include/test/arena_util.h', lines(
|
||||
'static inline unsigned',
|
||||
'do_arena_create(long dirty_decay_ms, long muzzy_decay_ms) {',
|
||||
' return 0;',
|
||||
'}',
|
||||
));
|
||||
write('test/unit/arena_reset.c', lines(
|
||||
'static unsigned',
|
||||
'do_arena_create(void *hooks) {',
|
||||
' return 1;',
|
||||
'}',
|
||||
'void test_reset(void) { do_arena_create(0); }',
|
||||
));
|
||||
write('test/unit/arena_decay.c', lines(
|
||||
'#include "test/include/test/arena_util.h"',
|
||||
'void test_decay(void) { unsigned arena = do_arena_create(0, 0); }',
|
||||
));
|
||||
// A function declared in a header and defined in another source file.
|
||||
write('lib/helper.h', lines('int helper(int x);'));
|
||||
write('lib/helper.c', lines('#include "lib/helper.h"', 'int helper(int x) { return x; }'));
|
||||
write('app/other.c', lines('static int helper(int x) { return -x; }', 'int other(void) { return helper(1); }'));
|
||||
write('app/main.c', lines('#include "lib/helper.h"', 'int run(void) { return helper(2); }'));
|
||||
// Two builds' variants of one function: C has no overloads to choose among.
|
||||
write('src/posix/log.c', lines('void log_msg(const char* msg) {}'));
|
||||
write('src/win/log.c', lines('void log_msg(int level, const char* msg) {}'));
|
||||
write('src/posix/app.c', lines('void start(void) { log_msg(1, "up"); }'));
|
||||
// The header's function, over one nearer the call that no include brings in.
|
||||
write('include/timer.h', lines('static inline long now_ns(void) { return 0; }'));
|
||||
write('bench/unit/clock.c', lines('static long now_ns(void) { return 1; }'));
|
||||
write('bench/other/fake_clock.c', lines('long now_ns(void) { return 2; }'));
|
||||
write('bench/unit/latency.c', lines('#include "include/timer.h"', 'long elapsed(void) { return now_ns(); }'));
|
||||
// ...and over one with external linkage the call could reach, where the file's header defines one.
|
||||
write('include/ticks.h', lines('static inline long ticks(void) { return 0; }'));
|
||||
write('bench/unit/ticks.c', lines('long ticks(void) { return 1; }'));
|
||||
write('bench/unit/loop.c', lines('#include "include/ticks.h"', 'long spin(void) { return ticks(); }'));
|
||||
// hiredis: a source file that includes another one calls its static functions.
|
||||
write('deps/hiredis/dict.c', lines('static unsigned int dictGenHashFunction(const char* key) { return 0; }'));
|
||||
write('deps/hiredis/async.c', lines('#include "deps/hiredis/dict.c"', 'unsigned int callbackHash(const char* key) { return dictGenHashFunction(key); }'));
|
||||
write('server/dict.c', lines('unsigned int dictGenHashFunction(const char* key) { return 1; }'));
|
||||
// jemalloc's sources include `jemalloc/internal/…` headers that only the end of their path names.
|
||||
write('deps/jemalloc/include/jemalloc/internal/bit_util.h', lines('static inline unsigned pow2_ceil_u32(unsigned x) { return x; }'));
|
||||
write('deps/jemalloc/src/arena.c', lines('#include "jemalloc/internal/bit_util.h"', 'unsigned arena_size(unsigned n) { return pow2_ceil_u32(n); }'));
|
||||
write('deps/jemalloc/src/bitmap.c', lines('unsigned pow2_ceil_u32(unsigned x) { return x + 1; }'));
|
||||
// ...and its tests a `test/math.h`, which no `#include <math.h>` names.
|
||||
write('deps/jemalloc/test/include/test/math.h', lines('static inline double ln_gamma(double x) { return x; }'));
|
||||
write('src/sort.c', lines('#include <math.h>', 'int sort_nan(double a) { return a != a; }'));
|
||||
// A file's own definition, under another build's configuration than its header's.
|
||||
write('include/fast.h', lines('#ifndef HAVE_FAST', 'static inline int fast_add(int a, int b) { return a + b; }', '#endif'));
|
||||
write('src/fast.c', lines(
|
||||
'#include "include/fast.h"',
|
||||
'#ifdef HAVE_FAST',
|
||||
'static int fast_add(int a, int b) { return a - b; }',
|
||||
'#endif',
|
||||
'int use_fast(void) { return fast_add(1, 2); }',
|
||||
));
|
||||
cg = await CodeGraph.init(root, { index: true });
|
||||
}, 120_000);
|
||||
|
||||
afterAll(() => {
|
||||
cg?.close();
|
||||
if (root) fs.rmSync(root, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
/** `<file>:<line>` of what the function `name` calls. */
|
||||
const calls = (name: string): string[] => {
|
||||
const caller = cg.getNodesByKind('function').find((n: Node) => n.name === name)!;
|
||||
expect(caller, name).toBeDefined();
|
||||
return cg.getCallees(caller.id).filter((r) => r.edge.kind === 'calls').map((r) => `${r.node.filePath}:${r.node.startLine}`);
|
||||
};
|
||||
|
||||
it("the header's function, past a neighbouring file's static", () => {
|
||||
expect(calls('test_decay')).toEqual(['test/include/test/arena_util.h:1']);
|
||||
// The file's own static stays its own.
|
||||
expect(calls('test_reset')).toEqual(['test/unit/arena_reset.c:1']);
|
||||
});
|
||||
|
||||
it('a function another source file defines, when no include defines one', () => {
|
||||
expect(calls('run')).toEqual(['lib/helper.c:2']);
|
||||
expect(calls('other')).toEqual(['app/other.c:1']);
|
||||
});
|
||||
|
||||
it('the function picked by where it is, when the file sees it', () => {
|
||||
expect(calls('start')).toEqual(['src/posix/log.c:1']);
|
||||
expect(calls('use_fast')).toEqual(['src/fast.c:3']);
|
||||
});
|
||||
|
||||
it('the static function of a source file the file includes', () => {
|
||||
expect(calls('callbackHash')).toEqual(['deps/hiredis/dict.c:1']);
|
||||
});
|
||||
|
||||
it('one a header the file includes defines, over a nearer one', () => {
|
||||
expect(calls('elapsed')).toEqual(['include/timer.h:1']);
|
||||
expect(calls('spin')).toEqual(['include/ticks.h:1']);
|
||||
});
|
||||
|
||||
it('one a header defines that the file includes by the end of its path', () => {
|
||||
expect(calls('arena_size')).toEqual(['deps/jemalloc/include/jemalloc/internal/bit_util.h:1']);
|
||||
// A standard header is not the project's header of its name.
|
||||
expect(cg.getFileDependencies('src/sort.c')).not.toContain('deps/jemalloc/test/include/test/math.h');
|
||||
});
|
||||
});
|
||||
@@ -3,8 +3,10 @@
|
||||
* then the files that include those, out to the translation units.
|
||||
*
|
||||
* Read from the `import` node extraction mints for every `#include`, each
|
||||
* resolved by the import resolver exactly as the graph's include edge is, so
|
||||
* "includes" here is the include edge a reader sees. The name matcher asks it
|
||||
* resolved by the coordinator's import lookup as the graph's include edge is
|
||||
* (by the end of its path when nothing else places it, and never to a project
|
||||
* file for a standard header like `<math.h>`), so "includes" here is the
|
||||
* include edge a reader sees. The name matcher asks it
|
||||
* whether a C or C++ file named like a test is part of a reference's
|
||||
* translation unit: protobuf's conformance framework lives in
|
||||
* `conformance/conformance_test.h` and `test_runner.h`, which the suites and
|
||||
|
||||
+10
-2
@@ -21,7 +21,7 @@ import {
|
||||
isImportableKind,
|
||||
CPP_DEFINE_SIGNATURE,
|
||||
} from './types';
|
||||
import { isPythonSelfCall, matchJsStoreBindingCall, isUnresolvedJsMemberCall, matchObjectPathCall, thisScopeCaller, isVisibleAcrossFiles, matchReference, matchFunctionRef, matchDottedCallChain, matchScopedCallChain, matchMethodCall, sameLanguageFamily, crossesCodeBoundary, gateLanguageMatch, dumpNameMatcherProfile, clearNameMatcherMemos, isRustNameInScope, CASE_INSENSITIVE_LANGUAGES, isDartMemberRead, matchDartMemberRead, isDartChainLink, matchDartChainLink, isDartAnnotation, matchDartAnnotation, isStdMethodName, isGoUnknownQualified, isGoProjectLocalCall, GO_STDLIB_PACKAGES, isGoBareName, goTypePositionTarget, GO_TYPE_KINDS, matchGoAssertedCall, isGoAssertedLocal } from './name-matcher';
|
||||
import { isPythonSelfCall, matchJsStoreBindingCall, isUnresolvedJsMemberCall, matchObjectPathCall, thisScopeCaller, isVisibleAcrossFiles, matchReference, matchByFilePath, matchFunctionRef, matchDottedCallChain, matchScopedCallChain, matchMethodCall, sameLanguageFamily, crossesCodeBoundary, gateLanguageMatch, dumpNameMatcherProfile, clearNameMatcherMemos, isRustNameInScope, CASE_INSENSITIVE_LANGUAGES, isDartMemberRead, matchDartMemberRead, isDartChainLink, matchDartChainLink, isDartAnnotation, matchDartAnnotation, isStdMethodName, isGoUnknownQualified, isGoProjectLocalCall, GO_STDLIB_PACKAGES, isGoBareName, goTypePositionTarget, GO_TYPE_KINDS, matchGoAssertedCall, isGoAssertedLocal } from './name-matcher';
|
||||
import { isVisibleCppMacro, clearCppMacroVisibility } from './cpp-macro-visibility';
|
||||
import { isCppConstructorRef, matchCppConstructor } from './cpp-constructor';
|
||||
import { isCppSupertypeRef, matchCppSupertype, clearCppSupertypeMemos } from './cpp-supertypes';
|
||||
@@ -668,7 +668,15 @@ export class ReferenceResolver {
|
||||
*/
|
||||
private createContext(): ResolutionContext {
|
||||
return {
|
||||
resolveImport: (ref) => resolveViaImport(ref, this.context),
|
||||
// A C or C++ `#include` the import resolver can't place is linked by the
|
||||
// end of its path, as resolveOneInner's name matching does: protobuf's
|
||||
// `google/protobuf/descriptor.upb.h`, which two generated copies end in.
|
||||
// cppIncluders reads includes through this, so it sees the include edges
|
||||
// a reader sees. A standard header is never the project's: `<math.h>`
|
||||
// is not jemalloc's test/math.h, which only its includers may see.
|
||||
resolveImport: (ref) => resolveViaImport(ref, this.context) ??
|
||||
((ref.language === 'c' || ref.language === 'cpp') && ref.referenceKind === 'imports' &&
|
||||
!isExternalImport(ref.referenceName, ref.language, this.context) ? matchByFilePath(ref, this.context) : null),
|
||||
// A path alias makes an import the project's only when it maps the
|
||||
// import to a file the index holds. Matching its prefix is not enough:
|
||||
// cord-field's `"*": ["./typings/*"]` matches every package, and
|
||||
|
||||
@@ -3008,7 +3008,10 @@ export function isVisibleAcrossFiles(candidate: Node, ref: UnresolvedRef, contex
|
||||
(!isCsharpTypeVisible(candidate, ref, context) || !isCsharpNestedTypeInScope(candidate, ref, context))) return false;
|
||||
const lang = candidate.language as string;
|
||||
if (lang === 'c' || lang === 'cpp') {
|
||||
if (candidate.kind === 'function' && C_SOURCE_EXT.test(candidate.filePath) && isStaticCFunction(candidate, context)) return false;
|
||||
// A source file's `static` function, only for that file and one that
|
||||
// `#include`s it: hiredis's async.c includes dict.c.
|
||||
if (candidate.kind === 'function' && C_SOURCE_EXT.test(candidate.filePath) && isStaticCFunction(candidate, context) &&
|
||||
!cppIncluders(candidate.filePath, context).has(ref.filePath)) return false;
|
||||
return !isCppTranslationUnitLocal(candidate, ref, context);
|
||||
}
|
||||
if (lang === 'go') {
|
||||
@@ -11211,14 +11214,23 @@ function cppArgumentTypeChoice(tied: Node[], name: string, args: string[], ref:
|
||||
* where it is; protobuf's `ParseWithLimit(limit, data, result)` on an
|
||||
* `int32_t result` is the overload taking an `int32_t&`, not the
|
||||
* `int64_t&` one nearer the call. A function hides a class of its name, so
|
||||
* a call is to the function when `chosen` is that class. `chosen` itself
|
||||
* for anything but a C++ call, or when no function is in sight.
|
||||
* a call is to the function when `chosen` is that class. C has no
|
||||
* overloads: a C call reaches the function of the name its file sees — its
|
||||
* own or one a header it includes defines, else one with external linkage —
|
||||
* the best placed of them (findBestMatch). jemalloc's tests call the
|
||||
* `static inline do_arena_create` of the arena_util.h they include, not a
|
||||
* neighbouring test's own `static do_arena_create`. `chosen` itself for
|
||||
* anything but a C or C++ call, or when no function is in sight.
|
||||
*/
|
||||
function cppFunctionOverload(chosen: Node, candidates: Node[], ref: UnresolvedRef, context: ResolutionContext): Node {
|
||||
if (ref.language !== 'cpp' || ref.referenceKind !== 'calls') return chosen;
|
||||
if ((ref.language !== 'cpp' && ref.language !== 'c') || ref.referenceKind !== 'calls') return chosen;
|
||||
const overloads = candidates.filter((n) => n.kind === 'function' && n.qualifiedName === chosen.qualifiedName &&
|
||||
isVisibleAcrossFiles(n, ref, context));
|
||||
if (overloads.length === 0) return chosen;
|
||||
if (ref.language === 'c') {
|
||||
const own = overloads.filter((n) => n.filePath === ref.filePath || cppIncluders(n.filePath, context).has(ref.filePath));
|
||||
return findBestMatch(ref, own.length > 0 ? own : overloads, context) ?? chosen;
|
||||
}
|
||||
return cppOverloadFor(overloads.includes(chosen) ? [chosen, ...overloads.filter((n) => n !== chosen)] : overloads, chosen.name, ref, context);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user