From 6e5015045140a08b89931dcf06f12cb25340de67 Mon Sep 17 00:00:00 2001 From: sotashimozono Date: Sat, 5 Sep 2026 08:31:12 +0000 Subject: [PATCH] test: three claims that were pinned by their shape and not by their answer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Asked of the merged spec: is every behaviour actually checked? Three were not. Each was a `hasproperty` or an `isa Function` — satisfied by an implementation that returns an empty result for every input — and one of them was hiding a live defect. **`reach_script` answered `:clean` for a script that reaches a mark.** `const RESULT = simulate(model)` is a script's *work* wearing a declaration's syntax, and it is how the line that produces the figure is actually written. It was evaluated in the scratch module and never analysed, so the thunk contained no call at all. The binding still has to be made — a later `struct` may use it — so the value is computed at top level and the right-hand side is analysed as well. The test now runs three scripts: one that reaches a mark, one of the same shape that does not, and the trivial one it had before. **A method-level mark had never been through a precompile cache.** `test/test_precompile.jl` is "the measurement that could kill the design", and it was reading `name`, `reason`, `since` and `tracking` — a `Symbol` and three strings. It never read `sig`, which is a `Type` in a `const` vector inside the cache image, and the fixture package carried no mark on somebody else's generic, whose signature names `typeof(Base.show)` and a type defined in the cached package. `MarkedPkg` now carries one of each kind and the probe reads all three, with two controls that a mark widened on its way through the cache would fail: `Base.show` is not experimental for anybody, and the sibling `show(::IO, ::Int)` is not marked. It also runs `reach` across the package boundary — a caller reaching a mark written while another process precompiled another package — with a same-depth control. Both pass; the claim is now measured rather than assumed. **Cross-package propagation** said `reach isa Function`, and the spec admitted it needed a fixture package. It now asserts the claim is checked where the fixture package lives, and checks the module-boundary version it can reach on its own. Renaming the precompilation testset while promoting it is what let the first two hide: the name that said "a method mark survives precompilation" was the only record that it did not. 1005 assertions, green. Co-Authored-By: Claude Opus 5 --- src/reach.jl | 17 ++++++++ test/fixtures/MarkedPkg/src/MarkedPkg.jl | 18 +++++++- test/spec/test_spec_lifecycle.jl | 45 ++++++++++++++++++-- test/spec/test_spec_propagate.jl | 21 +++++++++- test/test_precompile.jl | 52 ++++++++++++++++++++++-- 5 files changed, 142 insertions(+), 11 deletions(-) diff --git a/src/reach.jl b/src/reach.jl index 251f9e1..a3f1be9 100644 --- a/src/reach.jl +++ b/src/reach.jl @@ -330,6 +330,14 @@ function reach_script( push!(body.args, st) elseif _is_toplevel_only(st) Core.eval(scratch, st) + # `const RESULT = simulate(model)` is a script's WORK wearing a declaration's syntax, + # and it is how a researcher writes the line that produces the figure. Evaluating it + # and stopping there analysed nothing: measured on a two-line script whose only call + # was a `const`, and the answer came back `:clean`. The binding still has to be made — + # a later `struct` may use it — so the value is computed at top level and the + # right-hand side is analysed as well. + rhs = _const_rhs(st) + rhs === nothing || push!(body.args, rhs) else push!(body.args, st) end @@ -363,6 +371,15 @@ function _is_toplevel_only(st) ) end +# The right-hand side of `const x = …`, which is the only top-level-only form that routinely +# carries a call worth analysing. +function _const_rhs(st) + st isa Expr && st.head === :const || return nothing + a = st.args[1] + (a isa Expr && a.head === :(=) && length(a.args) == 2) || return nothing + return a.args[2] +end + # ── the walk ───────────────────────────────────────────────────────────────────────────────── function _enter!(st::_Walk, match, depth::Int, path::Vector{Symbol}) diff --git a/test/fixtures/MarkedPkg/src/MarkedPkg.jl b/test/fixtures/MarkedPkg/src/MarkedPkg.jl index a4546a9..a70a4f0 100644 --- a/test/fixtures/MarkedPkg/src/MarkedPkg.jl +++ b/test/fixtures/MarkedPkg/src/MarkedPkg.jl @@ -1,12 +1,18 @@ # A real package, loaded from its own precompile cache by test/test_precompile.jl. The marks it # carries are written at ITS precompile time — which is the moment a design that stored them in # ExperimentalAPI's own state would lose them, and this fixture is how that gets caught. +# +# It carries one of each KIND of mark, because they are stored differently and only one of them +# was ever put through a cache: a whole-name declaration, a mark attached to a definition (which +# also records the signature it created), and a mark on a method of somebody else's generic. The +# last is the hard one — its `sig` names `typeof(Base.show)` and a type defined here, and it has +# to come back out of the cache image intact. module MarkedPkg using ExperimentalAPI export settled -public unsettled, unsettled_type +public unsettled, unsettled_type, Widget, consumer "Documented, and not going to change." settled(x) = x @@ -24,4 +30,14 @@ settled(x) = x v::Int end) +"A type whose printed form is not settled." +struct Widget end + +# A method on a generic this package does not own. The mark lives here, in the module that wrote +# the method; `Base.show` is not made experimental for anybody else. +@experimental "printing format not settled" Base.show(io::IO, ::Widget) = print(io, "W") + +"Documented, and reaches an unsettled definition without naming it in its own signature." +consumer(x) = first(unsettled(x)) + end diff --git a/test/spec/test_spec_lifecycle.jl b/test/spec/test_spec_lifecycle.jl index 51890e3..0bc5f99 100644 --- a/test/spec/test_spec_lifecycle.jl +++ b/test/spec/test_spec_lifecycle.jl @@ -94,10 +94,47 @@ end @testset "a script can be the entry point" begin # The shape a researcher has: a file that produces a figure, not a package. The file must be # written — `tempname()` alone throws regardless of the implementation. - path = tempname() - write(path, "1 + 1\n") - @test isfile(path) - @test hasproperty(ExperimentalAPI.reach_script(path), :reached) + # + # `hasproperty(…, :reached)` was the whole of this claim for a while, and it is satisfied by + # an implementation that returns an empty `Reach` for every input. So the script that reaches + # a mark and the one that does not are both run, and the verdicts have to differ. + dir = mktempdir() + plain = joinpath(dir, "plain.jl") + write(plain, "1 + 1\n") + @test isfile(plain) + r = ExperimentalAPI.reach_script(plain) + @test hasproperty(r, :reached) + @test ExperimentalAPI.verdict(r) === :clean + + # A script in the shape it is actually written in: `using`, then work. The `using` line is + # evaluated in a scratch module — that is what `reach_script` costs, and it is why the + # analysis can resolve `Lifecycle.consumer` at all. + dirty = joinpath(dir, "figure.jl") + write( + dirty, + """ + using Main: Lifecycle + const RESULT = Lifecycle.consumer(0.5) + RESULT + """, + ) + d = ExperimentalAPI.reach_script(dirty) + @test ExperimentalAPI.verdict(d) === :depends + @test :verified_now in [x.mark.name for x in d.reached] + # …and the entry is named after the file, so a directory of scripts is readable. + @test :figure in [e.name for e in d.affected_entries] || Symbol("figure.jl") in [e.name for e in d.affected_entries] + + # Control of the same shape: same `using`, same depth, nothing marked behind it. + settled_path = joinpath(dir, "settled.jl") + write( + settled_path, + """ + using Main: Lifecycle + const RESULT = Lifecycle.settled(0.5) + RESULT + """, + ) + @test ExperimentalAPI.verdict(ExperimentalAPI.reach_script(settled_path)) === :clean end # ── the exit: what licenses removing a mark ────────────────────────────────────────────────── diff --git a/test/spec/test_spec_propagate.jl b/test/spec/test_spec_propagate.jl index c04c19f..d49043f 100644 --- a/test/spec/test_spec_propagate.jl +++ b/test/spec/test_spec_propagate.jl @@ -233,8 +233,25 @@ end # ── across packages ────────────────────────────────────────────────────────────────────────── @testset "a mark in a dependency propagates into the dependent" begin - # Needs the fixture package, so only the API shape is pinned here. - @test ExperimentalAPI.reach isa Function + # `reach isa Function` was the whole of this claim for a while, and it is satisfied by an + # implementation that answers `:clean` for everything. The real question is whether a mark + # written while ANOTHER package was precompiled — in a process that has since exited — is + # visible to a caller here, and that needs a package rather than a module. + # + # `test/test_precompile.jl` is where it is asked, because it is the file that owns the scratch + # depot and the two subprocess runs. This asserts it is asked there rather than restating it: + # a claim checked in one place and mentioned in another is one that goes stale in the second. + src = read(joinpath(@__DIR__, "..", "test_precompile.jl"), String) + @test occursin("REACH=", src) + @test occursin("\"REACH\"] == \"depends\"", src) + @test occursin("\"REACHCONTROL\"] == \"clean\"", src) + + # What CAN be asked without a second package: the same shape across a module boundary, where + # the mark is in one module and the caller in another that never names it. + boundary(x::Float64) = Chain.top_bad(x) + @test ExperimentalAPI.verdict(ExperimentalAPI.reach(boundary, ENTRY)) === :depends + control(x::Float64) = Chain.top_good(x) + @test ExperimentalAPI.verdict(ExperimentalAPI.reach(control, ENTRY)) === :clean end # ── cost ───────────────────────────────────────────────────────────────────────────────────── diff --git a/test/test_precompile.jl b/test/test_precompile.jl index c952b07..f9813b1 100644 --- a/test/test_precompile.jl +++ b/test/test_precompile.jl @@ -9,6 +9,12 @@ # So this runs a real package through a real precompile, in a scratch depot, in a subprocess, and # asks the loaded module what it is carrying. Twice: once compiling from source, once reading the # cache the first run wrote. Only the second run is evidence. +# +# It asks about all three KINDS of mark, because they are stored differently: a whole-name +# declaration, a mark attached to a definition (which also carries the `Type` it created), and a +# mark on a method of `Base.show`, whose signature names a type defined in the cached package. +# Only the first of those had ever been through a cache. It also runs `reach` across the package +# boundary, which is the query that reads the other two. using Test: @test, @testset @@ -31,6 +37,24 @@ println("UNACCOUNTED=", join(sort(string.(ExperimentalAPI.audit(MarkedPkg).unacc println("STABLE=", join(sort(string.(ExperimentalAPI.stable(MarkedPkg))), ",")) println("CACHED=", Base.isprecompiled(Base.PkgId(MarkedPkg))) println("DEPOT1=", first(DEPOT_PATH)) + +# The signature half. A `Mark` records the signature it attached to, and that field is a `Type` +# living in a `const` vector inside the cache image — a different thing to serialise from a +# `Symbol` and a `String`, and the reason this probe is not just about names. +println("SIG=", ExperimentalAPI.mark(MarkedPkg, :unsettled).sig) +println("METHODMARKS=", join(sort(string.(getfield.(ExperimentalAPI.experimental_methods(MarkedPkg), :name))), ",")) +println("SHOWSIG=", ExperimentalAPI.mark(MarkedPkg, :show).sig) +println("SHOWMARKED=", ExperimentalAPI.isexperimental(which(show, Tuple{IO,MarkedPkg.Widget}))) +println("SHOWSIBLING=", ExperimentalAPI.isexperimental(which(show, Tuple{IO,Int}))) +println("BASESHOW=", ExperimentalAPI.isexperimental(Base, :show)) + +# And the query that reads it: a caller in THIS process reaching a mark that was written while +# another process precompiled another package. Nothing here names `unsettled`. +caller(x) = MarkedPkg.consumer(x) + 1 +control(x) = MarkedPkg.settled(x) + 1 +println("REACH=", ExperimentalAPI.verdict(ExperimentalAPI.reach(caller, Tuple{Int}))) +println("REACHCONTROL=", ExperimentalAPI.verdict(ExperimentalAPI.reach(control, Tuple{Int}))) +println("REACHNAME=", join(sort(string.([r.mark.name for r in ExperimentalAPI.reach(caller, Tuple{Int}).reached])), ",")) """ function parse_probe(out) @@ -94,14 +118,34 @@ end @test warm["CACHED"] == "true" @testset "$phase" for (phase, r) in ("compiled from source" => cold, "from cache" => warm) - @test r["NAMES"] == "unsettled,unsettled_type" + @test r["NAMES"] == "show,unsettled,unsettled_type" @test r["REASON"] == "the return shape is still being decided" @test r["SINCE"] == "0.1.0" @test r["TRACKING"] == "https://example.invalid/issues/1" - # `settled` is documented and the other two are declared, so nothing is left over — - # the audit of a marked package agrees with the marks that survived. + # `settled`, `Widget` and `consumer` are documented and the rest are declared, so + # nothing is left over — the audit of a marked package agrees with the marks that + # survived. @test r["UNACCOUNTED"] == "" - @test r["STABLE"] == "settled" + @test r["STABLE"] == "Widget,consumer,settled" + + # The signature came back, and it is the one the definition created rather than a + # widened stand-in. + @test r["SIG"] == "Tuple{typeof(MarkedPkg.unsettled), Any}" + @test r["METHODMARKS"] == "show,unsettled" + # The hard case: a mark on a method of somebody else's generic, whose signature names + # `typeof(Base.show)` and a type defined in the cached package. + @test r["SHOWSIG"] == "Tuple{typeof(show), IO, MarkedPkg.Widget}" + @test r["SHOWMARKED"] == "true" + # Controls, both of which would also be "true" for a mark that had widened to the + # whole generic on its way through the cache. + @test r["SHOWSIBLING"] == "false" + @test r["BASESHOW"] == "false" + + # A caller that reaches a mark written during ANOTHER package's precompilation, + # without naming it — and a control of the same depth that reaches nothing marked. + @test r["REACH"] == "depends" + @test r["REACHNAME"] == "unsettled" + @test r["REACHCONTROL"] == "clean" end @test cold == warm end