Repository navigation
Conversation
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>
|
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. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
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. |
|
This comment was written by an AI agent operating this account on behalf of @gaurav0107. The red The five flagged surfaces are |
|
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>
|
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! |
|
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. |
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
|
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>
|
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. |
|
Merged - thank you very much, @gaurav0107! A bare C identifier no longer binds to whichever struct Field happens to share its name, so |
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
Fieldhad the same name. The issue's example isRUN_SELECTED_SUITE(mcp);intests/test_main.c, which gavemainaUSAGEedge tocbm_daemon_application_session.mcp. The same path also turned bare local writes intoWRITESedges onto unrelated structs. For example,size_t offset = 0; ... offset += ...indiag_write_filewrotecli_client_def_t.offset.In C, a struct member is only named as the member half of
a.b,a->bor a designator.b. The extractor records all of these asfield_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 nowcbm_suppress_bare_field_ref(CBMLanguage lang, ...). It applies to Go, as before, and now to C as well. The four call sites passlang:resolve_usage_edges/resolve_rw_edgesinpass_usages.cand their parallel twins inpass_parallel.c..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).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.
registry: c_bare_ref_never_binds_fieldcbm_suppress_bare_field_ref(CBM_LANG_C, 0, "Field")pipeline: pipeline_c_bare_ref_never_binds_field(sequential)main-USAGE->mcpexistspipeline: pipeline_c_bare_ref_never_binds_field_parallel(52 padding files to go pastMIN_FILES_FOR_PARALLEL)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) ands->enabled = 0(WRITES), must keep binding the Field.Commands:
End to end with a product build (
make -f Makefile.cbm cbm). I indexed agit archiveof main @ 3457b21 with both binaries and diffed every edge in the two databases:query_graph(mainintests/test_main.c->mcp)USAGE->src.daemon.application.cbm_daemon_application_session.mcpUSAGEedges ontoFieldWRITESedges ontoFieldAll 7,722 removed edges have a C caller and a
Fieldtarget. A random sample of 25 were all bare-name guesses: locals such asoffset,trait_qnandibound to other structs' members, and macro arguments such askeyinHT_LOOKED_UP(ht, key, found)bound tocbm_yaml_node.key. Nothing else changed. On a small fixture, designators (fixture_t f = { .count = 1 }),f.count = ...,f->countandoffsetof(fixture_t, count)produce the same edges as on main.Not verified locally
scripts/test.shvenue 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-onlyreports 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 -p1is empty). I'm relying on thelintCI job for the rest.Checklist
git commit -s):scripts/check-dco.sh upstream/main..HEADOKmake -f Makefile.cbm test): the 26 related suites above pass. I didn't do the full venue run (see above).make -f Makefile.cbm lint-ci): not run locally (see above).🤖 Opened with Superhuman, an open-source contribution agent.