Skip to content

fix(cxx): respect C and C++ compilation languages in AST analysis - #910

Open
aasul wants to merge 1 commit into
peteromallet:mainfrom
aasul:feat/c-native-analysis
Open

aasul wants to merge 1 commit into
peteromallet:mainfrom
aasul:feat/c-native-analysis

Conversation

@aasul

@aasul aasul commented Oct 29, 2026 •

Copy link
Copy Markdown

Problem and reproduced behavior

The existing cxx plugin binds its Tree-sitter AST phases to CPP_SPEC for every discovered file, including native C. This produces two demonstrated defects in valid C17 code:

  • A callback named throw is an ordinary C identifier. In void dispatch(void (*throw)(int), void (*cleanup)(void)) { throw(1); cleanup(); }, the C++ grammar interprets the call as a throw statement and reports the reachable cleanup() call as unreachable. Both grammars accept the source without syntax errors, so parse-error fallback cannot disambiguate it.
  • A C function named mutable is silently omitted by the C++ function query. The cohesion regression fixture consequently yields seven functions instead of eight, dropping below the detector's minimum and suppressing an otherwise valid cohesion finding.

Dispatch also needs to preserve existing C++ behavior for a .c translation unit explicitly compiled as C++. For example, void f() { throw(1); f(); } compiled with -x c++ -std=c++17 must retain the unreachable-code finding after the throw. Selecting C solely from the extension would introduce the inverse false negative; both arguments and command compilation-database regressions cover this case.

Implementation

  • Reuse the existing Cxx compilation-database reading/tokenization/path-resolution logic through a shared normalized loader used by dependency analysis and AST selection. Relative command directories are resolved from the database root, then source paths from the command directory; malformed entries do not discard valid peers.
  • Prepare source-to-grammar selection once per AST phase invocation, with no per-file database reads or global compilation-context cache. Without matching metadata, .c uses C and ordinary C++ sources use C++. Headers, including .h, retain their established C++ parsing policy.
  • Recognize GCC/Clang -x modes, joined forms and -x none, supported MSVC /TC//TP and per-file /Tc//Tp forms, recognized C++ driver defaults, simple compiler-cache wrappers, and Clang --driver-mode. Do not infer input language from -std= alone; driver/override behavior was checked with installed compilers.
  • Add optional per-file selection to the existing shared AST-smells and cohesion factories. Partition runtime-discovered files by specification identity, then aggregate entries/counts within each existing phase. Identity grouping preserves specs whose resolver callables are unhashable. The convenience bundle and unused-import APIs remain unchanged; Cxx still does not run unused-import analysis.
  • For conflicting C/C++ configurations of one normalized source, deterministically retain the previous C++ interpretation, warn that only that context is represented, and mark affected findings low confidence with both configured languages in their details. Equivalent commands do not duplicate findings or reduce confidence.

There is no separate C plugin, parser framework, or new package dependency.

Validation

Executed locally on Python 3.14 / Linux with Tree-sitter 0.26.0 and tree-sitter-language-pack 1.6.2. Parser-dependent runs used an already-populated temporary grammar cache via XDG_CACHE_HOME; all Python commands used the project virtualenv.

Command Result
.venv/bin/python -m pytest desloppify/languages/cxx/tests -q 123 passed
.venv/bin/python -m pytest desloppify/tests/lang/common -q 466 passed, 2 skipped
Focused suites plus CI contract tests 598 passed, 2 skipped
.venv/bin/python -m pytest -q 6,902 passed, 18 skipped, 2 failed; baseline failures below
.venv/bin/python -m ruff check . --select E9,F63,F7,F82 Passed
Default Ruff checks on changed helper/detector/test modules and shared phases; formatting checks on new helpers/tests Passed
git diff --check Passed

The focused Cxx/shared suites were rerun immediately before committing: 589 passed, 2 skipped. Coverage includes both DB representations, native C-only and mixed projects, C++ source/header preservation, relative paths and quoted filenames, language-option precedence, malformed metadata, independent scan roots and metadata edits in one process, conflict handling, parser-cache separation, missing optional parsers, scan integration and per-phase DB load counts.

C17/C++17 syntax checks of the reproducers passed, including clang-cl checks with C/C++ modes. CLI scans confirm that native C retains only the genuine finding after return, while the explicit C++ case retains the finding after throw; each has unreachable potential count 1. The conflicting-context scan produces the documented warning and low-confidence result. These scans had reduced external-tool coverage because cppcheck was unavailable; they are evidence for the AST changes, not a claim of complete native analysis.

Known baseline full-suite failures (not fixed here)

Both failures were independently reproduced by running the exact tests against untouched upstream base 3a7735d:

  1. desloppify/tests/review/integration/test_review_commands.py::TestCmdReviewPrepare::test_do_run_batches_dry_run_generates_packet_and_prompts
  2. desloppify/tests/review/test_review_commands.py::TestCmdReviewPrepare::test_do_run_batches_dry_run_generates_packet_and_prompts

Both assert that the first generated review prompt contains Previously flagged issues; the assertion fails on the base and this branch. They remain disclosed failures, not xfailed, skipped or weakened tests. The previous full-suite baseline was 6,845 passed, 18 skipped and these same two failures; the additional 57 Cxx cases account for the increase to 6,902 passing tests.

Remaining limitations

  • Header language is not inferred from importing translation units, and preprocessing/build configurations are not simulated.
  • Conflicting C/C++ source configurations are disclosed but only the C++ context is analyzed, so C-specific findings can remain incomplete in those files.
  • Response files, forwarded frontend flags, arbitrary wrapper/configuration options, shell expansion, cross-host command syntax and languages outside the supported C/C++ subset are not interpreted. Windows command strings retain the existing tokenizer's quoting limits; prefer arguments. MSVC option behavior was checked using clang-cl on Linux, not a native Windows installation.
  • Metadata is read from the scan root's compile_commands.json; no new ancestor/build-directory discovery was added.
  • Regex callback/complex-declarator extraction and Cxx structural complexity signals are separate paths and are not claimed fixed by this change.

Description written with claude.

Use the C grammar for native C AST analysis while honoring compilation-language overrides and recognized compiler-driver defaults. Reuse compilation-database normalization for dependency analysis and prepare grammar selection per phase invocation.

Preserve shared phase aggregation and header behavior, disclose conflicting compilation contexts, and cover C keyword identifiers, explicit C++ compilation, path normalization, optional parsers, and scan isolation with regression tests.
github-actions Bot added a commit to citizenadam/desloppify that referenced this pull request Oct 29, 2026

This branch has not been deployed

No deployments
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