Skip to content

fix(elixir): guarded def heads, multi-clause spans, and typespecs walked as code - #2371

Open
BobbieBarker wants to merge 5 commits into
DeusData:mainfrom
BobbieBarker:fix/elixir-extraction-defects
Open

BobbieBarker wants to merge 5 commits into
DeusData:mainfrom
BobbieBarker:fix/elixir-extraction-defects

Conversation

@BobbieBarker

@BobbieBarker BobbieBarker commented Sep 26, 2026 •

Copy link
Copy Markdown

This PR was written by an AI agent working on my behalf. I certify the DCO sign-off on every commit and answer for the change.

Part of #2312. Split out of #2310 at your request.

This is the extraction half: three of the five defects in #2312, and the three that need no change to qualified_name. It carries no index-format bump and no identity change, so an existing store keeps resolving. #2312 stays open until the identity work follows in its own PR.

5 commits, 7 files, +1206/-11. Source is +428, tests are +778 across 11 new cases. It stands alone on current main: zero file overlap with #2370, and it deletes only 11 lines that exist on main.

What it fixes

A guarded def never reaches the graph. def f(x) when g parses its whole head as one when binary_operator, so extract_elixir_func_def finds no call or identifier where it looks and returns. A function whose clauses all carry guards gets no node at all, and search_graph answers "No nodes match", which reads identically to dead code.

A multi-clause function returns one clause. The node is anchored at the last clause, so get_code_snippet returns that clause as the whole function. A two-clause admin?/1 comes back as def admin?(%__MODULE__{}), do: false, so the graph asserts a predicate that always returns false.

A typespec is walked as code. @spec foo(t) :: t puts foo(t) in the same call node an invocation occupies, so the unified walk sources a reference from the enclosing module. On a codebase with @spec on every public function, fan_in never reaches 0 and "which exported functions does nothing call" cannot be answered.

Why these five commits are one PR

Two of them exist to close regressions the others open, and both pairs are inside this set.

fix(elixir): extract a def head wrapped in a when guard gives a guarded clause a node, and that node is where two pre-existing phantoms land that previously had nowhere to go. fix(elixir): open a function scope for a guarded clause body closes them.

fix(elixir): span a multi-clause function's node over all of its clauses widens the span to cover the @spec and @doc heads above the first clause, which the unified walk then reads as code. fix(elixir): stop walking @spec and @type subtrees as code closes that, and reports 0 CALLS edges anchored on a typespec-attribute line against 4,565 without it.

Splitting either pair ships a state where main is measurably worse than before. Both commit messages carry the counts on both sides.

Re-measured on current main

Every figure in these commit messages was taken against e783f73d, which is now 17 commits behind. I re-took them against 64c23fab on the same 970-file Elixir tree, darwin arm64. Two corrections, and one result the commit messages undersold.

Pristine CALLS is 55,851, not the 55,812 the commits cite. And fix(elixir): open a function scope for a guarded clause body no longer returns WRITES exactly to pristine: it lands at 11,207, thirteen below. That net is the residue of a much larger movement, 895 edges withdrawn and 882 added, which makes it a reattribution. The corrected sentence is above.

The undersold result is which node an edge is sourced from, which matters more here than the edge total. On main, a quarter of Elixir reference edges are attributed to a Module or a File rather than to the function that performs them, because a guarded clause body opened no function scope:

edge main: sourced from a Function after this PR
CALLS 42,659 of 55,851 (76.4%) 49,520 of 50,230 (98.6%)
USAGE 22,633 of 27,418 (82.5%) 31,456 of 31,765 (99.0%)
WRITES 8,600 of 11,220 (76.6%) 11,197 of 11,207 (99.9%)

Module-sourced CALLS fall from 11,061 to 541, and File-sourced from 2,131 to 169. So the CALLS total falling by 5,621 and Function-sourced CALLS rising by 6,861 are the same change seen twice: fewer edges overall, far more of them answering "which function does this". For 13,192 call sites the graph could name the file or the module but not the caller.

The corpus indexes deterministically: indexing pristine main twice gives identical counts on every metric above, so these deltas carry no noise.

Blast radius

Nine of the eleven deleted lines are on the Elixir path. The other two are in cbm_extract_unified, the walk every grammar runs, and the replacement is gated: is_elixir_typespec_attribute opens with if (ctx->language != CBM_LANG_ELIXIR || ...) return false;. No other grammar changes behaviour, and because the gate sits in the predicate's first line there is no call-site check to miss.

Verification

Full default suite, plain build, darwin arm64:

8201 passed, 0 failed, 10 skipped  (144 suites, 10 jobs)
Language-count contract passed (7 surfaces, 162 grammars vendored)
memory-core linter passed
[no-skips] OK
clang-format (clang-format-20) clean

11 new cases, each named in the commit that adds it:

elixir_def_head_is_covers_the_head_and_every_guard_above_it
extract_elixir_guarded_def_head
extract_elixir_guarded_def_head_guard_usages
extract_elixir_guarded_def_head_scope
extract_elixir_guarded_clause_head_is_not_a_self_call
extract_elixir_clauses_fold_only_when_adjacent_in_one_module
extract_elixir_nested_module_clauses_do_not_fold
extract_elixir_folded_clause_is_exported_is_the_or
extract_elixir_declaration_head_mints_no_reference_of_any_kind
extract_elixir_real_recursion_survives_head_suppression
extract_elixir_typespec_attribute_is_not_code

Two things this host could not run, so CI is the authority:

ASan aborts at sanitizer_malloc_mac.inc:189 on this macOS box for every suite including ones this change does not touch, so the run above is a plain build. The arena suite aborts identically on unmodified code, which is how I established it is the host.

cppcheck reports two warnings, src/mcp/mcp.c:870 and src/daemon/application.c:403. Both reproduce on pristine main at 64c23fab with the same two lines, and neither file is in this diff.

One step of the full run also fails on this host for an unrelated reason: Step 5e refuses to start because ten copies of the installed codebase-memory-mcp are live and the guard compares build hashes across running processes.

A note on the #1731 overlap you flagged

#1731 lands complexity, fingerprint and line count on Elixir functions, in the same region of extract_defs.c this PR edits, and GitHub currently reports it CONFLICTING against main. The interaction is not textual. It computes those values from a folded function's first clause, and this PR changes what a folded function's span is. pass_complexity.c then propagates loop_depth interprocedurally along CALLS edges, so a clause-1-only value does not stay on its node. Whichever of us lands second should recompute against the other's span rather than rebase the hunks. I am happy to take that side if you would rather merge #1731 first.

`def name(args) when guard` does not parse as a call with a guard beside it.
tree-sitter-elixir builds one `when` binary_operator for the whole head, and
the `name(args)` call that names the function is that operator's left operand.

Four walks read that same node, the def's first argument, and each asks a
different question of it:

  the defs walk names the function from it;
  the unified walk opens the function's call scope from it;
  the usages walk bounds the head's bindings with it;
  the calls walk tells a definition head apart from an invocation by it.

A private copy in one of them makes the rest disagree about what a guarded
clause is. The disagreement is silent, and it mints an edge: a head one walk
treats as a declaration and another does not is recorded as a call to the very
function being defined.

Two questions, so two exported entry points:

  cbm_elixir_def_head_unwrap_guard(first_arg) returns the head UNDER every
  guard, or the node unchanged when there is none. The three walks that need a
  name or a span take this.

  cbm_elixir_def_head_is(signature, node) is true for that head OR for any
  `when` operator above it. The calls walk takes this, because a guarded head
  reaches it by two routes and it sees only one node per visit: `def f(x) when
  g` arrives as the inner `f(x)` call, and `def f when g` has no inner call at
  all, so the `when` operator itself arrives. Recognising one route and not the
  other leaves the phantom in place for the other.

Both test the operator TOKEN rather than the node kind. `def a + b` is a
binary_operator head too, and peeling it would name the function after its own
left parameter.

This commit changes no extraction behaviour: nothing calls either helper yet.
The walks are separate commits, each with its own measurement.

Measured while writing the test, which corrects a claim worth not repeating:
tree-sitter-elixir nests multiple guards
RIGHT-associatively. `def f(x) when is_atom(x) when is_binary(x)` parses as
`when(f(x), when(is_atom(x), is_binary(x)))`, so the head is the outer
operator's left operand and one peel reaches it. The peel still runs to a fixed
point (one extra token compare), so the helper's contract is "the head under
every guard" even where one peel would do.

In tests/test_extraction.c, the test
elixir_def_head_is_covers_the_head_and_every_guard_above_it drives the helpers
on the AST directly rather than through cbm_extract_file. It has to: the two
routes into the calls walk are the pair a single-walk test cannot separate,
because each walk only ever sees the node its own traversal reaches. It pins
the unguarded case, both guarded shapes, the double guard, and that
`def a + b` is returned unchanged with its left operand answering false.

Signed-off-by: Chad <4307099+BobbieBarker@users.noreply.github.com>
extract_elixir_func_def reads the def's first argument directly and accepts
only a `call` or an `identifier` there. A guard makes that argument a
binary_operator, so the extractor found no name and returned. Every guarded
clause was dropped, and a function whose clauses ALL carry guards never reached
the graph at all. The loss is silent: a missing definition is not an error
anywhere in the pipeline.

Guards are ordinary Elixir. Measured by indexing one 970-file Elixir lib tree
with the binary built from e783f73 and with this one, and diffing the `nodes`
and `edges` rows out of the two SQLite stores:

  Function nodes          20,882 -> 21,941
  new qualified names      1,059  (functions with no node at all before)
  lost qualified names         0
  CALLS edges             55,812 -> 57,090
  USAGE edges             27,418 -> 28,223

Every one of those 1,059 is a function whose every clause carries a guard: a
clause without one already minted the node they collided on. An agent asking
"who calls this" about any of them got an empty answer that reads exactly like
dead code.

The unwrap goes in the shared helper. The calls, unified and usages walks read
the same node, and a private copy in one of them makes the rest disagree about
what a guarded clause is.

Two regressions come with the new node. Neither is minted here: both are
pre-existing phantoms that had nowhere to land while guarded clauses were
dropped, and now have a node.

1. A phantom CALLS edge onto the function being defined. The calls walk decides
   a def head is a declaration by comparing the visited node against the def's
   first argument, and a guard makes that argument the `when` operator, so the
   inner `name(args)` call no longer compares equal. cbm.c flags self-recursion
   by line containment, so the phantom lands inside the function's own span:

     self_recursive Function nodes       129 -> 1,342
     ...with no self-call form in span    24 -> 1,241  (18.6% -> 92.5%)

   (measured by reading each flagged node's own source span and looking for any
   non-declaration line that names it as a call, a capture or a pipe target.)

2. A phantom WRITES edge onto it, from a paren-less guarded head. `def f when g`
   has no inner call at all, so the `when` operator reaches the read/write
   extractor and its first identifier is recorded as an assignment target.
   Reproduced on `def bare_guarded when true, do: :ok`: one CBMReadWrite row,
   var_name "bare_guarded", is_write true, scoped to the file's Module node
   (the graph asserting that a module mutates a function). Corpus WRITES
   11,220 -> 11,302 (+82).

Both become observable with this commit, and the guarded call-scope commit later
in this stack closes them, by different mechanisms. The CALLS phantom goes
because that commit admits the `when` operator to the definition-container
check, so a paren-less head is recognised as a declaration. The WRITES phantom
goes because that commit opens a function scope for the clause: the read/write
row is still recorded, but its enclosing_func_qn becomes the guarded clause
itself rather than the enclosing module, so resolve_rw_edges finds
src->id == tgt->id and drops the self-edge. They are stated here because a
stack that lands partially must not hide them.

tests/test_extraction.c: extract_elixir_guarded_def_head covers every guarded
shape (one guard, two guards, a `defp`, a paren-less `def bare when true`) and
both operator-definition forms that must NOT be unwrapped (`def a + b`,
`def c - d when is_integer(c)`, neither of which is extracted, as before). It
fails on the current tree for each of the guarded names.

Depends on the shared def-head helper commit.

Signed-off-by: Chad <4307099+BobbieBarker@users.noreply.github.com>

@DeusData DeusData left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @BobbieBarker. This is exactly the split we hoped for. The cross-language pieces are out, and the one shared-walk change (is_elixir_typespec_attribute) is gated on CBM_LANG_ELIXIR in its first line, which made it easy to confirm that nothing else changes. The clause fold only looks at the previous definition, so there's no quadratic risk. The corpus numbers (2,303 phantom edges gone, none added, and typespec-anchored CALLS down to 0) and the disclosed costs are exactly what we like to see.

Two small things before merge:

  1. Stale comment. The block above elixir_call_is_definition_role in internal/cbm/extract_calls.c still says is_elixir_def_binding is "deliberately WIDER than the unwrap -- it covers the guard expression too". With this PR, extract_usages.c unwraps to the head, so guard reads now count as usages. Please update the comment to match the code, or tell us if we've misread it.
  2. Final-tip number. The self_recursive 129 → 349 figure in extract_defs.c was measured before the typespec skip. Could you give the count on the final tip?

On ordering with #1731: you're right that the two conflict semantically, since complexity comes from the first clause only. Thank you for offering to handle whichever lands second. We'll coordinate that on our side and let you know which goes first. Thanks again!

@DeusData

Copy link
Copy Markdown
Owner

@BobbieBarker, following up on ordering with #1731, as promised. This PR goes first. Our carry of #1731 (Elixir complexity and fingerprints) will rebase onto it afterwards, and we'll resolve the overlap on our side. You don't need to do anything for it beyond the two small items in the review above.

The reasoning: the two conflict textually in extract_defs.c and test_extraction.c, and in meaning, because complexity is computed from the first clause only. Once your clause fold lands, a multi-clause function is one node whose span covers every clause. That's the right unit for complexity to be summed over, so it makes sense for the complexity change to build on your fold rather than the other way round. Thank you again for offering to take either side of it!

@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.

Every clause of an Elixir function is its own `def` call, and an Elixir QN
carries neither module nor arity, so all clauses compute one qualified name and
collide on a single graph node. cbm_gbuf_upsert_node breaks a same-QN collision
by keeping the largest start_line, so the survivor was whichever clause was
written last, and get_code_snippet returned that clause as the whole function:

    def admin?(%__MODULE__{role: :admin}), do: true
    def admin?(%__MODULE__{}), do: false

read back as `def admin?(%__MODULE__{}), do: false`, so an agent asking what the
function does is told the predicate always returns false.

One node per function is right, so the fix widens that node's span instead of
splitting each clause onto its own node. Two bounds keep the widened span honest, and a clause folds only
if it clears both: it must sit in the same module body as the previous one (an
Elixir QN carries no module, so `Outer.run` and `Outer.Inner.run` compute the
same name), and it must be adjacent to it (only the immediately preceding
EXTRACTED definition is a candidate, so a def of another name ends the group).

Measured by indexing one 970-file Elixir lib tree with the binary built from
e783f73 and with this one, and diffing the `nodes` and `edges` rows out of the
two SQLite stores:

2,311 functions stop being reported as one line of source, which is the
get_code_snippet defect measured on this corpus.

A widened span is only honest if nothing phantom lives inside it, which is why
the head suppression in extract_calls.c lands with the fold rather than as a
follow-up. A
guarded clause head is a declaration of the name, never a call to it, and the
calls walk did not agree: it suppressed a head only when the head node IS the
def's first named argument, and `def f(x) when g` parses its whole head as a
`when` binary_operator, so the inner `f(x)` was not that argument. It now asks
cbm_elixir_def_head_is, the same helper the defs walk reads the head through.
cbm.c flags self-recursion by line containment, so under the folded span that
phantom would land inside the function's own node, and `recursive` is a
queryable node property that seeds the cycle detection in pass_complexity.

The widened span has a real cost. Reading each flagged node's own source span
for any line that names it as a call, a capture or a pipe target, and counting a
one-line clause's body after `, do:` as such a line:

  self_recursive Function nodes               129 -> 430
  ...with no self-call form in span             6 ->  48

The lines responsible are the typespec heads a widened span now covers: `@spec
f(t) :: u` puts the declared name in a `call` node and the unified walk reads it
as code. That is a separate defect with its own fix later in this stack, and a
fold cannot close it.

Two things a reader of the graph will notice:

  Edge metadata moves on edges that SURVIVE. `edges` is UNIQUE on
  (source, target, type), so where the def-head phantom happened to be the dedup
  survivor for a pair, deleting it promotes another row and the surviving edge
  takes that row's `line`. Of the 119,696 edges that survive this commit, 1,054
  do. The 2,436 that go are 2,104 CALLS and 332 USAGE, measured against this
  commit's parent, and none is added back.

  is_exported becomes the OR over the folded clauses. The arity-free QN puts a
  public entry point and its private accumulator on one node, and a name
  callable from outside must stay exported. Exactly 6 Function nodes change
  is_exported on this corpus, all false -> true and none the other way. Each is
  a `def` clause written above a `defp` clause of the same name, where the
  pre-patch last-clause-wins record reported a publicly callable name as
  unexported. Verified against the source in every one: the new value is correct.

The typespec attributes put a declared name in a `call` node too and are
deliberately NOT suppressed in the calls walk. A def head can be, because
is_elixir_def_binding already treats a def's first argument as a binding, so the
identifier handle_calls declines is declined by handle_usages too. Nothing
treats a typespec subject as a binding, so declining one here would not remove
its phantom, it would RELABEL it as a USAGE: a separate row under
UNIQUE(source, target, type) that no longer collides with the real call it used
to hide behind. extract_elixir_declaration_head_mints_no_reference_of_any_kind
pins that as a relation rather than a count, because at extraction the relabel
is invisible.

Depends on the shared def-head helper and guarded def-heads commits.

Signed-off-by: Chad <4307099+BobbieBarker@users.noreply.github.com>
A guard wraps the whole head in a `when` binary_operator, and three walks that
read a def's first argument were not expecting one. The three failures are
independent, and none is an error at runtime:

  The unified walk resolves a def's QN to open the function's call scope, and
  compute_elixir_func_qn accepts only a `call` or an `identifier` there. Handed
  a binary_operator it returns NULL, no function scope is pushed, and every call
  in a guarded body is attributed to the File node. The function reports no
  outgoing edges at all, and each of its callees gains an in-edge from the file.
  So "what does this function call" answers empty for a guarded clause, and "who
  calls this" answers with a file.

  The calls walk tells a definition head apart from an invocation through
  cbm_elixir_def_head_is, which covers the head and the `when` operators above
  it. A paren-less guarded clause (`def bare_guarded when true, do: ...`) never
  reaches that check: call_node_is_definition_container only admitted a `call`
  node, and here the head is the operator itself. Elixir's call node types
  include binary_operator, so the operator reached extract_callee_name and, with
  no callee of its own, took that function's last resort, the first identifier
  child, minting a phantom CALLS edge onto the very function being defined.

  The usages walk asks whether an identifier sits inside the def's signature and
  treats everything that does as a binding site rather than a reference. A guard
  makes that signature the whole `when` operator, which spans the guard
  expression too, so a parameter read by the guard was classified as its own
  binding and emitted no USAGE row at all. A guard is an expression over
  parameters that `guarded(a, b)` has already bound, so every mention of one to
  its right is a read, exactly as the same mention in the body already is.

Measured by indexing one 970-file Elixir lib tree with the binary built from
e783f73 and with this one, and diffing the `edges` rows out of the two SQLite
stores:

Counted as the cumulative effect of the first four commits against e783f73,
because this commit's attribution change is only observable once guarded
clauses have nodes to be attributed to:

  CALLS sourced from a Module node     11,053 -> 5,840   (-5,213)
  CALLS sourced from a Function node   42,629 -> 49,542  (+6,913)
  CALLS sourced from a File node        2,130 -> 1,269     (-861)

Calls written inside a guarded body stop being attributed to the enclosing
module and are attributed to the function that writes them. The File count
falls rather than rises, because the guarded-def-heads commit earlier in this
stack gives those functions a node to source from. 5,275 USAGE rows appear for parameters read inside
a guard that had been classified as their own binding. Node counts are identical
in the two stores: the change moves where an edge comes from and records reads
that were dropped.

This also closes the WRITES phantom disclosed in the guarded-def-heads commit,
though the extraction-level row does not go away. `def bare_guarded when true`
still records one CBMReadWrite with var_name "bare_guarded" and is_write true.
Its scope changes: with a function scope now open, enclosing_func_qn is the
guarded clause where it had been the enclosing module, so resolve_rw_edges finds
src->id == tgt->id and drops the self-edge. Measured on that exact fixture:
before, one row scoped to `t.store`; after, one row scoped to
`t.store.bare_guarded`. Corpus WRITES returns to 11,220. The row is still wrong
at extraction and would reappear as an edge the moment anything sourced it
elsewhere; removing it means teaching the read/write extractor about `when`,
which is a fourth walk and belongs in its own change.

tests/test_extraction.c: two tests. extract_elixir_guarded_def_head_scope
asserts call attribution by exact scope QN rather than by callee name, because a
call the scope walk failed to place carries the FILE QN, and a bare count of the
callee passes just as happily with every call homed on the file. The same test
pins the file-node count at 0. extract_elixir_guarded_def_head_guard_usages
asserts each name both within its scope and in total, so an unwrap that took the
guard instead of the head (admitting the binding sites themselves) fails on the
totals.

Only a `when` operator is admitted to the definition-container check. The
generic first-identifier last resort still overrides a deliberate NULL for every
other Elixir binary_operator that extract_scripting_callee declines (`=`, `<-`,
`->`, `\\`, `::`), so `def g(c \\ 2)` still emits a phantom `c` and
`e = target(d)` a phantom `e`. That is pre-existing behaviour for those
operators and is untouched here.

Depends on the shared def-head helper and multi-clause span commits.

Signed-off-by: Chad <4307099+BobbieBarker@users.noreply.github.com>
An Elixir module attribute parses as `unary_operator(@, call(<attr>, args))`,
so the typespec family reaches the unified walk in exactly the shape real code
has. `@spec foo(t) :: t` puts `foo(t)` in the same `call` node an invocation
would occupy, and `@type entry :: String.t()` does the same to a type name. The
walk then reads a declaration as code and mints reference edges from it,
sourced at the enclosing Module.

Measured on a fixture of three typespec lines (`@type state :: map()`,
`@spec fetch(opts) :: state`, `@spec load(User.t()) :: :ok`) beside `def state`,
`def opts` and a remote `MyApp.User.t/0`: seven edges, every one phantom and
every one landing on a Function. CALLS onto each specified function;
CALLS + USAGE + WRITES onto `state`, the WRITES asserting a mutation that does
not exist; and USAGE onto `opts` and onto `MyApp.User.t/0`, because a bare or
remote TYPE name resolves to the same-named FUNCTION.

On a codebase whose convention is @SPEC on every public function, every specced
function carries an inbound edge from its own module, so fan_in never reaches 0
and "which exported functions does nothing call" is unanswerable. Measured by
indexing one 970-file Elixir lib tree before and after: every specced function
loses the inbound reference its own typespec minted, and no node is added or
removed.

This is one of two mechanisms. It removes the phantom a typespec line mints;
the def-head phantom is removed by the calls walk recognising a `when`-wrapped
head as a declaration, earlier in this stack. A function carrying both an @SPEC
and a `when` clause needs both, which is why this commit is ordered after that
one and its test can assert 0 references onto such a function rather than
pinning a remaining 1.

The skip covers the whole subtree, and two costs come with that. Both were
measured by indexing a fixture repo (a declaring file per attribute head plus
the file each one reaches) with the pristine binary and with this one:

  A typespec's type references are not indexed at all. That loses nothing
  correct: a type declaration mints no node, so such a reference resolves onto
  nothing or onto a same-named FUNCTION, which is itself the pollution.
  `@spec build(MyApp.User.t())` where that module's `t` is a `@type` produced
  no edge in either build; the ones that did resolve landed on a same-named
  Function, cross-file included.

  @callback and @macrocallback lose a real relationship. The declared name is a
  function name, so it resolved cross-file onto the functions implementing the
  behaviour, and nothing replaces that edge: `@behaviour MyBehaviour` mints none
  of its own. It goes anyway because it is only name resolution: on a fixture
  with one @callback and three same-named `handle_it/1` defs, the single edge
  landed on the one module that implements nothing, leaving both real
  implementors at fan_in 0. Recording that relationship correctly means minting
  it from `@behaviour`, which is a separate change.

A typespec head a widened span covers was also being read as a self-call, which
is the other half of the cost the multi-clause span commit disclosed. Same
970-file tree, same method it used (each flagged node's own source span, counting
a one-line clause's body after `, do:` as a line that names the function):

  self_recursive Function nodes               430 -> 389
  ...with no self-call form in span            48 ->   8

The base is 129 and 6. All 8 that remain are the phantom a local variable
sharing the function's name mints (`defp slug(value) do slug = ...`), which
nothing in this stack touches: 6 are on the base as well, and the other 2 are
guarded clauses that only have a node of their own to be flagged on because of
the earlier commits here.

`unquote(...)` inside a typespec does execute at compile time, so
`@type t :: unquote(build_type())` loses its genuine `build_type` edge too.

Every other attribute VALUE is ordinary compile-time code
(`@timeout Application.compile_env(:app, :timeout)` really does call
compile_env), so those subtrees are deliberately walked exactly as before, and
their attribute NAME still mints one phantom onto a same-named Function. That
half of the defect class needs its own fix.

tests/test_extraction.c: extract_elixir_typespec_attribute_is_not_code
exercises all six heads in the skip list, so deleting any one entry breaks it.
It pins the two accepted costs as decisions, and it carries a control name
(`without_spec`) that reads 0 on the pristine build too, so an instrument that
scored a definition as a reference could not make the other zeroes vacuous.
extract_elixir_declaration_head_mints_no_reference_of_any_kind tightens from a
relation to 0: with both mechanisms present, a function with an @SPEC and a
guard carries no reference of any kind.

Depends on the guarded call-scope commit, for the ordering above. It touches no file
the earlier commits in this stack touch except extract_unified.c, and none of
their lines.

Signed-off-by: Chad <4307099+BobbieBarker@users.noreply.github.com>
@BobbieBarker
BobbieBarker force-pushed the fix/elixir-extraction-defects branch from 0dba492 to eafaab1 Compare September 27, 2026 01:00
@BobbieBarker

BobbieBarker commented Sep 27, 2026 •

Copy link
Copy Markdown
Author

You read it correctly on both counts, and the second one turned up two more numbers that needed fixing.

1. The stale comment

The unwrap in is_elixir_def_binding arrived in the guarded-call-scope commit, two commits after the one carrying that comment, and that commit edited extract_calls.c without touching the paragraph describing it. Fixed in the commit that makes it stale, so each commit still describes its own tree. The third bullet in the same block had gone stale the same way: compute_elixir_func_qn reads the head through the same helper now, so "does NOT unwrap ... fixed separately" was wrong at the tip too.

What it says now: all three walks find the head through one unwrap chain in helpers.c, and they ask different questions of it. This file asks whether the node IS the head, or a when wrapper above it; the usages walk asks whether the node sits INSIDE the head. The containment stops at the head deliberately, so only the parameters being bound are excluded and a parameter read inside the guard is a reference, exactly as the same read in the body already is.

2. The final-tip count

389, with 8 carrying no self-call form. Two corrections come with that.

The 129 → 349 pair was measured on the pre-split chain, where the typespec skip sat earlier, so it describes no commit in this PR.

My detector was also wrong. It skipped every line starting with def, so def prepare(c), do: prepare(c, config.workspace.root) counted as carrying no self-call form when the delegation is on that line after , do:. Reading that body takes the base from 24 to 6.

Per commit on this stack, same 970-file tree, corrected detector:

commit Function nodes self_recursive no self-call form
64c23fab (base) 20,882 129 6
read a def head through its guard 20,882 129 6
extract a def head wrapped in a guard 21,941 1,342 1,235
span a multi-clause function's node 21,941 430 48
open a function scope for a guarded clause 21,941 430 48
stop walking @SPEC and @type as code 21,941 389 8

All 8 at the tip are the phantom a local variable sharing the function's name mints, as in defp slug(value) do slug = .... 6 of them are on the base as well; the other 2 are guarded clauses that only have a node of their own to be flagged on because of this stack. That phantom comes from the = operator's first-identifier last resort, which the guarded-heads commit already discloses and which nothing here touches.

I dropped the @doc half of the stated cause. After the typespec skip the residue holds no @doc-driven flag, so @SPEC was doing the work; a @doc heredoc is a string, not a call node.

One intermediate matters if you read commit by commit: at the guarded-def-heads commit the count peaks at 1,342, because that commit gives 1,059 guarded clauses nodes of their own and the head suppression lands in the commit after it. A bisect that stops there sees the worst state in the stack. The figures now sit in the commit that produces them.

3. A number you did not ask for

Re-measuring exposed that 2,303 was taken on the pre-split chain too, where the def-head extraction sat elsewhere. Measured against its own parent on current main, the fold and the head suppression remove 2,436 edges, 2,104 CALLS and 332 USAGE, and add none. 1,054 of the 119,696 survivors record a different line.

The CONFIGURES edge in that sentence is gone: 64c23fab mints no CONFIGURES edge on this corpus at all, so it is not in the removed set any more. The shape of the claim is unchanged, nothing is added back, and the "6 Function nodes change is_exported, all false → true" figure reproduces exactly. The comments and the commit message carry the new numbers.

While I was in there I re-checked every figure in this PR's description against a fresh index of 64c23fab and of the tip. All of them reproduce: 55,851 base CALLS, the -5,621 and +6,861, 11,220 → 11,207 WRITES, and 11,061 → 541 and 2,131 → 169 for Module- and File-sourced CALLS. The description needs no correction.

4. Ordering

Understood on #1731 going second, and thank you for taking the overlap. Nothing needed from me there.

The stack is force-pushed. Every change in it is a comment or a commit message; the diff against the version you reviewed contains no non-comment line.

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.

2 participants