Skip to content

fix(cpp): a class keeps its members past attribute macros the parser can't read - #2449

Merged
colbymchenry merged 4 commits into
mainfrom
claude/funny-pasteur-044c66
Oct 7, 2026
Merged

colbymchenry merged 4 commits into
mainfrom
claude/funny-pasteur-044c66

Conversation

@colbymchenry

@colbymchenry colbymchenry commented Oct 7, 2026 •

Copy link
Copy Markdown
Owner

Summary

tree-sitter-cpp has no preprocessor, so an attribute macro where it expects a name or a type misparses the declaration around it, and often the class around that. The C++ preParse already blanked one export macro before a class name, a curated list of inline macros before a return type, *_API prefixes and lone macro lines. It missed four shapes, all common in Google-style C++:

  1. Several macros, or one with arguments, between class and the name. Every generated class in protocolbuffers/protobuf's checked-in .pb.h files starts class PROTOBUF_EXPORT PROTOBUF_FUTURE_ADD_EARLY_WARN_UNUSED Any final : public ::google::protobuf::Message. repeated_field.h splits two macros over two lines, and rocksdb has struct ALIGN_AS(64U) HandleImpl.
  2. A macro before a template specialization's name, as in class PROTOBUF_DECLSPEC_EMPTY_BASES RepeatedFieldProxyWithSet<…> {.
  3. Macros on members:
    • in front: PROTOBUF_FUTURE_ADD_EARLY_NODISCARD absl::string_view name() const (250 members of descriptor.h), GTEST_API_ std::string …;
    • between a pointer and its name: const Descriptor* PROTOBUF_NONNULL descriptor(), Any* PROTOBUF_RESTRICT PROTOBUF_NONNULL other;
    • after the parameter list: unknown_fields() const ABSL_ATTRIBUTE_LIFETIME_BOUND {, ~Any() PROTOBUF_FINAL;, leveldb's void Wait() LOCKS_EXCLUDED(mu_) {;
    • after a declared name: int count_ GUARDED_BY(mu_);, protobuf's Any_globals_ PROTOBUF_MESSAGE_GLOBALS_SECTION(.data.rel.ro);.
  4. A lone macro line with a comment under it. {fmt}'s FMT_BEGIN_EXPORT above // A generic formatting context … and template <…> class generic_context {. blankLoneMacroLines required the next line to start a declaration, so the comment rejected it.

The effects on the index:

  • protobuf. Each generated message class was missing, and its members (GetDescriptor, GetReflection, descriptor, unknown_fields, …) were indexed as namespace-level functions. Out-of-line definitions carried the macro in their name (google::protobuf::PROTOBUF_NONNULL Any::mutable_type_url). A definition whose parameter is annotated (void Any::InternalSwap(Any* PROTOBUF_NONNULL other) { … }) parsed as a variable and was not indexed at all.
  • leveldb. Annotated methods were indexed under the annotation's name: AtomicCounter::LOCKS_EXCLUDED instead of IncrementBy, port::Mutex::EXCLUSIVE_LOCK_FUNCTION instead of Lock. So mutex_.Lock() linked to nothing.

The rules (src/extraction/languages/c-cpp.ts)

Every pass replaces macro tokens with spaces, so offsets, lines and columns stay as they were. Each pass matches on a masked copy of the source:

  • comments are spaces;
  • string and char literals are placeholder characters, so a literal is still a token;
  • preprocessor directives, with their \ continuation lines, are spaces.

Only the macro tokens are blanked, never the text between them.

  • blankCppExportMacros (extended). It now blanks a run of ALL-CAPS macros, each optionally with arguments, between class/struct and the name. The name may be qualified and may take template arguments, which must balance before [final] : or {. Declarations ending in ; or = are still left alone, and so are class GTEST_1_TUPLE_(T) { (a macro that is the name) and a comment.

  • blankCppPointerAnnotationMacros. An ALL-CAPS macro with an underscore right after * or &, followed by the declared name or (after *) another *.

    • C++ never puts two names in a row there.
    • A product or bit test is followed by an operator or a closer: a * MAX_LEN + 1, flags & FOO_BIT).
    • A * after ), ] or a number is a product too (sizeof(int) * CHAR_BIT * 3).
    • After &, only a name counts (flags & FOO_BIT && ok stays).
  • blankCppTrailingAttributeMacros. One or more macros, optionally with arguments, after a parameter list and before {, ;, =, a constructor's :, ->, override, final, const or noexcept.

    • The ) must close a parameter list: the ( it matches must follow a function name (name(, ~Name(, operator==(, operator()().
    • Not a keyword: if, while, return, and constexpr, as in if constexpr (…).
    • Not a cast, and not an ALL-CAPS macro call such as FMT_PRAGMA_CLANG(…) above TEST(…) {.
    • No blank line in between.
  • blankCppDeclaratorAttributeMacros. An ALL-CAPS macro with an underscore, optionally with arguments, after ] or after a declared name and before ;, =, , or ).

    • The name must contain a lowercase letter, and a type must stand before it: a type name, a type keyword, *, &, >, or const after a pointer.
    • So a variable named in capitals (Foo DEFAULT_OPTIONS;, const Foo DEFAULT_OPTIONS = …;), a typedef (typedef Foo BAR_T;), return x FOO; and f(a, b FOO) stay.
  • blankCppLeadingAttributeMacros. A run of ALL-CAPS macros with an underscore, at the start of a line or after [[…]], followed by something a type can't be followed by:

    • a declaration keyword (static, inline, virtual, void, struct, template, return, [[, …);
    • an out-of-line constructor (Any::Impl_::Impl_();
    • or a whole declaration of its own, a type and then a name (Type name(, ns::Type<T>* name;).

    A type in capitals is followed by the name alone (DWORD_PTR value = 0;, HANDLE_T Open(…), GNU-style RESULT_T\nFunc(…)), so it never matches. const after the token (SIZE_T const n) only counts with a type and a name after it.

  • blankLoneMacroLines (shared with C) skips comments when it looks for the next line of code.

All new passes are C++ only, except the lone-macro change.

Validation

Tests

  • __tests__/cpp-attribute-macros.test.ts, 19 tests:
    • each pass on the real shapes;
    • negative controls (products, bit tests, casts, statements, if constexpr, directive lines, macro calls, capitals-named variables and types, string concatenations with macros, comments);
    • offsets through the whole preParse;
    • an indexed project with protobuf-, leveldb- and fmt-shaped files (classes, members as methods, extends, no phantoms).
  • __tests__/kernel-ccpp-parity.test.ts: a protobuf/leveldb-shaped header (LF and CRLF) now parses clean, goes through the kernel instead of deferring, and matches the wasm walker.
  • Red phase: on main's c-cpp.ts all 21 new tests fail.
  • fix(cpp): a class keeps its namespaces and enclosing class past code the parser misreads #2426's cpp-brace-scopes.test.ts. Its fixtures misparsed only through the two macros this PR now blanks (PROTOBUF_FUTURE_ADD_EARLY_NODISCARD, ALIGN_AS(64U)), so its "every fixture still misparses" check failed. The fixtures now use macros the preParse doesn't know, NODISCARD and cacheline_aligned(64U). Every other expectation in that file is unchanged.
  • tsc is clean.
  • Full suite: 6,687 tests. The parallel run failed 39 tests in 16 git, sync, MCP-daemon and WAL files on this loaded box. Rerun serially with long timeouts, all 971 tests in those files passed.
  • Cost. The new passes add about 0.7–0.9 s of preParse time per 25–35 MB of C++: protobuf 1.7 → 2.5 s, rocksdb 1.8 → 2.5 s. They share one mask per file, and the output is byte-identical to building a fresh mask per pass. (Whole-index time was not benchmarked.)

Parse errors (C++ files, wasm grammar after the preParse)

repo files with errors error sites
protocolbuffers/protobuf 223 → 172 (of 941) 7,237 → 1,000
google/leveldb 19 → 8 (of 128) 104 → 28
fmtlib/fmt 31 → 30 (of 72) 620 → 442
facebook/rocksdb 210 → 201 (of 1,372) 1,520 → 1,412
redis (C control, lone-macro change only) 186 → 176 files
  • No file gained an error in any of these repos.
  • Every blanked line was listed (base vs fix preParse output) and read: only macro tokens, plus the arguments of macro calls. Only one change was in a file that already parsed clean. In protobuf io/coded_stream.cc, a lone PROTOBUF_ALWAYS_INLINE let the parser read PROTOBUF_ALWAYS_INLINE::std::pair<…> as one qualified type.

Kernel parity

The preParse is hoisted to the kernel route point, so a file that stops erroring now goes through the native kernel.

  • scripts/kernel-parity.mjs --lang c,cpp --max-deferral 0.5 gives 0 files with diffs on all five repos. Deferrals, main → this branch: leveldb 20 → 9, fmt 32 → 31, protobuf 343 → 292, rocksdb 223 → 214, redis 186 → 176.
  • Under fix(cpp): a class keeps its namespaces and enclosing class past code the parser misreads #2426's error-extract hatch (CODEGRAPH_KERNEL_CCPP_ERROR_EXTRACT=1), files with diffs go from 4 → 2, 9 → 5, 56 → 10 and 11 → 8. This branch's set is a strict subset of main's: no new divergence.

A/B on main ed199e60 (with #2426 and #2413)

Each repo was fully indexed with codegraph init -y on main and on this branch, both built from the same tree with the kernel rebuilt.

Nodes (C/C++):

repo classes methods functions phantom nodes gone
protobuf 908 → 968 12,131 → 13,648 18,802 → 18,116 79
fmt 394 → 403 2,688 → 2,775 1,999 → 1,921 68
rocksdb 2,718 → 2,723 24,531 → 24,566 12,617 → 12,591 33
leveldb 149 → 149 1,307 → 1,306 511 → 511 15
redis — — — 0 (one typedef added)
  • protobuf gains 60 classes: every generated message class, plus google::protobuf::Arena, the gtest/plugin classes and RepeatedField's specializations.
    • 595 members move from namespace-level functions to methods of their class.
    • 458 method names are corrected. 341 lose a macro or keyword glued into them (google::protobuf::PROTOBUF_NONNULL Any::mutable_type_url, google::protobuf::void Any::SharedDtor). 117 gain the class they are in (Impl_::Impl_ → Any::Impl_::Impl_, Map::operator++ → Map::const_iterator::operator++).
    • 40 Impl_ structs and 135 type aliases move into their class.
  • leveldb. All 15 phantoms (…::LOCKS_EXCLUDED, port::Mutex::EXCLUSIVE_LOCK_FUNCTION, …) become the 14 real methods. The 15th phantom, MutexLock::EXCLUSIVE_LOCK_FUNCTION, had held the body of the existing MutexLock::MutexLock, which now spans it.
  • Every node that disappears was checked: a node named after a macro (ABSL_ATTRIBUTE_LIFETIME_BOUND, PROTOBUF_CONSTINIT, GTEST_LOCK_EXCLUDED_, …), a field misread as a namespace-level variable, or a misparse (fmt's "method" named after the whole text of class basic_string_view {…).

Edges, by call site (kind, file, line, column, ref name, so a source that became a method is still the same site):

repo removed added retargeted same edge, new metadata
protobuf 790 2,286 1,147 531
rocksdb 80 194 16 70
fmt 22 198 67 40
leveldb 7 68 9 3
redis 0 1 0 0

What they are:

protobuf, removed (790).

  • About 718 were wrong edges:
    • 228 "calls" were declarations or definitions the misparse read as calls (Any::Any(::google::protobuf::Arena* PROTOBUF_NULLABLE arena)).
    • 117 pointed every generated class's Super_(…) at one class's alias.
    • 73 sent _impl_.methods_.InternalSwap(…) to InternalHelper.
    • 72 sent default_instance().GetMetadata() in every class to Any's.
    • 63 linked ObjC [self class] to a phantom google::protobuf::class.
    • 73 were name-only guesses on std receivers (line.find('#'), v.emplace_back(…)), dropped because the name is no longer unique.
    • The rest are #include <set> linking to a phantom set, and _InternalSerialize calls sent to Any's.
  • 36 are the same edge under a new ref name. _internal_metadata_.mutable_unknown_fields<…>() now carries its receiver; it is counted again under "added".
  • About 36 right edges are lost, mostly msg->GetReflection() guessed onto Message::GetReflection. That only worked while it was the one GetReflection method; see the note on name-only guesses below.

protobuf, added (2,286).

  • 1,834 are typed resolutions: qualified names, receiver types, return-type chains. In a random 30, 29 were right (Arena::Create<…>, CodedInputStream::ReadTag, Any::PackFrom, MapKey::SetUInt64Value, extends Message). The one wrong one is a template parameter named Map read as the project's Map class.
  • 440 are name-only picks: about 294 right, about 146 wrong.
    • Right: the PROTOBUF_MUSTTAIL return SingularVarint<…>(…) tail calls inside TcParser, calls to the class's own members, the *_globals_ reads.
    • Wrong: see the note below.

protobuf, retargeted (1,147). Checked with explicit per-pattern rules (verdict-retargets.mjs):

  • 546 wrong → right.
    • 191 instantiates edges move from phantom functions to the CodedInputStream, CodedOutputStream, Gzip*Stream and Arena classes.
    • 113 arena.SpaceUsed()-style calls reach Arena, not SerialArena/ThreadSafeArena.
    • new (&_impl_) Impl_(…) reaches each class's own Impl_.
    • Plus WireFormatLite::WireTypeForFieldType, and calls inside RepeatedField/Map that reach their own members.
  • About 36 right → wrong. this_._impl_._extensions_.IsInitialized(…), MessageType::GetDescriptor() on a template parameter, repeated->GetArena().
  • 404 trade one wrong target for another, e.g. _impl_.type_url_.Destroy() goes from CleanupNode::Destroy to Arena::Destroy.
  • 164 are the same target, re-keyed.

leveldb.

  • Added (68): right — mutex_.Lock() / Unlock() / AssertHeld() reach port::Mutex (the methods didn't exist before), plus own-class calls like IncrementBy(1).
  • Retargeted, 6 now right: env_->random_read_counter_.Reset() / Read() reach AtomicCounter instead of BlockBuilder::Reset / CountingFile::Read, and state.Wait(…) reaches TestState::Wait.
  • Lost: 5 right CondVar::Wait edges (shared->cv.Wait(), w.cv.Wait(), state_cv_.Wait()).
  • Retargeted, 3 now wrong: state.cvar.Wait() calls move to TestState::Wait.
  • Removed: 2 wrong locks_.Remove(…) guesses on a std::set.

rocksdb.

  • Added (194):
    • InstrumentedMutex::Lock / Unlock / AssertHeld (88);
    • the clock cache's HandleImpl / NextWithShift members;
    • CountingSemaphore;
    • extends edges for the ALIGN_AS(…) classes.
  • Removed: 72 wrong t.join() guesses (a std::thread) onto port::WindowsThread::join, plus calls misread from declarations.
  • Retargeted:
    • 2 right → wrong: gtest's ~linked_ptr() { depart(); }.
    • 6 questionable: CacheShard::ComputeHash on a template parameter now matches ClockCacheShard::ComputeHash by an unanchored string suffix (name-matcher.ts:7423).

fmt.

  • Calls to the phantom function basic_string_view now instantiate the class or call its constructor.
  • lhs.compare(rhs) in basic_string_view's operators reaches basic_string_view::compare instead of bigint::compare.
  • basic_ostream_formatter and generic_context are indexed.
  • Vendored gmock's methods link their own and their base class's members.
  • Removed (22): calls misread from declarations, #include <locale> linking to a project node, and edges from phantom gtest methods.

redis (C control). jemalloc's tests now run from TEST_BEGIN(name) to TEST_END. A TEST_END above a comment is now blanked, so the next test no longer starts at the previous one's TEST_END under the name TEST_BEGIN. It is named like the first test in each file, (test_oom_errors) (the C extractor's existing form). A typedef below a lone macro and a comment is indexed. No edge changes.

Name-only guesses onto the restored members

When the .pb.h members became methods, names that used to have one method stopped being unique, and other names gained their first. Strategy 3's "the only method with this name" and receiver-word scoring then pick differently.

  • With fix(cpp): a receiver declared as a std or other outside type with a lowercase name calls that type's own member #2413 merged, std / abseil receivers declared in the caller are gated: expected_to_fail_.contains(…), seen.insert(…) and similar no longer guess onto the restored google::protobuf::Map.
  • The new wrong guesses that remain in protobuf (about 146) fall into three groups. All are existing resolver gaps that only show now that the targets exist:
    1. Static calls through a generated class the index doesn't hold. FeatureSet::descriptor() (descriptor.pb.h is over the 1 MB indexing cap), OneofOptions::default_instance(). The receiver-word score picks a restored generated class (JavaFeatures_NestInFileClassFeature::descriptor, about 80).
    2. Unqualified calls inside a class method that don't prefer the class's own or inherited member. GetArena() → Arena::InternalHelper::GetArena instead of MessageLite::GetArena (34). InternalSwap(&from) in each generated class → Any::InternalSwap (86, already wrong before, toward InternalHelper).
    3. Containers the gate can't see. A member declared elsewhere or reached through a pointer parameter: classes_.emplace(…), result->emplace(…) → Map::emplace (20).
  • leveldb's lost CondVar::Wait edges come from receivers the C++ inference can't type: a member reached through another (shared->cv), or a field declared below the inline method that uses it (it scans upward only). They resolved only while CondVar::Wait was the one Wait.

These are filed as follow-ups rather than folded in here.

Overlap

🤖 Generated with Claude Code

colbymchenry and others added 4 commits October 7, 2026 07:26
…can't read

tree-sitter-cpp has no preprocessor, so an attribute macro where it
expects a name or a type misparses the declaration around it. The C++
preParse already blanked one export macro before a class name, an inline
macro before a return type, and a lone macro line before a declaration.
It missed:

- several macros, or one with arguments, between `class` and the name
  (protobuf's generated `class PROTOBUF_EXPORT
  PROTOBUF_FUTURE_ADD_EARLY_WARN_UNUSED Any final : public Message`,
  rocksdb's `struct ALIGN_AS(64U) HandleImpl`), and a macro before a
  partial specialization's name;
- a macro between a pointer and the declared name (`const Descriptor*
  PROTOBUF_NONNULL descriptor()`), after a parameter list
  (`unknown_fields() const ABSL_ATTRIBUTE_LIFETIME_BOUND {`, leveldb's
  `LOCKS_EXCLUDED(mu_) {`), after a declared name (`int count_
  GUARDED_BY(mu_);`), or opening a declaration
  (`PROTOBUF_FUTURE_ADD_EARLY_NODISCARD absl::string_view name() const`);
- a lone macro line with a comment under it ({fmt}'s
  `FMT_BEGIN_EXPORT` above `// A generic formatting context ...`).

Every generated protobuf message class misparsed, so its members were
indexed as namespace-level functions; leveldb's annotated methods became
phantoms named after the annotation.

The new passes match on the code alone (comments and directives as
spaces, literals as placeholders), blank only the macro tokens, and keep
every offset. They run in the hoisted preParse, so files that now parse
clean go through the native kernel, at parity.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…s fixtures misparsing

The new passes each masked the file again. A pass that blanks nothing now
hands the next the same string, and one that blanks hands it the mask
blanked the same way, so a file is masked once. The declarator pass also
looks for the macro first instead of trying every word. The preParse
output is byte-identical on every C/C++ file of leveldb, fmt, protobuf,
rocksdb and redis.

#2426's brace-scope fixtures misparsed only through
PROTOBUF_FUTURE_ADD_EARLY_NODISCARD and ALIGN_AS(64U), which the preParse
now blanks. They now use macros it doesn't know (NODISCARD,
cacheline_aligned(64U)), with the same expected scopes.

CHANGELOG entry under [Unreleased] -> Fixes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant