Skip to content

fix: escape analysis — loop elements, (; v) shorthand, short-form nested defs - #56

Open
mgyoo86 wants to merge 10 commits into
masterfrom
fix/escape-binding-positions
Open

mgyoo86 wants to merge 10 commits into
masterfrom
fix/escape-binding-positions

Conversation

@mgyoo86

@mgyoo86 mgyoo86 commented Oct 24, 2026

Copy link
Copy Markdown
Member

The compile-time escape analysis treated every x = y node as an assignment, but = also appears in for heads, NamedTuple fields, and short-form f(x) = … definitions. That caused:

  • false escape warnings and errors, including a precompile failure reported from FastInterpolations;
  • one silent reuse of live pool memory;
  • a few escapes that were never reported.

This PR makes the analysis respect those syntactic positions.

Fixed

  • for k in pool_array: the element k was treated as a pool array.
  • (; s, v) block tail: the guard rewrite produced invalid syntax, which failed precompilation.
  • Short-form g(x) = (return …) nested in a scope: the return rewound the pool mid-scope (silent wrong result). Short-form defs also hit false PoolEscapeError / PoolMutationError, where long-form defs passed.
  • NamedTuple field names ((n = v,)) were treated as variables.
  • Missed escapes in function form: a (; v) tail, and acquire! wrapped in a literal or in identity, returned reused memory with no error.

Performance: no change. This is compile-time analysis only; acquire/rewind code is untouched.
Behavior: code that wrongly warned or errored now works. Function-form code that really escapes via (; v) or (acquire!(…), x) now raises PoolEscapeError.

@with_pool pool begin
    order = acquire!(pool, Int, n)
    contact = 0
    for k in order
        k == target && (contact = k)
    end
    (; contact)            # before: syntax error at expansion · after: works
end
@with_pool pool begin
    v = acquire!(pool, Float64, 3); v .= 1
    inc(x) = (return x + 1)
    n = inc(1)
    w = acquire!(pool, Float64, 3); w .= 2
    sum(v)                 # before: 6.0 (w reused v's memory) · after: 3.0
end
@with_pool pool function f(x)
    v = acquire!(pool, Float64, 3); v .= x
    (; v)                  # before: no error, returns reused memory · after: PoolEscapeError
end

Known limitation, documented: iterating a container variable (t = (v, w); for c in t) is not tracked at compile time. runtime_check = 1 catches it at scope exit.

Validated: the full suite passes on Julia 1.13 and 1.10, and test_compile_escape.jl passes on 1.11 and 1.12.

… checker detects (; v)

`(; s, v)` with a pool-backed `v` was rewritten to `(; s, EscapedPoolArray(v, …))`,
which is not a legal shorthand element ("invalid named tuple element" → precompile
failure). The poisoner now expands the slot to `v = guard`. The checker and literal
helpers ignored bare names inside `:parameters`, so the same shorthand escaped
undetected in function form; they now see it. Adds a checker ⇔ poisoner lockstep
corpus test.
…er taint as aliases

`for k in order` parses as `Expr(:for, :(k = order), body)`, and the taint walker
read the spec as an alias assignment, so an element read from a pool-backed array
(`contact = k`) was treated as an escaping pool array. Iteration specs (for and
generator/comprehension) now bind elements: only a container literal of pool arrays
taints the loop variable; a fresh loop variable shadows an outer tainted name; specs
are bound before the generator body. A `name = value` tuple child is a NamedTuple
field and no longer taints `name`.
…orm by escape/mutation walkers

The taint, explicit-return and mutation walkers skipped long-form `function`
bodies but not short-form `g(x) = …` (an `:(=)` node), so correct code failed to
expand: `g(x) = (return v)` raised PoolEscapeError and `g() = resize!(v, 0)`
raised PoolMutationError, while the long-form spellings passed. Reuses the
existing `_is_function_def` predicate.
… matching the guard rewrite

The block-tail poisoner already guarded acquire calls inside tuple/vect/NamedTuple
literals, but the checker never reported them: block form rewrote silently and
function form `(acquire!(pool, T, n),)` escaped with no PoolEscapeError. Adds the
`:literal_acquire_call` incidental kind (skipped when the literal also exposes an
acquired var, which the direct-exposure stage already reports once), and lets
`_tuple_contains_acquire_call` see `(; a = acquire!(…))`.
…enclosing pool scope

_transform_return_stmts skipped long-form `function` bodies but not short-form
`g(x) = …`, so `g(x) = (return x + 1)` got the scope's rewind injected: calling
`g` rewound the pool mid-scope and the next acquire reused live memory. The
`g() = (return v)` shape, a hard PoolEscapeError before c05e4dc, had become this
silent aliasing. Found in final review.
…r binding

Iterating a literal of pool arrays (`for c in (v,)`) tainted `c` for the body and
left it tainted after the loop, so an unrelated outer `c` (`c = 1.0 … c`, or
`push!(c, 1.0)`) was flagged after the loop — a guard rewrite, or a hard
PoolMutationError. A non-`outer` loop variable is a fresh local: its pre-loop
taint state is now restored exactly after the body. Found in final review.
…uard rewrite

`identity(...)` is transparent to the direct-exposure stage and the poisoner, but
the incidental-tail stage did not look through it: `identity(acquire!(…))`,
`(identity(acquire!(…)),)`, `identity((acquire!(…),))` and `identity(v .= x)` were
rewritten to guards without a warning (and escaped in function form). Adds
`_unwrap_identity` and uses it in `_incidental_exposure` and
`_tuple_contains_acquire_call`. Found in final review.
… note made precise

- @with_pool docstring, _check_compile_time_escape docstring, compile-time.md and
  the testset name said "three" incidental patterns; the literal-holding-an-acquire
  -call pattern (and identity unwrapping) is now listed with an example. The
  _check_compile_time_escape docstring also no longer claims Stage 1 always throws
  (block-form implicit tails follow ESCAPE_LINT since the guard rewrite).
- compile-time.md: with runtime_check = 1 the untracked loop shapes are caught by
  the scope-exit return-value check (PoolRuntimeEscapeError, also through
  Tuple/NamedTuple; verified on 1.13 and 1.10), not only by poisoning.
- Tests: two assertions added earlier moved out of the middle of existing
  assertion groups.
…everts 54d5d7d)

54d5d7d restored a loop variable's pre-loop taint after the body to stop a
same-named outer variable from being flagged after `for c in (v,)`. But the escape
and mutation checkers are flow-insensitive over the final taint set, so this hid
`return c` and `resize!(c, 0)` INSIDE the loop (both caught on master) — a silent
escape under the default runtime_check = 0. Back to the conservative behavior;
the outer-name false positive is pinned as a known limitation. Found by Codex review.
@codecov

codecov Bot commented Oct 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.57143% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 96.22%. Comparing base (505cf00) to head (dac8d08).

Files with missing lines Patch % Lines
src/macros.jl 98.57% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #56      +/-   ##
==========================================
+ Coverage   96.14%   96.22%   +0.07%     
==========================================
  Files          17       17              
  Lines        3996     4054      +58     
==========================================
+ Hits         3842     3901      +59     
+ Misses        154      153       -1     
Files with missing lines Coverage Δ
src/debug.jl 95.03% <ø> (ø)
src/macros.jl 97.61% <98.57%> (+0.14%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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