Skip to content

fix(pipeline): a bare C reference never binds a struct Field - #2754

Merged
DeusData merged 5 commits into
DeusData:mainfrom
gaurav0107:fix/2666-c-extraction-a-macro-argument-identifier
Oct 29, 2026
Merged

DeusData merged 5 commits into
DeusData:mainfrom
gaurav0107:fix/2666-c-extraction-a-macro-argument-identifier

Conversation

@gaurav0107

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #2666. A bare C identifier that names no visible declaration reached the short-name registry fallback in the USAGE and READS/WRITES resolvers, and the registry bound it to whichever struct Field had the same name. The issue's example is RUN_SELECTED_SUITE(mcp); in tests/test_main.c, which gave main a USAGE edge to cbm_daemon_application_session.mcp. The same path also turned bare local writes into WRITES edges onto unrelated structs. For example, size_t offset = 0; ... offset += ... in diag_write_file wrote cli_client_def_t.offset.

In C, a struct member is only named as the member half of a.b, a->b or a designator .b. The extractor records all of these as field_identifier (is_member_access), and they already bind through the C LSP owner rows (cbm_c_member_binds_by_owner). The call path already drops a bare C call onto a Field (cbm_call_onto_field_policy). References had no matching rule.

The change:

  • cbm_go_suppress_bare_field_ref(bool is_go, ...) is now cbm_suppress_bare_field_ref(CBMLanguage lang, ...). It applies to Go, as before, and now to C as well. The four call sites pass lang: resolve_usage_edges / resolve_rw_edges in pass_usages.c and their parallel twins in pass_parallel.c.
  • C++ is unchanged, and so is every .h, since headers are parsed as C++. A method body names its own members without a selector, so a veto there would break C++ (and C#, Java and the other languages that already keep this behavior).
  • Member-half references, designators and offsetof(T, member) keep their edges (checked below).

Evidence

Machine: Amazon Linux 2023, x86_64, clang 15.0.7. gcc here has no libasan, so the sanitizer build uses clang.

New tests. I ran them first against a RED tree: main's Go-only gate behind the new function signature, so only the behavior differed.

Test RED tree With fix
registry: c_bare_ref_never_binds_field FAIL: cbm_suppress_bare_field_ref(CBM_LANG_C, 0, "Field") PASS
pipeline: pipeline_c_bare_ref_never_binds_field (sequential) FAIL: main -USAGE-> mcp exists PASS
pipeline: pipeline_c_bare_ref_never_binds_field_parallel (52 padding files to go past MIN_FILES_FOR_PARALLEL) FAIL: same PASS

The pipeline fixture covers the issue's macro-argument shape, plus a bare write (enabled = 1;) to a name a struct member also has. Its positive controls, s->count (USAGE) and s->enabled = 0 (WRITES), must keep binding the Field.

Commands:

scripts/test.sh --suites "registry pipeline" CC=clang CXX=clang++ SANITIZE="-fsanitize=address -fno-omit-frame-pointer"
  RED tree:  507 passed, 3 failed (the three new tests)
  with fix:  510 passed

scripts/test.sh --suites "registry pipeline c_lsp go_lsp extraction call_reference_contract call_reference_language_complex_contract repro_call_scope_usages repro_call_argument_usages repro_reference_precision repro_call_argument_matrix_a repro_call_argument_matrix_b repro_lexical_binding_precision edge_types_probe matrix_known_classes matrix_new_constructs lang_contract integration incremental parallel test_impact_engine conditional_variants edge_structural lsp_resolution_probe convergence_probe" CC=clang CXX=clang++ SANITIZE=...
  2875 passed, 0 failed
scripts/test.sh --suites cli ...   364 passed

End to end with a product build (make -f Makefile.cbm cbm). I indexed a git archive of main @ 3457b21 with both binaries and diffed every edge in the two databases:

main @ 3457b21 this branch
The issue's query_graph (main in tests/test_main.c -> mcp) 1 row: USAGE -> src.daemon.application.cbm_daemon_application_session.mcp 0 rows
USAGE edges onto Field 53,133 52,727 (-406)
WRITES edges onto Field 18,708 11,392 (-7,316)
Edges added 0

All 7,722 removed edges have a C caller and a Field target. A random sample of 25 were all bare-name guesses: locals such as offset, trait_qn and i bound to other structs' members, and macro arguments such as key in HT_LOOKED_UP(ht, key, found) bound to cbm_yaml_node.key. Nothing else changed. On a small fixture, designators (fixture_t f = { .count = 1 }), f.count = ..., f->count and offsetof(fixture_t, count) produce the same edges as on main.

Not verified locally

  • The full default scripts/test.sh venue run: on main it stops at Step 0w, the published language-count contract (docs/index.html, docs/llms.txt, pkg/npm/README.md, ...). scripts/test.sh --contracts-only reports the same failures on this branch, and this diff does not touch those files.
  • scripts/lint.sh: cppcheck 2.20 and clang-format-20 aren't installed here. The changed lines pass clang-format 15 (git diff -U0 upstream/main..HEAD | clang-format-diff -p1 is empty). I'm relying on the lint CI job for the rest.

Checklist

  • Every commit is signed off (git commit -s): scripts/check-dco.sh upstream/main..HEAD OK
  • Tests pass locally (make -f Makefile.cbm test): the 26 related suites above pass. I didn't do the full venue run (see above).
  • Lint passes (make -f Makefile.cbm lint-ci): not run locally (see above).
  • New behavior is covered by a test (reproduce-first for bug fixes)

🤖 Opened with Superhuman, an open-source contribution agent.

A bare C identifier that names no visible declaration reached the
short-name registry fallback in the USAGE and READS/WRITES resolvers,
which bound it to whichever struct member carried the same name.
tests/test_main.c's RUN_SELECTED_SUITE(mcp) gave `main` a USAGE edge
to cbm_daemon_application_session.mcp, and every bare local write
(`offset = 0`, `trait_qn = head`) became a WRITES edge to an unrelated
struct's field.

In C a member is only ever named as the member half of `a.b`, `a->b`
or a designator `.b`, all recorded as is_member_access and bound
through the C LSP owner rows. Widen the Go rule (DeusData#1942/DeusData#1962) to C:
cbm_go_suppress_bare_field_ref becomes cbm_suppress_bare_field_ref and
takes the caller language. C++ (and every .h, parsed as C++) keeps its
bare member references, as a method body names its own members
without a selector. Sequential and parallel resolvers both consult it.

Fixes DeusData#2666

Signed-off-by: Gaurav Dubey <gauravdubey0107@gmail.com>
@github-actions

Copy link
Copy Markdown

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.

@gaurav0107
gaurav0107 marked this pull request as ready for review October 21, 2026 19:53
@gaurav0107
gaurav0107 requested a review from DeusData as a code owner October 21, 2026 19:53
@gaurav0107

Copy link
Copy Markdown
Contributor Author

This comment was written by an AI agent operating this account on behalf of @gaurav0107.

The red test-unix (ubuntu, macos) and test-windows legs come from a check that also fails on main, not from this change. shard-completeness and ci-ok are red only because they depend on those legs. scripts/test.sh stops at Step 0w, the published language-count contract, before it runs any suite:

FAIL: 5 language-count contract violation(s)

The five flagged surfaces are docs/index.html, docs/llms.txt, pkg/npm/README.md, scripts/package-release.sh and server.json. Locally, scripts/test.sh --contracts-only reports the same five on pristine main @ 3457b21, and this PR doesn't touch any of those files. Everything else is green: lint (cppcheck + clang-format-20), lint-mem, test-tsan, test-ui, pr-smoke, codeql, security-static and dco. The suites I ran locally are listed in the description.

@DeusData

Copy link
Copy Markdown
Owner

Thanks for the focused C field-reference fix, the regression tests, and the CI investigation. You were right about the language-count preflight failure: the shared repair has now merged as #2732. I merged main into this PR in bc59284 so the existing branch picks it up while preserving your implementation and commit history. Fresh PR CI is now running.

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

Copy link
Copy Markdown
Owner

Thanks again for the C field-reference fix! I merged current main into this branch in 17a85cc so GitHub CI validates it alongside the recently merged pipeline changes. The merge was conflict-free, and I verified that your patch is unchanged relative to main. This also brings in the shared CI fixes. Thank you for the contribution!

@gaurav0107

Copy link
Copy Markdown
Contributor Author

This comment was written by an AI agent operating this account on behalf of @gaurav0107.

Thanks for landing the language-count fix in #2732 and for the two main merges (bc59284, 17a85cc). I checked that CI is now green on 17a85cc with no failing checks. The branch is ready from my side, and I will not push over your merges.

@DeusData DeusData added bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges labels Oct 25, 2026
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
@DeusData

Copy link
Copy Markdown
Owner

Thank you, @gaurav0107, for checking CI and for keeping our merges in place. Main has moved again since then (tonight's merges touched tests/test_pipeline.c, which this pull request also changes), so we merged current main into your branch once more (one merge commit, a0940ce; your changes are untouched). We plan to merge once the required checks are green on this head. Please pull before pushing again.

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

Copy link
Copy Markdown
Owner

Thank you for your patience, @gaurav0107. CI went green on this branch, but this evening's merges on main touched files this pull request also changes, so that green no longer covered today's main. We merged current main into the branch once more (one merge commit, 1aeaf6f; your changes are untouched) and will merge once the fresh checks are green. Please pull before pushing again.

@gaurav0107

Copy link
Copy Markdown
Contributor Author

Thanks @DeusData for merging main in again (a0940ce, then 1aeaf6f). I won't push anything on top of 1aeaf6f unless you ask, and if I do I'll pull first. The checks on that head have passed apart from test-unix (ubuntu), which is still running.

@DeusData
DeusData merged commit 442989e into DeusData:main Oct 29, 2026
26 checks passed
@DeusData

Copy link
Copy Markdown
Owner

Merged - thank you very much, @gaurav0107! A bare C identifier no longer binds to whichever struct Field happens to share its name, so RUN_SELECTED_SUITE(mcp); stops pointing main at an unrelated session struct, and plain local writes like offset += ... stop turning into WRITES edges onto other structs. Reusing the existing Go suppression rule for C, while leaving C++ and headers alone because a method body really does name its own members without a selector, was exactly the right line to draw, and the registry and pipeline tests pin both sides of it. Thank you also for your patience while main kept moving under this branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges

Projects

None yet

Development

Successfully merging this pull request may close these issues.

C extraction: a macro-argument identifier becomes a USAGE edge to a same-named struct field

2 participants