fix(parsing): recover export-macro-hidden type definitions (#1989)

[XIYBHK]

What does this PR do?

Fixes #1989.

Recover C/C++ definitions hidden by conventional build-system export macros through the existing preprocessing second pass. Candidate collection is lexically bounded to 32 names per file; explicit caller defines retain priority. Recovered definitions pass line mapping, original-source visibility, and qualified-name reconciliation gates. The change covers class/struct/enum definitions, free functions, and inline methods, with negative controls for ordinary all-caps identifiers.

Rebased onto main at 92abefa3. The implementation preserves the upstream error-region dropped counter initialization and uses cbm_calloc/cbm_free with CBM_MEM_CLASS_EXTRACT for its temporary superseded-definition bitmap. All 19 original regression tests remain registered. The PR contains one signed-off commit, fcf451c6, touching only the original four files. Project-level macro configuration remains in follow-up #2049.

Validation

• git diff --check, diff-scoped formatting checks, and strict GCC/G++ syntax checks passed for the changed translation units.
• scripts/security-audit.sh passed.
• Local runtime tests have not passed: after supplying zlib, the unsanitized MinGW GCC runner exited with heap corruption in the first existing extraction test, before the export-macro regressions ran. No local sanitizer validation is claimed.
• The maintainer confirmed the current upstream lint/test blockers are being addressed in fix: restore main to green after #1723 — memory-core linter and worker-scope Step 5f #2257 and will update this branch and run full CI plus local extraction tests after that fix lands. This PR does not include the unrelated request-handler cleanup or relax any lint baseline.

Known boundaries: one-character export prefixes such as X_API remain intentionally rejected; header-only prototypes retain the existing extraction behavior; UINTERFACE cascade handling remains outside this PR.

[DeusData]

Holding note, because you are owed one before any review verdict.

On #1989 you asked two direct questions:

Is the ^[A-Z0-9_]*_(API|EXPORT|IMPORT|DLLEXPORT|DEPRECATED)$ shape acceptable as the candidate-macro filter, or would you prefer an explicit config-driven list only (option 2)?

Since this changes what gets extracted (per CONTRIBUTING), may I open a focused PR for (1)+(2)?

Nobody answered. You did the right thing by asking, waited, and then built it anyway rather than letting the report rot — but this PR's description calls it "the agreed option 1", and I want to be straight with you: it was never agreed, because we never replied. That is our failure, not a liberty you took.

I have put the direction question — heuristic shape versus config-driven defines versus both — in front of the maintainer now, and I will bring you a real answer rather than letting it sit again.

Two things I can say already, having read the diff rather than the summary:

The report is exemplary. Naming UBT's /D "MODULE_API=" and CMake's generate_export_header() as the mechanism, then pinning it to three public headers — spdlog logger.h:50, fmt os.h:222, protobuf descriptor.h — turns "my code doesn't index" into a defect anyone can reproduce in a minute. Enumerating three fixes with costs, and asking which we wanted, is more than most maintainers get.

And the implementation is careful in the places that are easy to get wrong. skip_raw_string handles the fact that " is a legal d-char, so R"""(...)""" does not derail the scan; skip_non_code handles line-spliced // comments, where a backslash before the newline continues the comment; the >= slen + 2 prefix rule keeps a single-letter X_API out; and the uncertain paths return -1 and stop collecting rather than guessing. Failing toward not injecting a define is the right direction for a heuristic that changes what gets extracted.

None of that decides the direction question, which is genuinely the maintainer's. But it does mean that if the answer is yes, this is in good shape.

Thank you for the patience, and sorry it took a PR to get a reply.

[github-actions]

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

[XIYBHK]

Thanks for the clarification. I may be interpreting the timeline differently, so I checked the issue history.

Before opening this PR, I asked whether to pursue the heuristic option 1 or the config-driven option 2, and whether to combine them. The reply on #1989 explicitly asked me to "open a focused PR for option 1 only" and specified the candidate cap, negative controls, no user-configured define plumbing, and the class/struct/enum/free-function test boundary: #1989 (comment)

I used that as the scope for this PR. I agree that the exact regex was not approved character-for-character; the implementation made the candidate filter deliberately conservative and bounded. If the exact filter or any other part of the scope should change, I am happy to adjust it.

I just wanted to clarify that the option-1 direction and scope were discussed in the issue before implementation.

[DeusData]

You are right, and thank you for correcting the record.

The maintainer reply on #1989 did explicitly ask for a focused PR implementing option 1 with the bounded candidate scan and the stated negative controls. Our earlier comment here saying that the direction had never been agreed was wrong. I am sorry: you followed the scope we gave you.

The option-1 direction therefore remains the approved scope. The exact candidate filter and implementation still receive normal review on their merits, but you do not need to re-litigate why this PR exists. Thank you for responding with the thread reference and keeping the correction factual.

[XIYBHK]

Rebased onto the latest main (2f9828d) and resolved the conflict in tests/test_extraction.c by preserving both the upstream extraction tests and the #1989 regression coverage. The PR remains a single commit and still changes only the four intended files.

The rebased files pass single-file compilation and clang-format checks locally. The full sanitizer test runner cannot be linked in the local MinGW environment because its sanitizer runtime/spec file is unavailable; the refreshed CI matrix is running now.

[DeusData]

Direction settled — thank you for building this, and apologies that the two questions on #1989 went unanswered before you did. That was on us, and the answer was yes.

The zero-config shape rule (option 1) is accepted as the default: the candidate collection is bounded and lexically careful, the recovered definitions go through the line-mapping / visibility / qualified-name gates before they replace anything, and explicitly configured defines keep priority. That is the right layer and the right amount of caution.

Two things so you know what happens next:

1. Merge sequencing. Your branch is green and cleared. Three other PRs that also append to tests/test_extraction.c (fix(extraction): reach a Swift URL built by a constructor #1976, fix(elixir): populate first_string_arg for Elixir calls #1721, fix(elixir): Phoenix channel extraction is unreachable in both branches #1730) are mid-way through their CI right now; landing yours first would void all three matrices in a very loaded queue, so I will let them land and then bring this branch up to date once and merge on green. Nothing needed from you — if the branch update needs a hand I will say so here.

2. Follow-up, not a blocker: an explicit override for the rule (extend the candidate set for macros the shape rule cannot see, exclude a name that is a genuine symbol) — the reporter's option 2 layered on top of yours. Filed as Export-macro recovery: project-level override for the candidate rule (follow-up to #2005) #2049; you are the natural person to take it if you want to, no timing pressure.

[DeusData]

Thank you for rebasing onto today's main — and please stop there for a moment, because you are about to spend effort on a problem that is ours, not yours.

The lint / lint red you are trying to get past is main's own. The memory-core linter counts raw free() sites in src/mcp/mcp.c against a ratcheted baseline, and a merge on our side on Saturday pushed that file three over (814 -> 817). Every PR based on main fails that check right now, whatever it touches. Your second commit, 6eb89924 ("share index request cleanup on early exits"), is a reasonable way to claw the count back — but:

• it is already being fixed, in fix: restore main to green after #1723 — memory-core linter and worker-scope Step 5f #2257, which gives handle_index_repository a single release helper for its three argument strings (817 -> 785) and is in its last CI legs as I write this;
• your commit conflicts with it — both reshape the same early-return blocks, so once fix: restore main to green after #1723 — memory-core linter and worker-scope Step 5f #2257 lands your branch would go CONFLICTING on a file your PR has no reason to touch;
• and it folds the cross-repo-mode branch into the policy-error branch, which changes when handle_cross_repo_mode runs relative to the policy load. That may well be harmless, but it is a behaviour question in a request handler, and it should not ride along inside a C/C++ parsing fix where nobody will think to look for it.

So: please drop 6eb89924 and keep the PR at the single commit fcf451c6 and the four files it was always meant to touch (git reset --hard fcf451c6 && git push --force-with-lease — force-pushing your own PR branch is completely fine here). Do not worry about the lint red in the meantime; it will not be held against this PR.

What happens next, and it is all on our side: #2257 lands (it also fixes a second main red, in the test stage, that was hidden behind the lint failure). Then I bring your branch up to date with main, which gives it a fresh, full CI run; I build and run the extraction suites on the merge result locally; and it merges. The direction was settled on 4 September and nothing about that has changed — the sequencing I described then simply took far longer than it should have, and I am sorry for that.

Thank you for your patience, and for keeping the PR current through a very busy stretch on main.

[XIYBHK]

Thank you for the clear explanation and for coordinating the fix in #2257.

I've dropped 6eb89924 and restored the branch to fcf451c6. The PR is back to one signed-off commit and the original four files, and I've updated the description accordingly. Sorry for adding an unrelated cleanup while trying to get past the CI failure.

I'll leave the branch as-is and let you handle the update and validation after #2257 lands. Thank you again for reviewing this and keeping us informed.

[DeusData]

Merged as c7488b0 — thank you, and thank you for your patience through a sequencing promise that took far longer than it should have.

Verified on the merge result before landing: extraction 369/0 with all the #1989 tests green; c_lsp, pipeline, lang_contract, parse_coverage, grammar_regression and complexity green; with production reverted the tests no longer compile, so they bind to cbm_export_macro_candidates; memory-core linter clean; no raw allocator or fopen in the added lines.

A nice coincidence worth telling you about: on the morning of the merge a reporter on #1153 ("method calls through object pointers are dropped", priority/high) independently traced that bug to exactly this defect — class MYMODULE_EXPORT Worker { being read as a function. I ran their example through main before and after your change: Worker goes from Function back to Class and the missing CALLS edge returns, identical to the no-macro control. So this fix closes more than the issue it was written for.

The explicit override layered on top of the shape rule (the reporter's option 2 on #1989) remains open as a follow-up, as discussed.