fix(nargs): count args on the innermost C declarator

[dekobon]

Fixes #1200
Fixes #1210

C, C++, Mozcpp and Objective-C functions whose return type is a pointer or a reference reported nargs.function_args = 0 regardless of their real arity — 7.9% of the matched functions in the DeepSpeech/kenlm C++ corpus. Since #1196 made the nargs gate read a callable's own parameter count, every one of them was also invisible to bca check --threshold nargs=N.

The issue's prescribed fix would have shipped half the bug

Both the issue body and its resolution plan specify walking the declarator field down to a node carrying parameters. Read against the pinned grammars' node-types.json, that repairs the pointer cases and leaves every reference case at 0 — one of the shapes the issue itself lists and demands fixtures for:

rule	declarator field
pointer_declarator	required
function_declarator	required
abstract_function_declarator	optional
reference_declarator	absent — no fields at all
parenthesized_declarator	absent — no fields at all
attributed_declarator	absent — no fields at all

In reference_declarator and parenthesized_declarator the inner declarator is a plain named child with no field name, so a field-only descent stops dead.

The rule that shipped

Walk the declarator chain and take the innermost node carrying a parameters field:

step(n):
    n.child_by_field_name("declarator")   -> that child
    else if n has a "parameters" field    -> None   (chain ends)
    else                                  -> last named non-comment non-attribute child

…cut at any operator_cast, and seeded from the declarator field rather than the function node.

Every clause is load-bearing:

• Field first, per grammar-dispatch §3, which also sidesteps §1 — PointerDeclarator2, FunctionDeclarator2/3, ReferenceDeclarator2/3/4 are aliases a kind_id match would have to enumerate.
• Stop on a parameters-bearing node with no declarator field — the C++ lambda. abstract_function_declarator.declarator is optional, so [](int a, int (*cb)(int x)) would otherwise return cb's (int x): 1 instead of 2.
• Last, not sole or first, named child — int (__cdecl *w(int a, int b))(int c) puts a real ms_call_modifier ahead of the declarator.
• Excluding attribute_declaration — attributed_declarator is the one fieldless rule putting its declarator first, so without this int f(int a, int b) [[deprecated]] reports 0. The GNU __attribute__((…)) spelling was never affected: all four grammars absorb it into the function_declarator instead of wrapping it.
• Cutting at operator_cast — its declarator field is the type the operator converts to. A conversion operator takes no arguments however many its target type has.

Innermost, not first, is what also fixes a function returning a function pointer. int (*fp(int a, int b))(int c) had reported 1 — the return type's list — where fp takes 2. The issue's plan proposed a FIXME for this; the rule gets it for free.

Consolidated rather than patched in place

CppCode, CCode and MozcppCode carried three byte-identical NArgs::compute overrides — which is why one bug existed in triplicate. All three are gone, replaced by a one-line params_owner, the hook whose own doc comment says re-stating compute is the drift #1142 and #1162 were filed about. The default compute now routes closures through it too (a no-op elsewhere: only JavaCode overrides it, and only for record compact constructors). This also removed CCode's dead closure branch — CCode::is_closure is a constant false.

ObjcCode keeps its own compute (three unrelated parameter shapes) with only the FunctionDefinition arm rerouted.

Also fixed: C's (void) marker (#1210)

int f(void) declares no parameters, but the grammar emits a real parameter_declaration for the void and every filter counted it, so f(void) and f(int) both reported 1. The distinction needs the source bytes — an unnamed parameter is the same shape and really is one argument — so count_args now threads &[u8] to a new Checker::is_empty_param_marker, defaulting to false and overridden by the four C-family grammars through one shared helper. void *p keeps counting: it carries a declarator.

This is an independent behaviour change in its own commit (7253f144), reviewable and revertable apart from the rest.

Verification

Seven perturbations, each applied to one clause alone and run against the whole lib suite, each producing a disjoint failure set:

perturbation	failures
field-only descent (the plan's fix)	22/30 — every pointer, reference and nested row
first instead of innermost	the 4 nested fp rows
drop the lambda stop clause	the 2 lambda-guard rows
first instead of last named child	the 4 __cdecl rows
start off the declarator chain	the 2 local-declaration rows
drop the operator_cast cut	the 4 conversion-operator rows
drop the attribute_declaration exclusion	the 3 [[deprecated]] rows
drop the (void) exclusion	the 4 (void) rows
structural-only (void) check	the 4 unnamed rows instead

Two of those fixtures only became real after the selector was fixed, and both times the selector was the weak point rather than the assertion:

• The parameterless-lambda row I added to close a coverage gap gave 100% region coverage of the walk's entry guard and failed 0 of 3,322 tests when that guard was perturbed. Two further candidate fixtures also failed to fail — the declarator fields keep pulling the walk back onto declarator-shaped nodes, which is precisely why the guard is safe. The row that discriminates puts a local function declaration last in the lambda body.
• Both operator_cast fixtures passed with the regression reinstated, because sole_space_args read the first nested space and a conversion operator sits two levels down, behind its struct's container space. The helper now descends to the innermost sole space.

Also probed as paired plain/pointer variants: constructors, destructors, operator=, templates, [[nodiscard]], trailing return types, variadics, const-qualified pointer returns, 2000-level pointer nesting (0.00 s — the walk is an iterative successors unfold, not recursion), and malformed input.

Drift

• fix(nargs): C/C++ pointer/reference-return functions report nargs = 0 #1200: 3,336 nargs values rise across 276 DeepSpeech snapshot files. Nothing falls; no other metric key moves.
• fix(nargs): C's (void) marker is counted as a parameter #1210: 24 values fall to 0, all in pywrapfst.cc, the corpus's only (void) definitions.

Submodule accepted, committed and pushed (1b032d7c), with the SHA recorded in the parent commits alongside each fix.

Follow-ups filed

• fix(getter): C-family function returning a function pointer resolves its space name to null #1208 — the same nested shape resolves its space name to null. A getter bug in a different metric surface; pinned in CI here by a_parenthesised_declarator_still_loses_its_space_name, which fails the day it is fixed.
• fix(c): K&R function with a pointer return type opens no function space (upstream grammar) #1209 — a K&R function with a pointer return opens no function space at all. Upstream: tree-sitter-c parses it as a declaration, so there is no function_definition for is_func to match and nothing in our wrapper to fix.

Gates

make pre-commit → BCA_GATE: pass. make bench-scaling → all 26 probes within their complexity bound. No public-API change (NArgs is pub(crate) + #[doc(hidden)]), so no STABILITY.md cross-reference is needed.

[codecov]

Codecov Report

❌ Patch coverage is 98.14126% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.30%. Comparing base (dbc24cf) to head (690cc31).

Files with missing lines	Patch %	Lines
src/metrics/nargs.rs	97.97%	3 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1211      +/-   ##
==========================================
+ Coverage   98.27%   98.30%   +0.03%     
==========================================
  Files         276      276              
  Lines       71658    71858     +200     
  Branches    71228    71428     +200     
==========================================
+ Hits        70423    70643     +220     
+ Misses        818      802      -16     
+ Partials      417      413       -4     

Flag	Coverage Δ
rust	98.29% <98.14%> (+0.03%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/checker.rs	96.21% <100.00%> (+0.04%)	⬆️
src/checker/c.rs	89.65% <100.00%> (+1.19%)	⬆️
src/checker/cpp.rs	90.00% <100.00%> (+1.11%)	⬆️
src/checker/mozcpp.rs	90.00% <100.00%> (+1.11%)	⬆️
src/checker/objc.rs	87.09% <100.00%> (+1.38%)	⬆️
src/metrics/nargs.rs	99.41% <97.97%> (+0.13%)	⬆️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[dekobon]

Review: PR #1211 (fix/1200-c-family-nargs-declarator, 10 commits)

Full review per the review skill checklist, plus independent empirical verification of the walk against parse shapes outside the committed fixture tables. Verdict: APPROVE WITH COMMENTS — one trivial docs fix, two optional test-gap suggestions, no bugs found.

Independently verified

• The walk's correctness beyond the fixtures. Probed with the branch binary against shapes the tables don't pin, all correct:
  • Out-of-line qualified conversion operator S::operator int (*)(int x) { … } → 0. The AST routes it as function_definition.declarator = qualified_identifier whose last named child is the operator_cast, so the take_while cut fires via the fallback path, not the seeded field — a different chain than the in-class fixtures exercise.
  • Trailing-return function pointer auto f(int a, int b) -> int (*)(int c) → 2 (the stop-on-parameters clause keeps the walk out of the trailing return).
  • Lambda with fn-pointer trailing return [](int a) -> int (*)(int q) → closure 1.
  • (void) lambda [](void){ … } → closure 0 — a shape the fix handles for free because the closure channel now routes through the same count_args/marker path.
  • Redundant parens int (f)(int a) → 1; int *f(int a) noexcept → 1; comment inside the marker int f(/* nothing */ void) → 0; 50-level pointer nesting → instant; truncated int *f( → no panic, no space.
• One perturbation replicated. Removed the operator_cast take_while cut and re-ran: exactly 4/61 rows fail — the two conversion-operator fixtures × {Cpp, Mozcpp} — matching the verification table's claimed failure set. Restored; tree clean; full nargs module (149 tests) green.
• Submodule drift matches the claims: 89e4c973..28dd2dec touches 276 files (fix(nargs): C/C++ pointer/reference-return functions report nargs = 0 #1200); 28dd2dec..1b032d7c touches only the two pywrapfst.cc snapshots (fix(nargs): C's (void) marker is counted as a parameter #1210). The recorded SHA is on the submodule's origin/main.
• The params_owner doc inventory is exact: compute-overriders are precisely Objc, Go, Kotlin, Lua, Tcl, iRules, Perl, Elixir, Groovy; the only other params_owner override is JavaCode, and it is kind-guarded (CompactConstructorDeclaration), so routing the closure channel through params_owner cannot misfire on Java lambdas.
• CHANGELOG entries sit inside [Unreleased] (section spans lines 25–1546; the edited ### Fixed is at 767).
• [crate::nargs] doc link resolves (the module is re-exported; parser.rs/traits.rs already use that path). Worth knowing it would not have been caught if broken: Checker is pub(crate), so the cargo doc -D warnings gate never sees its links.
• The vacuity-guard asserts (shared > 0 && …) can't fire on the feature-matrix CI legs — those run cargo check --all-targets only — and the pattern matches the established fix(nargs): comments inside parameter lists are counted as parameters #1201 module convention.

Docs

#	Finding	File	Effort
1	The (void)-marker CHANGELOG entry cites no issue number. The sibling entry ends its first sentence with “(#1200)”; this one never mentions #1210, though the PR closes it and the commit message references it.	CHANGELOG.md (the ### Fixed entry beginning “C's (void) marker…”)	trivial

Test gaps (optional; non-blocking)

#	Finding	File	Effort
2	No committed fixture pins the out-of-line/qualified conversion operator. It works today, but through a different chain than the two in-class fixtures (the cut fires off qualified_identifier's last-named-child fallback rather than off the seeded declarator field), so a grammar bump that reshapes qualified operator-cast parsing could regress it while both existing operator_cast rows stay green. One row in cpp_only_shapes covers it: "struct S { operator int (*)(int x); }; S::operator int (*)(int x) { return nullptr; }" → (0, 0).	src/metrics/nargs.rs (cpp_only_shapes)	trivial
3	Three shapes the PR narrative says were probed but that no committed row pins: trailing-return fn-pointer (auto f(int a, int b) -> int (*)(int c) → 2 — exercises the stop clause from the function channel, which no current fixture does), the (void) lambda ([](void){} → closure 0 — the marker on the closure channel), and int (f)(int a) → 1. All verified correct on this branch; the tables make adding rows cheap.	src/metrics/nargs.rs	trivial

Pre-existing observations (not introduced here, no action needed)

• int v(int a, ...) reports 2 — the ... counts as a parameter. Unchanged by this PR (the plain-return path counted it identically before), and the fix keeps plain/pointer variants consistent, but it's a semantic choice nobody has pinned in a test or doc.
• The probes reproduce the known name-resolution gaps the PR already tracks: int (f)(int a) and the out-of-line conversion operator both report name: null (fix(getter): C-family function returning a function pointer resolves its space name to null #1208 family), and K&R with pointer return opens no space (fix(c): K&R function with a pointer return type opens no function space (upstream grammar) #1209).
• The PR body's perturbation table row counts (e.g. “22/30”) reflect the table size at the commit each perturbation ran against; the final suite has 61 rows. Narrative only.

Checklist areas with nothing to report

Correctness of the marker predicate (void *p, unnamed int, optional_parameter_declaration, variadic_parameter all correctly non-markers; byte read is bounds-safe via code.get), termination (iterative successors, strictly descending), performance (O(depth + width) per function node, no allocation), security (no panics on malformed input), API surface (NArgs is pub(crate) + #[doc(hidden)], no STABILITY.md impact), and the code-threading changes to Kotlin/Perl/Elixir/Groovy are pure plumbing (marker defaults to false).

Summary

• Files reviewed: 8 (all, in full) + submodule diff
• Findings: 1 docs (trivial), 2 test-gap (optional)
• Verdict: APPROVE WITH COMMENTS