fix(extraction): collapse a damaged C #if group to its first branch before parsing - #1736
Open
danusha2345 wants to merge 2 commits into
Open
fix(extraction): collapse a damaged C #if group to its first branch before parsing#1736danusha2345 wants to merge 2 commits into
#if group to its first branch before parsing#1736danusha2345 wants to merge 2 commits into
Conversation
…nts before parsing tree-sitter-c has no rule for `.field = value` as a call argument. A statement-level `MACRO(a, b, .x = …, .y = …, …);` parses with an ERROR per argument, and past roughly a hundred of them the grammar's error recovery gives up on the enclosing function: the function_definition runs to the end of the file, the function after it produces no node at all, and every later function is nested under the first (colbymchenry#1729). betaflight resets each config struct that way — `resetPidProfile` was lines 168–1667 in the graph against 168–309 in the source, with 45 functions as its children; 310 such functions in 73 files across the tree. Name matching read the nesting as a scope (colbymchenry#1230), so exact-match declined those 45 for every cross-file caller and the fuzzy fallback carried 117 real calls at 0.5. blankCDesignatedMacroArgs runs at the head of preParseCSource: a statement-level call of a macro-cased name whose argument list holds a designator (`.name =` or `[index] =` at argument depth) has that list emptied to spaces, newlines kept, so `RESET_CONFIG(\n\n…\n);` parses cleanly and every offset survives. The references inside the initializer are the price; the broken parse was not yielding them either. The pre-parse runs before the kernel route point, so both arms see the same bytes and the kernel needs no change. betaflight fork, 2,109 C files, against b9ca4b7: resetPidProfile 168–309, nested C functions 310 → 265 (the rest are `#if`-damaged HAL sources, a different shape), +3 function nodes (`isTpaActive` and two more the old parse dropped), 117 fuzzy calls → 118 exact-match calls, 45 `contains` edges moved from resetPidProfile to the file node. Nothing else moved. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… before parsing
tree-sitter-c keeps every branch of a `#if … #elif … #else … #endif` group
and parses each as a run of block items, so a group is only harmless when
each branch is complete on its own. Three shapes in real C are not: a branch
that begins with `else` (`} #ifdef X else if (…) { … } #endif`), a branch
that ends with a bare `if (…)` header whose body follows the `#endif` (the
ST HAL's per-device flash-latency tables), and a branch whose braces do not
balance (a signature or a `{` that differs per configuration, body shared).
Each ends in a K&R-shaped `if(cond) { … }` that the grammar reads as an
implicit-int function definition named `if`; the extractor then files it —
and every function after it — under the enclosing function, or the enclosing
function runs to the end of the file and the ones after it vanish (265 such
nested "functions" in 72 files on a betaflight tree, the remainder after
colbymchenry#1729's designated-initializer blank). A second, smaller cause was an
all-caps block macro in statement position (`ATOMIC_BLOCK(NVIC_PRIO_MAX) {`,
`PG_FOREACH(reg) {`), which the statement-macro blank skipped by design
because it only matched lowercase iterator macros.
blankCUnbalancedConditionalBranches runs at the tail of preParseCSource,
after restoreDirectiveLines, because it edits directive lines on purpose:
for a group where any branch shows one of the three symptoms, the first
branch not written `#if 0` is kept verbatim and every other branch plus the
group's directive lines (`\`-continuations included) are blanked to spaces,
newlines and `\r` kept — what the preprocessor would hand a compiler for
that configuration. Balanced groups, the vast majority, are untouched.
Groups nest innermost-first. blankCStatementMacroCalls now also accepts an
ALL_CAPS name (PascalCase — a constructor, if the file is C++ — still
excluded). The C++ grammar parses both shapes cleanly (verified on the same
snippets as .cpp), so preParseCppSource is unchanged; the pre-parse runs
before the kernel route point, so the kernel arm needs no change.
Measured on betaflight (2,109 .c files), against the colbymchenry#1729 build: nested C
functions 265 → 5 (left: `if MACRO {` conditions with no parentheses, and a
`STATIC_DMA_DATA_AUTO union { … } x;` local); C function nodes 39,993 →
39,772 — 260 phantoms gone, ~39 real functions back (spiInternalInitStream,
spiInternalStartDMA, spiInternalStopDMA, processSmartPortTelemetry,
esc4wayProcess, arm_mat_mult_q15, …). Edges: 1,263 lost / 729 gained; the
lost rows are calls previously attributed to the file node or to a phantom
`if` (709 exact-match calls, 490 file-level `contains` of what were really
locals), the gained rows are the same calls re-attributed to the real
function (671 exact-match). Fuzzy-resolved calls 154 → 37.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Stacked on #1733 (its commit comes first; rebases to one commit once that lands). Together they close the C extraction extent problem behind #1729: after #1733 a 2,109-file betaflight tree still had 265 C functions the graph filed inside another function; after this, 5.
What the 265 were
Not one cause, and mostly not real functions. 192 of them are literally named
if, the rest(NVIC_PRIO_TIMER),(reg),ATMEL_DEVICE_MATCH— phantoms. Verified shape by shape with the shippedtree-sitter-c.wasm:#ifbranch that begins withelse. tree-sitter-c parses each preprocessor branch as a run of block items, andelsecannot start one, so theif (cond) { … }that follows is read as a K&R implicit-int function definition namedif, nested in the enclosing function:current.c,stm32*_ll_usart.c,_ll_tim.c,_ll_gpio.c,flash_w25n.c, …)if (…)/else if (…)header whose{body sits after#endif— the STM32H7ll_utils.clatency tables, 24 phantoms in one file.arm_mat_*,smartport.c,bus_spi_ll.c): here the enclosing function vanished entirely and its locals leaked to file scope —arm_mat_mult_q15.chad zero function nodes.ATOMIC_BLOCK(NVIC_PRIO_MAX) {,PG_FOREACH(reg) {— whichblankCStatementMacroCallsmatched in lowercase only.The fix
blankCUnbalancedConditionalBranches: for a#if … #else/#elif … #endifgroup where any branch shows one of shapes 1–3, keep the first branch not written#if 0verbatim and blank every other branch and the group's directive lines (continuations included) to spaces, newlines kept. Balanced groups are untouched; innermost groups first. It runs at the tail ofpreParseCSource, afterrestoreDirectiveLines, for the same reasonblankCNamedVariadicDefineDotsdoes — it edits directive lines deliberately.blankCStatementMacroCallsalso accepts[A-Z_][A-Z0-9_]*now (PascalCase still excluded, for the C++-constructor reason in its comment). The C++ grammar parses both snippets cleanly, so the C++ path is left alone; the pre-parse precedes the kernel route point, so the kernel needs no change.Measured
betaflight fork, 2,109
.cfiles, wasm arm:fuzzycallsEdge-set delta against the #1733 index, rows keyed with
resolvedBy: 1,263 lost / 729 gained. Lost: 709exact-matchcalls whose source was a phantom (if@…) or a file node holding leaked locals, 490 file-levelcontainsfor those locals (pInA,row,flag), 53 references, 11 function-refs. Gained: 671exact-matchcalls from the recovered functions (USBD_LL_Init → HAL_PCD_Init,LL_TIM_DeInit → …,processSmartPortTelemetry → erpmToRpm) and 43containsfor them. Functions that now exist with their real extents:spiInternalInitStream/StartDMA/StopDMA(bus_spi_ll.c),processSmartPortTelemetry(522–903),esc4wayProcess(429–929),arm_mat_mult_q15. Ten lost and ten gained rows read against source: every lost row is a re-attribution from a phantom or a file node, not a real call gone.The 5 that remain: four
if SILABS_DEVICE_MATCH {/else if ATMEL_DEVICE_MATCH {inserial_4way.c(a paren-less macro as anifcondition) and oneSTATIC_DMA_DATA_AUTO union { … } testBuffer;inflashfs.c— each its own narrow blank, not folded in here.Tests
extraction.test.ts: a unit test of the blank (length and line count preserved, a continuation line blanked, the first branch kept, the second branch and the directives blanked, a balanced#ifdefuntouched,#if 0keeps the live branch, identity on a file without groups) and an end-to-end index where a function after a damaged group used to be a phantom(NVIC_PRIO_MAX)and now has its own extent and top-level name; the end-to-end test fails without the change.extraction.test.ts+resolution.test.ts: 815,tscclean.🤖 Generated with Claude Code