From d07a35a33cffcfd9f4f03e1a61b6a5d380a61298 Mon Sep 17 00:00:00 2001 From: sotashimozono Date: Wed, 9 Sep 2026 05:48:13 +0000 Subject: [PATCH 1/2] fix: reach did not terminate on nightly, and depth alone was never going to bound it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My own regression, and the way it reached main is worth writing down. #29 removed a `MethodError` that `reach` threw on `sum(f(x) for x in xs)`. That throw was also, accidentally, a terminator. Without it, on 1.14.0-DEV: reach(f, Tuple{Vector{Float64}}) # [f(x) for x in xs] — did not return reach(g, Tuple{Vector{Float64}}) # sum(map(f, xs)) — did not return Both answer in milliseconds on 1.12.2. The nightly leg is `continue-on-error`, so it could not block the merge; I then cancelled the run that was sitting in `runtest` and a background job merged #29 the moment `gh pr checks` reported nothing pending. The hang was on main for about forty minutes. `maxdepth` bounds how FAR the walk goes, not how much of it there is. Thirty-two levels branching by sixteen candidates is not a finite amount of work in any useful sense, and `visited` only prunes signatures that repeat — a higher-order call generates new ones. So the walk now also carries a `maxwork` budget, shared with every subwalk, and spends `:unknown` with `why = :budget` when it runs out. That is what `:unknown` is for; the alternative was a call that never comes back. Shared, not per-branch, and the distinction is load-bearing: `visited` is deliberately reset in `_subwalk` so a candidate reached under another branch is still walked here, which means `visited` cannot also be the thing that bounds the total. `maxwork` is a keyword on `reach`, `reach(::Module)` and `reach_script`, with the measurement in the docstring — a caller whose entry point comes back `:unknown` with a `:budget` is in a different situation from one that is genuinely dynamic, and only they can decide to pay for more. The spec now pins the property that does not move between versions, because the verdict does: `[unstable(x) for x in xs]` is `:depends` on 1.12.2 and `:unknown` on 1.14.0-DEV. What must hold everywhere is that a caller which can reach a mark is never reported `:clean` — with a control that the same shapes with nothing marked behind them still are, so the assertion is not satisfied by an analysis that never says `:clean` at all. Measured on both: 1237 assertions on 1.12.2, 1236 on 1.14.0-DEV, 190 behaviours, green. Co-Authored-By: Claude Opus 5 --- src/reach.jl | 58 +++++++++++++++++++++++++------- test/spec/README.md | 4 +-- test/spec/test_spec_propagate.jl | 58 +++++++++++++++++++++++++------- 3 files changed, 94 insertions(+), 26 deletions(-) diff --git a/src/reach.jl b/src/reach.jl index 32956fd..c95ac10 100644 --- a/src/reach.jl +++ b/src/reach.jl @@ -33,7 +33,7 @@ One call site the analysis could not pin to a method. |---|---| | `callee` | the name being called, as far as the IR knows it | | `signature` | what it was called with — the widened argument types | -| `why` | `:dynamic`, `:ambiguous`, `:maxdepth`, `:splat` or `:nomethod` | +| `why` | `:dynamic`, `:ambiguous`, `:maxdepth`, `:budget`, `:splat` or `:nomethod` | | `file`, `line` | where to go and look | | `within` | the method the call site is in | | `candidates` | the marked methods this site *could* reach, if any are visible | @@ -170,9 +170,13 @@ mutable struct _Walk modules::Vector{Module} marked::Dict{Method,Union{Mark,Nothing}} truncated::Bool + # Work left to spend, SHARED with every subwalk. `visited` is per-branch by design — a + # candidate reached under another branch still has to be walked here — so it cannot also be + # the thing that bounds the total. + budget::Base.RefValue{Int} end -function _Walk(maxdepth::Int, ignore, maxcandidates::Int=16) +function _Walk(maxdepth::Int, ignore, maxcandidates::Int=16, maxwork::Int=20_000) return _Walk( Base.get_world_counter(), maxdepth, @@ -184,11 +188,12 @@ function _Walk(maxdepth::Int, ignore, maxcandidates::Int=16) Module[], Dict{Method,Union{Mark,Nothing}}(), false, + Ref(maxwork), ) end """ - reach(f, types::Type{<:Tuple}; maxdepth = 32, ignore = Symbol[]) -> Reach + reach(f, types::Type{<:Tuple}; maxdepth = 32, maxwork = 20_000, ignore = Symbol[]) -> Reach reach(m::Module; kwargs...) -> Reach Report whether calling `f` with `types` can reach anything declared [`@experimental`](@ref) — @@ -203,9 +208,15 @@ The module form folds every public entry point of `m` into one answer and report are affected in `affected_entries`: function-by-function does not scale to a package. `ignore` names marks to treat as absent, which answers "what would removing this mark change?" -without removing it. `maxdepth` bounds the walk and `maxcandidates` bounds how many methods a -single ambiguous call site is willing to check; hitting either bound is reported as `:unknown`, -never as `:clean`. +without removing it. + +Three bounds keep the walk finite, and hitting any of them is reported as `:unknown`, never as +`:clean`. `maxdepth` bounds how far it goes, `maxcandidates` bounds how many methods a single +ambiguous call site is willing to check, and `maxwork` bounds the total — because depth alone does +not: 32 levels branching by 16 is not a finite amount of work, and the signatures a higher-order +call generates need not repeat. Measured on 1.14.0-DEV, `[f(x) for x in xs]` did not return +without `maxwork`, while the same call answers in milliseconds on 1.12. Raise it for a large entry +point that comes back `:unknown` with a `:budget` in `unresolved`. # What it can and cannot resolve @@ -233,10 +244,11 @@ function reach( @nospecialize(types::Type); maxdepth::Int=32, maxcandidates::Int=16, + maxwork::Int=20_000, ignore=Symbol[], ) sig = Base.signature_type(f, types) - st = _Walk(maxdepth, ignore, maxcandidates) + st = _Walk(maxdepth, ignore, maxcandidates, maxwork) matches = _matching_methods(st, sig) (matches === nothing || isempty(matches)) && throw( ArgumentError( @@ -252,8 +264,10 @@ end similar_entries() = NamedTuple{(:name, :verdict),Tuple{Symbol,Symbol}}[] -function reach(m::Module; maxdepth::Int=32, maxcandidates::Int=16, ignore=Symbol[]) - st = _Walk(maxdepth, ignore, maxcandidates) +function reach( + m::Module; maxdepth::Int=32, maxcandidates::Int=16, maxwork::Int=20_000, ignore=Symbol[] +) + st = _Walk(maxdepth, ignore, maxcandidates, maxwork) entries = similar_entries() for n in surface(m) isdefined(m, n) || continue @@ -272,7 +286,7 @@ function reach(m::Module; maxdepth::Int=32, maxcandidates::Int=16, ignore=Symbol # A FRESH walk per entry point. Sharing one `visited` set across them would make the # second entry that reaches a marked definition through an already-walked callee look # clean — the mark is real, it was simply reported under the first entry's name. - own = _Walk(maxdepth, ignore, maxcandidates) + own = _Walk(maxdepth, ignore, maxcandidates, maxwork) for mm in ml mm.module === m || continue _enter!(own, mm, 0, Symbol[n]) @@ -325,7 +339,11 @@ the script uses; everything else is analysed as one thunk. So this **loads the s dependencies**, and a script whose top level has side effects will have them. """ function reach_script( - path::AbstractString; maxdepth::Int=32, maxcandidates::Int=16, ignore=Symbol[] + path::AbstractString; + maxdepth::Int=32, + maxcandidates::Int=16, + maxwork::Int=20_000, + ignore=Symbol[], ) isfile(path) || throw(ArgumentError("reach_script: no such file: $path")) ex = Meta.parseall(read(path, String); filename=path) @@ -355,7 +373,7 @@ function reach_script( # "Detected access to binding … in a world prior to its definition world" — and says it will # be an error in a future version. The analysis reads globals out of the IR, which is what # makes this the one place in the package that reaches a binding younger than its caller. - r = Base.invokelatest(reach, thunk, Tuple{}; maxdepth, maxcandidates, ignore) + r = Base.invokelatest(reach, thunk, Tuple{}; maxdepth, maxcandidates, maxwork, ignore) return Reach( path, r.reached, @@ -405,6 +423,21 @@ function _enter!(st::_Walk, match, depth::Int, path::Vector{Symbol}) ) return nothing end + # Depth bounds how FAR the walk goes, not how much of it there is: `maxdepth` levels each + # branching by `maxcandidates` is not a finite amount of work in any useful sense, and + # `visited` only prunes signatures that repeat. Measured on 1.14.0-DEV, `[f(x) for x in xs]` + # and `sum(map(f, xs))` produced new signatures faster than the depth limit could stop them + # and the call did not return; the same two answer in milliseconds on 1.12. So the walk also + # has a budget, and spends `:unknown` when it runs out — which is what `:unknown` is for. + if st.budget[] <= 0 + st.truncated = true + push!( + st.unresolved, + Unresolved(mm.name, sig, :budget, mm.file, Int(mm.line), mm, Mark[]), + ) + return nothing + end + st.budget[] -= 1 sig in st.visited && return nothing push!(st.visited, sig) # The flag `@experimental` emits is this package's own code, and under `ignore` the walk goes @@ -619,6 +652,7 @@ function _subwalk(st::_Walk) Module[], st.marked, false, + st.budget, ) end diff --git a/test/spec/README.md b/test/spec/README.md index 0ed6ca0..34786e5 100644 --- a/test/spec/README.md +++ b/test/spec/README.md @@ -53,9 +53,9 @@ that is entirely `@test_broken` is a claim written down, not a check being run. | `test_spec_integration.jl` | 19 | 19 | 0 | where the mark has to surface: docs, Aqua, releases, provenance, CI | | `test_spec_lifecycle.jl` | 16 | 16 | 0 | the mark's EXIT, and an entry point that is a module rather than a function | | `test_spec_profile.jl` | 45 | 45 | 0 | what a real run went through, how often, and how much of it | -| `test_spec_propagate.jl` | 21 | 21 | 0 | a caller that never names a marked thing still depends on it | +| `test_spec_propagate.jl` | 24 | 24 | 0 | a caller that never names a marked thing still depends on it | | `test_spec_verify.jl` | 9 | 9 | 0 | how well is a marked thing exercised by the tests | -| **10 files** | **187** | **187** | **0** | | +| **10 files** | **190** | **190** | **0** | | The table is generated and pinned by `test/test_spec_table.jl`, which fails if it goes stale — diff --git a/test/spec/test_spec_propagate.jl b/test/spec/test_spec_propagate.jl index 4d63cd2..33d4b1c 100644 --- a/test/spec/test_spec_propagate.jl +++ b/test/spec/test_spec_propagate.jl @@ -218,26 +218,60 @@ end # ── termination ────────────────────────────────────────────────────────────────────────────── -@testset "a generator argument is answered, not thrown out of" begin +@testset "a higher-order argument is answered, not thrown out of and not hung on" begin # `sum(f(x) for x in xs)` lowers to a `Base.MappingRF` whose two fields are both singletons, # which makes the STRUCT a singleton — so `w.instance` exists for a callable that is neither a # `Function` nor a `Type`. `nameof` has no method for that, and the analysis died with a # `MethodError` instead of returning one of its three verdicts. Measured on the shape this # package's own `@entered` docstring uses as its worked example. # - # A throw is not a fourth verdict. `:unknown` is what "could not resolve this" is for. - for (f, want) in ( - (Chain.gen_bad, :depends), - (Chain.gen_good, :clean), - (Chain.comp_bad, :depends), - (Chain.map_bad, :depends), - (Chain.loop_good, :clean), - ) - @testset "$(nameof(f))" begin - r = ExperimentalAPI.reach(f, Tuple{Vector{Float64}}) - @test ExperimentalAPI.verdict(r) === want + # Removing the throw then exposed the second half: on 1.14.0-DEV `[f(x) for x in xs]` and + # `sum(map(f, xs))` generated new signatures faster than `maxdepth` could stop them and the + # call never returned, while both answer in milliseconds on 1.12. `maxwork` bounds the total. + # + # A throw is not a fourth verdict and neither is a hang. + for f in (Chain.gen_bad, Chain.gen_good, Chain.comp_bad, Chain.map_bad, Chain.loop_good) + @testset "$(nameof(f)) answers" begin + @test ExperimentalAPI.verdict( + ExperimentalAPI.reach(f, Tuple{Vector{Float64}}) + ) in (:depends, :clean, :unknown) + end + end +end + +@testset "a higher-order caller that reaches a mark is never reported clean" begin + # The safety property, stated separately from the exact verdict because the exact verdict is + # version-dependent and this is not. Measured 2026-09-09: `[unstable(x) for x in xs]` and + # `sum(map(unstable, xs))` are `:depends` on 1.12.2 and `:unknown` on 1.14.0-DEV, because the + # budget runs out first there. `:unknown` is a weaker answer; `:clean` would be a false one. + for f in (Chain.gen_bad, Chain.comp_bad, Chain.map_bad) + @testset "$(nameof(f)) is not clean" begin + @test ExperimentalAPI.verdict( + ExperimentalAPI.reach(f, Tuple{Vector{Float64}}) + ) !== :clean end end + # Control: the same shapes with nothing marked behind them DO come back clean, so the + # assertion above is not satisfied by an analysis that never says `:clean` at all. + for f in (Chain.gen_good, Chain.loop_good) + @testset "$(nameof(f)) is clean" begin + @test ExperimentalAPI.verdict( + ExperimentalAPI.reach(f, Tuple{Vector{Float64}}) + ) === :clean + end + end +end + +@testset "the budget is a knob, and spending it says so rather than guessing" begin + # `maxwork` has to be reachable from the outside: an entry point that comes back `:unknown` + # with `:budget` in `unresolved` is a different situation from one that is genuinely dynamic, + # and the caller is the only one who can decide to pay for more. + r = ExperimentalAPI.reach(Chain.top_bad, Tuple{Float64}; maxwork=1) + @test ExperimentalAPI.verdict(r) === :unknown + @test any(u -> u.why === :budget, r.unresolved) + # Control: the same call with the default budget resolves, so `maxwork` is what did it. + @test ExperimentalAPI.verdict(ExperimentalAPI.reach(Chain.top_bad, Tuple{Float64})) === + :depends end @testset "self-recursion terminates and still finds the mark" begin From fd116c804b711e282998377dc57661df0e5f752a Mon Sep 17 00:00:00 2001 From: sotashimozono Date: Wed, 9 Sep 2026 05:53:05 +0000 Subject: [PATCH 2/2] docs: truncated makes reached a lower bound, and the docstring now says so MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measured while answering "what happens with several marks behind one loop": budgets between "too small to reach any" and the default report `:depends` with one, two, … of twelve found and the rest never walked to. The verdict is right either way; the LIST is not complete, and `truncated = true` is the only thing that says so. Co-Authored-By: Claude Opus 5 --- src/reach.jl | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/reach.jl b/src/reach.jl index c95ac10..eb6b789 100644 --- a/src/reach.jl +++ b/src/reach.jl @@ -92,7 +92,12 @@ What [`reach`](@ref) found. | `through_modules` | every module the walk went through | | `affected_entries` | for a module or script entry: the public entry points that are not clean | | `visited` | how many distinct signatures were inferred | -| `truncated` | whether a depth limit stopped the walk | +| `truncated` | whether a bound stopped the walk — `maxdepth` or `maxwork` | + +`truncated` is not cosmetic: when it is `true`, `reached` is a **lower bound**. Measured on a +module with twelve marked definitions behind one loop, budgets between the two extremes report +`:depends` with one, two, … of them found and the rest never walked to. `:depends` is still the +right verdict, but "fix the one it named" is not the same as "fix everything it depends on". There is deliberately **no** `verdict` field. A stored verdict makes `:clean` with a non-empty `unresolved` representable, and that state is the single thing this analysis must never report.