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

Merged
CO
8d ago
+1015-24

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.
  • #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)

repofiles with errorserror sites
protocolbuffers/protobuf223 → 172 (of 941)7,237 → 1,000
google/leveldb19 → 8 (of 128)104 → 28
fmtlib/fmt31 → 30 (of 72)620 → 442
facebook/rocksdb210 → 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 #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++):

repoclassesmethodsfunctionsphantom nodes gone
protobuf908 → 96812,131 → 13,64818,802 → 18,11679
fmt394 → 4032,688 → 2,7751,999 → 1,92168
rocksdb2,718 → 2,72324,531 → 24,56612,617 → 12,59133
leveldb149 → 1491,307 → 1,306511 → 51115
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):

reporemovedaddedretargetedsame edge, new metadata
protobuf7902,2861,147531
rocksdb801941670
fmt221986740
leveldb76893
redis0100

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 #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

  • #2426 (brace scopes, merged) restores scopes in files that still error. This PR makes many of those files parse clean in the first place, so they go through the tree's own scopes and the kernel. #2426's tests needed new triggers (above).
  • #2413 (std receiver gate, merged) is what keeps the uniqueness flip on Map's members from turning into hundreds of std-container guesses.
  • The A/B above is against main with both.
  • #2446 (open: a type defined in a variable declaration is indexed) also edits c-cpp.ts, but only new helpers at the top of the file, and the walker. There is no shared code. Whichever lands second keeps both CHANGELOG bullets.
  • #2439 / #2440 / #2443 (open C++ receiver-inference PRs) don't touch the preParse. Their A/B counts on protobuf will shift once this lands, because the restored members become targets.

🤖 Generated with Claude Code