Skip to content

fix(extract): one definition per site when the raw and preprocessed parses disagree - #2629

Merged
DeusData merged 2 commits into
mainfrom
fix/rescue-one-def-per-site
Oct 13, 2026
Merged

DeusData merged 2 commits into
mainfrom
fix/rescue-one-def-per-site

Conversation

@DeusData

Copy link
Copy Markdown
Owner

The #961/#1989 rescue re-adopts definitions from the preprocessed second
pass when they sit in a raw parse error, and drops a copy only when its QN
equals a raw one. When the two parses scope the same definition
differently, both copies survived:

  • Linux lib/tests/randstruct_kunit.c: a macro-generated struct body makes
    the raw parse nest every later struct inside one ERROR region; the flat
    expanded copies were adopted next to the mis-nested raw ones (5
    duplicate groups).
  • coreclr/rapidjson: the expanded parse nests utilcode.h classes wrongly
    or loses rapidjson's scopes, and both copies survived.

One rule at one decision point: an expanded and a raw definition with the
same label, name and start line but different QNs are one definition. The
raw copy wins and whatever the expanded pass attributed to the expanded QN
moves to it -- unless

  • the raw copy's nesting exists only because of a raw ERROR region that
    the expanded parse resolved (the randstruct shape, six conditions in
    cbm_misnested_raw_twin): then the flat expanded copy replaces the raw
    one and the raw children (Fields, nested defs, call/usage scopes) are
    re-parented to it; or
  • the raw copy's QN is already held by a raw definition of another label
    (coreclr inc/holder.h: the raw parse reads a FORCEINLINE constructor as
    a Function of the class's scope-less QN): then the expanded copy keeps
    its own, correct QN instead of vanishing into the Function's node.

Proof (macOS arm64):

  • randstruct_kunit.c: 5 -> 0 duplicate groups, 12 Fields re-parented to
    the flat structs, CALLS/USAGE/WRITES unchanged.
  • dotnet/runtime native subset (src/coreclr + src/native): 700 -> 183
    groups; the remaining groups are no raw/expanded pairs at one site.
  • redis: node and edge sets byte-identical.
  • New tests (RED without the change): extraction
    extract_c_misnested_raw_struct_replaced_by_flat_copy,
    extract_cpp_preprocessed_rescue_skips_raw_def_on_same_line,
    extract_cpp_rescue_keeps_expanded_class_when_raw_qn_is_taken (fixture
    delta-minimized from holder.h).
  • conditional_variants, extraction, c_lsp, pipeline, edge_types_probe,
    parse_coverage, lsp_resolution_probe: 1854 passed.

…d pass

The preprocessed second pass rebuilt the expanded source with simplecpp's
TokenList::stringify(), which puts one space between every pair of tokens:
`template<> struct bitonic<int64_t, AVX2>` became
`bitonic < int64_t , AVX2 >`. Every QN builder takes the class segment from
node text, so a definition the #961 rescue re-adopted from that buffer
(vxsort's INLINE/NOINLINE macros break the raw parse on every method line)
got a second, token-spaced QN, and the QN-equality dedup kept both. CALLS
split across the two spellings (dotnet/runtime: 902 compact + 996 spaced
into the bitonic sorters).

The expanded buffer now keeps the source spacing: tokens that touched in
the source (same file and line, no macro expansion between them) stay
touching, every other gap is still one space, and the line structure and
#line directives are unchanged, so every line remap holds. One fix point
instead of a normaliser at each QN builder; no vendored code changes.

Proof (macOS arm64, dotnet/runtime 60108ba6, two identical runs each):
- duplicate definition groups 706 -> 217 (v0.11.0: 545); nodes with a
  token-spaced QN 495 -> 0; CALLS into bitonic 902 + 996 -> 998, one
  spelling. The remaining groups are coreclr gc.cpp's SVR/WKS builds (184)
  and 33 scope disagreements between the two parses.
- Side effect: rapidjson document.h's GenericValue members now exist once,
  with the raw parse's scope (the token-spaced copy that carried the
  nested scope is gone); `operator[]` / `operator()` in the expanded pass
  now parse as single tokens.
- New test (RED without the change): extraction
  extract_cpp_template_specialization_preprocessed_qn_single_spelling.
  Three c_lsp collision fixtures recalibrated to the new byte offsets
  (comment padding and offsets only, assertions unchanged).
- extraction, c_lsp, pipeline, edge_types_probe, parse_coverage,
  conditional_variants, lsp_resolution_probe: 1852 passed.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…arses disagree

The #961/#1989 rescue re-adopts definitions from the preprocessed second
pass when they sit in a raw parse error, and drops a copy only when its QN
equals a raw one. When the two parses scope the same definition
differently, both copies survived:

- Linux lib/tests/randstruct_kunit.c: a macro-generated struct body makes
  the raw parse nest every later struct inside one ERROR region; the flat
  expanded copies were adopted next to the mis-nested raw ones (5
  duplicate groups).
- coreclr/rapidjson: the expanded parse nests utilcode.h classes wrongly
  or loses rapidjson's scopes, and both copies survived.

One rule at one decision point: an expanded and a raw definition with the
same label, name and start line but different QNs are one definition. The
raw copy wins and whatever the expanded pass attributed to the expanded QN
moves to it -- unless
- the raw copy's nesting exists only because of a raw ERROR region that
  the expanded parse resolved (the randstruct shape, six conditions in
  cbm_misnested_raw_twin): then the flat expanded copy replaces the raw
  one and the raw children (Fields, nested defs, call/usage scopes) are
  re-parented to it; or
- the raw copy's QN is already held by a raw definition of another label
  (coreclr inc/holder.h: the raw parse reads a FORCEINLINE constructor as
  a Function of the class's scope-less QN): then the expanded copy keeps
  its own, correct QN instead of vanishing into the Function's node.

Proof (macOS arm64):
- randstruct_kunit.c: 5 -> 0 duplicate groups, 12 Fields re-parented to
  the flat structs, CALLS/USAGE/WRITES unchanged.
- dotnet/runtime native subset (src/coreclr + src/native): 700 -> 183
  groups; the remaining groups are no raw/expanded pairs at one site.
- redis: node and edge sets byte-identical.
- New tests (RED without the change): extraction
  extract_c_misnested_raw_struct_replaced_by_flat_copy,
  extract_cpp_preprocessed_rescue_skips_raw_def_on_same_line,
  extract_cpp_rescue_keeps_expanded_class_when_raw_qn_is_taken (fixture
  delta-minimized from holder.h).
- conditional_variants, extraction, c_lsp, pipeline, edge_types_probe,
  parse_coverage, lsp_resolution_probe: 1854 passed.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
@DeusData
DeusData force-pushed the fix/rescue-one-def-per-site branch from a0aee2b to c43b319 Compare October 13, 2026 16:48
@DeusData
DeusData merged commit 5058d57 into main Oct 13, 2026
3 checks passed
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