From c4683d1b63ad55eecdc20bc2d64db11632f7a903 Mon Sep 17 00:00:00 2001 From: Colby McHenry Date: Sat, 31 Oct 2026 22:09:40 -0500 Subject: [PATCH] fix(c,cpp): a member whose type a macro renames links what the macro names redis-cli.c and redis-benchmark.c include hiredis's sdscompat.h, which has `#define sds hisds`. Their 24 `sds` members were linked to src/sds.h's namesake typedef, a false edge new in PR 3144: the parser sees `sds`, and a macro with no parameters has no node. matchCppDeclaredType now asks what the name is renamed to where it is written. Headers are read once per resolver, by a search for `define`, into an index by macro name, so a name nothing renames costs one lookup. A rename counts as the preprocessor applies it: the translation unit from the asking file must have certainly defined the macro by that line (isCppMacroDefinedAt, the existing include walk following one object-like macro). So not above the `#include` or the `#define`, not under a flag no file sets, and not when two arms disagree. A new name that is no project type leaves the name as written. The scan stops at 16 MB of headers per resolver: betaflight has 109 MB, nearly all vendor register maps. The same rule corrects two more that nobody had read: - redis's core sets REDISMODULE_CORE, so `RedisModuleString` is `robj` in module.c, 19 edges. A module that includes the same header keeps the typedef, as does the header read on its own. - jq's `#define YYLTYPE location`: bison's struct is compiled out, 3 edges. Validation against main 4499d007: - Tests: 5 of 7 fail on main, all pass. 9 mutations each fail a test. - redis 43 edges moved, 24 to `hisds` and 19 to `robj`, each checked against the includes and the flag; nothing lost. jq 3 moved, right. - rocksdb, leveldb, glfw, betaflight, gin, flask: nothing from this commit. - No extraction change. Cost: a resolver's scan is 26 ms on redis and 81 ms on rocksdb, once per worker; 49 ms of a redis index's profile in all. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 2 +- __tests__/c-macro-renames.test.ts | 175 +++++++++++++++++++++++++ src/resolution/c-macro-renames.ts | 105 +++++++++++++++ src/resolution/cpp-macro-visibility.ts | 34 ++++- src/resolution/cpp-supertypes.ts | 6 +- src/resolution/index.ts | 2 + 6 files changed, 318 insertions(+), 6 deletions(-) create mode 100644 __tests__/c-macro-renames.test.ts create mode 100644 src/resolution/c-macro-renames.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index aa3abf420a..c7d5de3248 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -253,7 +253,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In a project that uses Vue, an import written through an `@/` or `~/` alias, like `import { Button } from '@/components'`, now links the file it imports, as it does in any other project, so `codegraph sync` and the file watcher update the files that import a module when that module, or a barrel they import through, is added or deleted. Before, Vue support linked such an import to the first declaration or the component in that file, which sync does not follow, and an import through a `tsconfig` alias that starts like one of Nuxt's built-in modules, like `#app/where`, linked the importing file to itself. Re-index Vue projects after upgrading. - `codegraph sync` and the file watcher now move an import written without an extension, like `import { sizeOf } from './button'`, to a file added beside the one it found before when the new file comes first, such as `button.ts` next to `button.vue`. Before, the import and the names it binds stayed linked to `button.vue` until the importing file changed or the project was re-indexed. JavaScript, TypeScript, Vue, Svelte and Astro. - 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 struct, class or union now links to the types its members are declared with, whether held by value, by pointer, in an array, through a function pointer or inside a template like `std::vector`, and a `typedef` or `using` alias links to the type it names, so impact on a type reaches every struct that holds one, like redis's `redisServer` for `pause_event`. A member's type is the one C++ name lookup finds from the class, so a builtin, a macro or a function of the same name links nothing. Re-index C and C++ projects after upgrading. +- In C and C++, a struct, class or union now links to the types its members are declared with, whether held by value, by pointer, in an array, through a function pointer or inside a template like `std::vector`, and a `typedef` or `using` alias links to the type it names, so impact on a type reaches every struct that holds one, like redis's `redisServer` for `pause_event`. A member's type is the one C++ name lookup finds from the class, so a builtin, a macro or a function of the same name links nothing, and a type a macro renames is what the macro names where the file sees it, like hiredis's `#define sds hisds` in redis-cli.c. Re-index C and 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. diff --git a/__tests__/c-macro-renames.test.ts b/__tests__/c-macro-renames.test.ts new file mode 100644 index 0000000000..47f89aed48 --- /dev/null +++ b/__tests__/c-macro-renames.test.ts @@ -0,0 +1,175 @@ +/** + * An object-like macro can rename a type. hiredis's sdscompat.h has `#define + * sds hisds`, so an `sds` member in redis-cli.c, which includes it, is a + * `hisds`, and src/sds.h's namesake typedef is one that file never compiles. + * The parser sees `sds`, and macros with no parameters get no node, so a + * member's type went to whatever else bears the name. + * + * The rename counts as the preprocessor applies it: from where a translation + * unit has certainly defined the macro. Not above the `#include` or the + * `#define`, not under a flag no file sets, and not when the new name is no + * type of the project. + */ +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/index'; + +describe('a C or C++ type a macro renames', () => { + let root = ''; + let cg: CodeGraph | undefined; + + afterEach(() => { + cg?.destroy(); + cg = undefined; + if (root) fs.rmSync(root, { recursive: true, force: true }); + root = ''; + }); + + /** Index `files` and return what a type references, as `kind name @file`. */ + async function index(files: Record): Promise<(holder: string) => string[]> { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'cg-c-renames-')); + 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); + } + const graph = (cg = await CodeGraph.init(root, { index: true })); + return (holder) => { + const node = graph.getNodesByName(holder).find((n) => n.kind === 'struct' || n.kind === 'type_alias' || n.kind === 'class'); + expect(node, holder).toBeDefined(); + return graph + .getOutgoingEdges(node!.id) + .filter((e) => e.kind === 'references') + .map((e) => graph.getNode(e.target)!) + .map((n) => `${n.kind} ${n.name} @${n.filePath}`) + .sort(); + }; + } + + const library = { + 'deps/lib/str.h': 'typedef char *histr;\n', + 'deps/lib/compat.h': [ + '#ifndef COMPAT_H', + '#define COMPAT_H', + '#define str histr /* the library\'s */', + '#endif', + '', + ].join('\n'), + 'src/str.h': 'typedef char *str;\n', + }; + const HISTR = 'type_alias histr @deps/lib/str.h'; + const STR = 'type_alias str @src/str.h'; + + it('links what the macro names, in a file that includes it', async () => { + const referenced = await index({ + ...library, + 'src/client.c': [ + '#include ', + '#include ', + 'struct client {', + ' str name;', + ' str *names;', + '};', + 'typedef str client_name;', + '', + ].join('\n'), + 'src/server.c': '#include "str.h"\nstruct server { str name; };\n', + }); + expect(referenced('client')).toEqual([HISTR, HISTR]); + expect(referenced('client_name')).toEqual([HISTR]); + expect(referenced('server')).toEqual([STR]); + }); + + it('reaches through an include, and holds for C++', async () => { + const referenced = await index({ + ...library, + 'src/indirect.h': '#include \n', + 'src/third.c': '#include "indirect.h"\nstruct third { str name; };\n', + 'src/fourth.cpp': '#include "indirect.h"\nclass fourth { str name; };\n', + }); + expect(referenced('third')).toEqual([HISTR]); + expect(referenced('fourth')).toEqual([HISTR]); + }); + + it('a file can rename a type for itself, from its `#define` on', async () => { + const referenced = await index({ + ...library, + 'src/own.c': [ + '#include "str.h"', + 'struct before { str name; };', + '#define str histr', + 'struct after { str name; };', + '', + ].join('\n'), + }); + expect(referenced('before')).toEqual([STR]); + expect(referenced('after')).toEqual([HISTR]); + }); + + it('starts at the `#include`, as the preprocessor does', async () => { + const referenced = await index({ + ...library, + 'src/later.c': [ + '#include "str.h"', + 'struct before { str name; };', + '#include ', + 'struct after { str name; };', + '', + ].join('\n'), + }); + expect(referenced('before')).toEqual([STR]); + expect(referenced('after')).toEqual([HISTR]); + }); + + it('holds only where the flag around it is set', async () => { + // redis's redismodule.h: `#define RedisModuleString robj` under REDISMODULE_CORE, + // which server.h sets and a module does not. + const referenced = await index({ + 'src/object.h': 'typedef struct redisObject { int x; } robj;\n', + 'src/redismodule.h': [ + 'typedef struct RedisModuleString RedisModuleString;', + '#if defined REDISMODULE_CORE', + '#include "object.h"', + '#define RedisModuleString robj', + '#endif', + 'struct header_holder { RedisModuleString *s; };', + '', + ].join('\n'), + 'src/server.h': '#define REDISMODULE_CORE 1\n#include "redismodule.h"\n', + 'src/core.c': '#include "server.h"\nstruct core_holder { RedisModuleString *s; };\n', + 'modules/module.c': '#include "../src/redismodule.h"\nstruct module_holder { RedisModuleString *s; };\n', + }); + expect(referenced('core_holder')).toEqual(['struct robj @src/object.h']); + expect(referenced('module_holder')).toEqual(['type_alias RedisModuleString @src/redismodule.h']); + expect(referenced('header_holder')).toEqual(['type_alias RedisModuleString @src/redismodule.h']); + }); + + it('leaves the name as written when the macro names no type of the project, or two', async () => { + const referenced = await index({ + ...library, + 'src/outside.c': '#include "str.h"\n#define str outside_type\nstruct outside { str name; };\n', + 'src/both.h': [ + '#ifdef ONE', + '#define str histr', + '#else', + '#define str histr_other', + '#endif', + '', + ].join('\n'), + 'deps/lib/other.h': 'typedef char *histr_other;\n', + 'src/both.c': '#define ONE\n#include "both.h"\n#include "str.h"\nstruct both { str name; };\n', + }); + expect(referenced('outside')).toEqual([STR]); + expect(referenced('both')).toEqual([STR]); + }); + + it('never reads a macro in a block comment', async () => { + const referenced = await index({ + ...library, + 'src/commented.h': '/*\n#define str histr\n*/\n', + 'src/commented.c': '#include "commented.h"\n#include "str.h"\nstruct commented { str name; };\n', + }); + expect(referenced('commented')).toEqual([STR]); + }); +}); diff --git a/src/resolution/c-macro-renames.ts b/src/resolution/c-macro-renames.ts new file mode 100644 index 0000000000..07da5d88c9 --- /dev/null +++ b/src/resolution/c-macro-renames.ts @@ -0,0 +1,105 @@ +/** + * What an object-like macro renames a C or C++ name to, in the files it + * reaches: hiredis's sdscompat.h has `#define sds hisds`, so a `sds` member in + * redis-cli.c, which includes it, is a `hisds`, whatever a namesake in + * src/sds.h says. The preprocessor sees this and the parser doesn't. + * + * A macro with no parameters gets no node, so only the source says what one + * does. Headers are read once per resolver, in one pass over their `define`s, + * into an index by macro name: a name no header renames costs one lookup, + * however many files write it. A vendor's register maps can run to hundreds of + * megabytes, so the pass stops at a budget, past which a name is left as written. + * A source file is read when a name in it is asked about. A rename counts + * where the translation unit from the asking file has certainly defined + * it by that line (isCppMacroDefinedAt): a macro under a flag no file sets, or + * written further down, renames nothing. Two renames of one name that disagree + * (`#ifdef` arms) say nothing either. + */ +import type { ResolutionContext, UnresolvedRef } from './types'; +import { cppIncluders } from './cpp-includers'; +import { directiveLines, isCppMacroDefinedAt } from './cpp-macro-visibility'; + +interface Memo { + /** Macro name → the files with a `#define` of it to one name. */ + byName: Map; + /** The source files read so far. */ + sources: Set; + /** `file\0name` → what the name is renamed to there (null: nothing). */ + answers: Map; +} + +const MEMO = new WeakMap(); + +export function clearCMacroRenames(context: ResolutionContext): void { + MEMO.delete(context); +} + +const HEADER_FILE = /\.(?:h|hh|hpp|hxx)$/i; +/** Characters of headers one resolver reads for renames: redis has 2 MB, rocksdb 9 MB, betaflight 109 MB. */ +const HEADER_SCAN_BUDGET = 16_000_000; +/** `#define sds hisds`, with a comment after it or none. */ +const RENAME_LINE = /^[ \t]*#[ \t]*define[ \t]+([A-Za-z_]\w*)[ \t]+([A-Za-z_]\w*)[ \t]*(?:\/[/*].*)?\r?$/; + +/** Add the renames `source` writes to the index, by a search for `define`: most of a file is no directive. */ +function index(file: string, source: string, byName: Map): void { + for (let at = source.indexOf('define'); at >= 0; at = source.indexOf('define', at + 6)) { + const next = source.charCodeAt(at + 6); + if (next !== 32 && next !== 9) continue; // `defined(X)` + const end = source.indexOf('\n', at); + const line = source.slice(source.lastIndexOf('\n', at) + 1, end < 0 ? source.length : end); + const m = RENAME_LINE.exec(line); + if (!m) continue; + const files = byName.get(m[1]!); + if (!files) byName.set(m[1]!, [file]); + else if (files[files.length - 1] !== file) files.push(file); + } +} + +function memoFor(context: ResolutionContext): Memo { + let memo = MEMO.get(context); + if (memo) return memo; + const byName = new Map(); + let budget = HEADER_SCAN_BUDGET; + for (const file of context.getAllFiles()) { + if (budget <= 0) break; + if (!HEADER_FILE.test(file)) continue; + const source = context.readFile(file); + if (!source) continue; + index(file, source, byName); + budget -= source.length; + } + MEMO.set(context, (memo = { byName, sources: new Set(), answers: new Map() })); + return memo; +} + +/** The one name `name` is renamed to at `line` of `file`, by the file or by what it includes. */ +export function cMacroRename( + name: string, + file: string, + line: number, + language: UnresolvedRef['language'], + context: ResolutionContext +): string | null { + const memo = memoFor(context); + if (!HEADER_FILE.test(file) && !memo.sources.has(file)) { + memo.sources.add(file); + index(file, context.readFile(file) ?? '', memo.byName); + } + const files = memo.byName.get(name); + if (!files) return null; + const key = `${file}\0${name}`; + let renamed = memo.answers.get(key); + if (renamed === undefined) { + const renames = new Set(); + for (const candidate of files) { + if (candidate !== file && !cppIncluders(candidate, context).has(file)) continue; + for (const directive of directiveLines(context.readFile(candidate) ?? '')) { + const m = RENAME_LINE.exec(directive); + if (m?.[1] === name) renames.add(m[2]!); + } + } + renamed = renames.size === 1 ? [...renames][0]! : null; + memo.answers.set(key, renamed); + } + return renamed !== null && isCppMacroDefinedAt(name, file, line, language, context) ? renamed : null; +} diff --git a/src/resolution/cpp-macro-visibility.ts b/src/resolution/cpp-macro-visibility.ts index 64032e9735..9b02cdcb47 100644 --- a/src/resolution/cpp-macro-visibility.ts +++ b/src/resolution/cpp-macro-visibility.ts @@ -44,6 +44,8 @@ type Cache = { byBasename: Map | null; /** Per root file: macro name → define/undef events in root-file line order. */ roots: Map; + /** Per root file and macro: the timeline of that macro alone (isCppMacroDefinedAt). */ + watched: Map; }; const memo = new WeakMap(); @@ -95,10 +97,32 @@ export function isVisibleCppMacro(ref: UnresolvedRef, context: ResolutionContext return before.length > 0 && before[before.length - 1]!.defined === true; } +/** + * Is `name` certainly defined as a macro at `line` of `file`, read as a + * translation unit? A `#define` under a flag no file sets is not: redis's + * `#define RedisModuleString robj` holds in the core, which sets + * REDISMODULE_CORE, and not in a module that includes the same header. + */ +export function isCppMacroDefinedAt( + name: string, + file: string, + line: number, + language: UnresolvedRef['language'], + context: ResolutionContext +): boolean { + const cache = cacheFor(context); + const key = `${language}\0${file}\0${name}`; + let timeline = cache.watched.get(key); + if (!timeline) cache.watched.set(key, (timeline = walkTranslationUnit(file, language, context, cache, name))); + if (line >= timeline.cutoff) return false; + const before = (timeline.events.get(name) ?? []).filter((e) => e.line <= line); + return before.length > 0 && before[before.length - 1]!.defined === true; +} + function cacheFor(context: ResolutionContext): Cache { let cache = memo.get(context); if (!cache) { - cache = { summaries: new Map(), includes: new Map(), byBasename: null, roots: new Map() }; + cache = { summaries: new Map(), includes: new Map(), byBasename: null, roots: new Map(), watched: new Map() }; memo.set(context, cache); } return cache; @@ -261,7 +285,9 @@ function walkTranslationUnit( rootFile: string, language: UnresolvedRef['language'], context: ResolutionContext, - cache: Cache + cache: Cache, + // Follow this macro alone, and whether it is defined, not whether it is function-like. + watched?: string ): Timeline { const timeline = new Map(); const definitions = new Map(); @@ -362,9 +388,9 @@ function walkTranslationUnit( macroNames.add(ev.name); version++; } - if (macroNames.has(ev.name)) { + if (watched === undefined ? macroNames.has(ev.name) : ev.name === watched) { const events = timeline.get(ev.name) ?? []; - events.push({ line, defined: now }); + events.push({ line, defined: watched === undefined ? now : next.defined }); timeline.set(ev.name, events); } } diff --git a/src/resolution/cpp-supertypes.ts b/src/resolution/cpp-supertypes.ts index 03f00dc597..745d9b3d51 100644 --- a/src/resolution/cpp-supertypes.ts +++ b/src/resolution/cpp-supertypes.ts @@ -63,6 +63,7 @@ import { stripCppTemplateArguments, type CppNamedType, } from './cpp-type-aliases'; +import { cMacroRename } from './c-macro-renames'; import { cppIncluders } from './cpp-includers'; import { cppMacroNamespaceFrames, cppNamespaceAliases } from './cpp-namespaces'; import { stripCommentsForRegex } from './strip-comments'; @@ -172,7 +173,10 @@ export function matchCppDeclaredType(ref: UnresolvedRef, context: ResolutionCont parameters: cppTemplateParameters(holder, context), declared: true, }; - const found = cppClassWritten(ref.referenceName, site, ref, context); + // A macro the file sees can rename the type (`#define sds hisds`), and a type + // the new name doesn't reach is left as written. + const renamed = cMacroRename(ref.referenceName, site.file, ref.line, ref.language, context); + const found = (renamed && cppClassWritten(renamed, site, ref, context)) || cppClassWritten(ref.referenceName, site, ref, context); const node = found === undefined ? definedByIncluder(ref.referenceName, site, context) : found?.node; if (!node || node.id === holder.id) return null; return found?.exact diff --git a/src/resolution/index.ts b/src/resolution/index.ts index 73ab65f209..c8c24065f7 100644 --- a/src/resolution/index.ts +++ b/src/resolution/index.ts @@ -36,6 +36,7 @@ import { gateTypeParameter, clearTypeParameterMemos } from './type-parameters'; import { gateDartLocal, clearDartLocalScopeMemos } from './dart-local-scope'; import { clearCppTypeAliasMemos } from './cpp-type-aliases'; import { clearCppIncluderMemos } from './cpp-includers'; +import { clearCMacroRenames } from './c-macro-renames'; import { clearCFeatureVariantMemos } from './c-feature-variants'; import { clearAmbientModuleMemos, ambientModuleDeclarations } from './ambient-modules'; import { matchShopifyThemeFile } from './shopify-themes'; @@ -692,6 +693,7 @@ export class ReferenceResolver { clearCppTypeAliasMemos(this.context); clearCppSupertypeMemos(this.context); clearCppIncluderMemos(this.context); + clearCMacroRenames(this.context); clearCFeatureVariantMemos(this.context); clearAmbientModuleMemos(this.context); clearDeclaredDependencies(this.context);