From 5d46397e810387b1b1d5cde6c2cffde5d2fa93e9 Mon Sep 17 00:00:00 2001 From: Fons van der Plas Date: Thu, 16 Nov 2023 12:21:56 +0100 Subject: [PATCH] Remove Configuration system --- README.md | 49 ++++--------- src/explore.jl | 20 ++---- test/Configuration.jl | 143 ------------------------------------- test/ExpressionExplorer.jl | 26 +++++++ test/PlutoConfiguration.jl | 141 ------------------------------------ test/helpers.jl | 8 +-- test/runtests.jl | 1 - 7 files changed, 52 insertions(+), 336 deletions(-) delete mode 100644 test/Configuration.jl delete mode 100644 test/PlutoConfiguration.jl diff --git a/README.md b/README.md index 491c80b..8818b89 100644 --- a/README.md +++ b/README.md @@ -168,50 +168,31 @@ SymbolsState( ``` -## Configuration +## Macro calls -It is possible to tweak the behaviour of ExpressionExplorer for specific expression types. For example, you can choose to never look inside a `for` expression (not sure why you would want that). In Pluto, we use this to tweak the way `macrocall` expressions are explored, in a way that we did not want to include in this more general package. +ExpressionExplorer ignores the arguments of macro calls. Macros can transform an expression into anything, so the output of ExpressionExplorer for expressions with a macro call is ambiguous. For example, the expression `@time x` contains a *reference* to `x`, while `@gensym x` contains a *definition* of `x`. -You can configure ExpressionExplorer by creating a new subtype: +In this example, notice that the assignment to `x` and reference to `y` are detected, but `AAA` and `BBB` are ignored, because they happen inside a macro call argument. ```julia -struct IgnoreForLoops <: ExpressionExplorer.AbstractExpressionExplorerConfiguration -end +julia> ExpressionExplorer.compute_reactive_node(quote + x = y + @time AAA = BBB + end) +ReactiveNode(Set([Symbol("@time"), :y]), Set([:x]), Set{Symbol}(), Set{FunctionNameSignaturePair}(), Set{Symbol}(), Set([Symbol("@time")])) ``` -Choose an `explore_...` method that you want to overload, and add an overload with `IgnoreForLoops`. You will need to read ExpressionExplorer's source code for this. +To solve this, you can **macroexpand expressions before giving them to ExpressionExplorer**. For example: ```julia -function ExpressionExplorer.explore_inner_scoped(ex::Expr, scopestate::ExpressionExplorer.ScopeState{IgnoreForLoops})::SymbolsState - if ex.head === :for - # ignore everything: report that nothing was found here: - return SymbolsState() - else - # the original code: - innerscopestate = deepcopy(scopestate) - innerscopestate.inglobalscope = false - - return mapfoldl(a -> explore!(a, innerscopestate), ex.args) - end -end +julia> ExpressionExplorer.compute_reactive_node(macroexpand(Main, quote + x = y + @time AAA = BBB + end)) +ReactiveNode(Set([:first, :GC_Diff, :isnothing, :gc_alloc_count, :-, :gc_num, :cumulative_compile_time_ns, :time_ns, :y, :BBB, :print, :cumulative_compile_timing, :time_print, :last, :!]), Set([:AAA, :x]), Set{Symbol}(), Set{FunctionNameSignaturePair}(), Set{Symbol}(), Set{Symbol}()) ``` -Then, use the `configuration` keyword argument when using ExpressionExplorer: - -```julia -julia> my_expr = quote - a = b - for x in z - w = rrr - end -end; - -julia> ExpressionExplorer.compute_reactive_node(my_expr; configuration=IgnoreForLoops()) -ReactiveNode(Set([:b]), Set([:a]), Set{Symbol}(), Set{FunctionNameSignaturePair}(), Set{Symbol}(), Set{Symbol}()) - -julia> ExpressionExplorer.compute_reactive_node(my_expr) -ReactiveNode(Set([:b, :rrr, :z]), Set([:a]), Set{Symbol}(), Set{FunctionNameSignaturePair}(), Set{Symbol}(), Set{Symbol}()) -``` +Notice that now, `AAA` and `BBB` are detected, along with functions used inside the `@time` expression. ## Utility functions diff --git a/src/explore.jl b/src/explore.jl index fe758cf..c68d0a0 100644 --- a/src/explore.jl +++ b/src/explore.jl @@ -53,20 +53,14 @@ Base.@kwdef mutable struct SymbolsState macrocalls::Set{FunctionName} = Set{FunctionName}() end - -abstract type AbstractExpressionExplorerConfiguration end - -struct DefaultConfiguration <: AbstractExpressionExplorerConfiguration end - "ScopeState moves _up_ the ASTree: it carries scope information up towards the endpoints." -mutable struct ScopeState{T <: AbstractExpressionExplorerConfiguration} +mutable struct ScopeState inglobalscope::Bool exposedglobals::Set{Symbol} hiddenglobals::Set{Symbol} definedfuncs::Set{Symbol} - configuration::T end -ScopeState(configuration::AbstractExpressionExplorerConfiguration=DefaultConfiguration()) = ScopeState(true, Set{Symbol}(), Set{Symbol}(), Set{Symbol}(), configuration) +ScopeState() = ScopeState(true, Set{Symbol}(), Set{Symbol}(), Set{Symbol}()) # The `union` and `union!` overloads define how two `SymbolsState`s or two `ScopeState`s are combined. @@ -443,7 +437,7 @@ function explore_macrocall!(ex::Expr, scopestate::ScopeState) symstate = SymbolsState(macrocalls = Set{FunctionName}([macro_name])) for arg in ex.args[begin+1:end] - macro_symstate = explore!(arg, ScopeState(scopestate.configuration)) + macro_symstate = explore!(arg, ScopeState()) union!(symstate, SymbolsState(macrocalls = macro_symstate.macrocalls)) end @@ -1155,9 +1149,9 @@ compute_symbols_state(ex::Any)::SymbolsState Return the global references, assignment, function calls and function definitions inside an arbitrary expression, in a `SymbolsState` object. """ -function compute_symbols_state(ex::Any; configuration::AbstractExpressionExplorerConfiguration=DefaultConfiguration())::SymbolsState +function compute_symbols_state(ex::Any)::SymbolsState try - compute_symbolreferences(ex; configuration) + compute_symbolreferences(ex) catch e if e isa InterruptException rethrow(e) @@ -1168,8 +1162,8 @@ function compute_symbols_state(ex::Any; configuration::AbstractExpressionExplore end end -function compute_symbolreferences(ex::Any; configuration::AbstractExpressionExplorerConfiguration=DefaultConfiguration())::SymbolsState - symstate = explore!(ex, ScopeState(configuration)) +function compute_symbolreferences(ex::Any)::SymbolsState + symstate = explore!(ex, ScopeState()) handle_recursive_functions!(symstate) return symstate end diff --git a/test/Configuration.jl b/test/Configuration.jl deleted file mode 100644 index 21fc65b..0000000 --- a/test/Configuration.jl +++ /dev/null @@ -1,143 +0,0 @@ -@testset "Macros and heuristics w/o Pluto" begin - @test test_expression_explorer(; - expr=:(@macro import Pkg), - macrocalls=[Symbol("@macro")], - definitions=[], - ) - @test test_expression_explorer(; - expr=:(@macro Pkg.activate("..")), - macrocalls=[Symbol("@macro")], - references=[], - funccalls=[], - ) - @test test_expression_explorer(; - expr=:(@macro Pkg.add("Pluto.jl")), - macrocalls=[Symbol("@macro")], - references=[], - funccalls=[], - ) - @test test_expression_explorer(; - expr=:(@macro include("Firebasey.jl")), - macrocalls=[Symbol("@macro")], - funccalls=[], - ) -end - - -include("PlutoConfiguration.jl") -import .PlutoConfigurationSetup - -configuration = PlutoConfigurationSetup.PlutoConfiguration() - - - - -@testset "Macros and heuristics w/ Pluto" begin - - @test test_expression_explorer(; - expr=:(@macro import Pkg), - macrocalls=[Symbol("@macro")], - definitions=[:Pkg], - configuration, - ) - @test test_expression_explorer(; - expr=:(@macro Pkg.activate("..")), - macrocalls=[Symbol("@macro")], - references=[:Pkg], - funccalls=[[:Pkg, :activate]], - configuration, - ) - @test test_expression_explorer(; - expr=:(@macro Pkg.add("Pluto.jl")), - macrocalls=[Symbol("@macro")], - references=[:Pkg], - funccalls=[[:Pkg, :add]], - configuration, - ) - @test test_expression_explorer(; - expr=:(@macro include("Firebasey.jl")), - macrocalls=[Symbol("@macro")], - funccalls=[[:include]], - configuration, - ) -end - - - - - -@testset "Macros w/ Pluto 1" begin - # Macros tests are not just in ExpressionExplorer now - - @test testee(:(@time a = 2), [], [], [], [], [Symbol("@time")]; configuration) - @test testee(:(@f(x; y=z)), [], [], [], [], [Symbol("@f")]; configuration) - @test testee(:(@f(x, y = z)), [], [], [], [], [Symbol("@f")]) # https://github.com/fonsp/Pluto.jl/issues/252 - @test testee(:(Base.@time a = 2), [], [], [], [], [[:Base, Symbol("@time")]]; configuration) - # @test_nowarn testee(:(@enum a b = d c), [:d], [:a, :b, :c], [Symbol("@enum")], []) - # @enum is tested in test/React.jl instead - @test testee(:(@gensym a b c), [], [:a, :b, :c], [:gensym], [], [Symbol("@gensym")]; configuration) - @test testee(:(Base.@gensym a b c), [], [:a, :b, :c], [:gensym], [], [[:Base, Symbol("@gensym")]]; configuration) - @test testee(:(Base.@kwdef struct A; x = 1; y::Int = two; z end), [], [], [], [], [[:Base, Symbol("@kwdef")]]; configuration) - @test testee(quote "asdf" f(x) = x end, [], [], [], [], [Symbol("@doc")]; configuration) - - # @test testee(:(@bind a b), [], [], [], [], [Symbol("@bind")]; configuration) - # @test testee(:(PlutoRunner.@bind a b), [], [], [], [], [[:PlutoRunner, Symbol("@bind")]]; configuration) - # @test_broken testee(:(Main.PlutoRunner.@bind a b), [:b], [:a], [[:Base, :get], [:Core, :applicable], [:PlutoRunner, :create_bond], [:PlutoRunner, Symbol("@bind")]], [], verbose=false; configuration) - # @test testee(:(let @bind a b end), [], [], [], [], [Symbol("@bind")]; configuration) - - @test testee(:(`hey $(a = 1) $(b)`), [:b], [], [:cmd_gen], [], [Symbol("@cmd")]; configuration) - # @test testee(:(md"hey $(@bind a b) $(a)"), [:a], [], [[:getindex]], [], [Symbol("@md_str"), Symbol("@bind")]; configuration) - # @test testee(:(md"hey $(a) $(@bind a b)"), [:a], [], [[:getindex]], [], [Symbol("@md_str"), Symbol("@bind")]; configuration) - - @test testee(:(@asdf a = x1 b = x2 c = x3), [], [], [], [], [Symbol("@asdf")]; configuration) # https://github.com/fonsp/Pluto.jl/issues/670 - - @test testee(:(@einsum a[i,j] := x[i]*y[j]), [], [], [], [], [Symbol("@einsum")]; configuration) - @test testee(:(@tullio a := f(x)[i+2j, k[j]] init=z), [], [], [], [], [Symbol("@tullio")]; configuration) - @test testee(:(Pack.@asdf a[1,k[j]] := log(x[i]/y[j])), [], [], [], [], [[:Pack, Symbol("@asdf")]]; configuration) - - - @test testee(:(html"a $(b = c)"), [], [], [], [], [Symbol("@html_str")]; configuration) - @test testee(:(md"a $(b = c) $(b)"), [:c], [:b], [:getindex], [], [Symbol("@md_str")]; configuration) - @test testee(:(md"\* $r"), [:r], [], [:getindex], [], [Symbol("@md_str")]; configuration) - @test testee(:(md"a \$(b = c)"), [], [], [:getindex], [], [Symbol("@md_str")]; configuration) - @test testee(:(macro a() end), [], [], [], [ - Symbol("@a") => ([], [], [], []) - ]; configuration) - @test testee(:(macro a(b::Int); b end), [], [], [], [ - Symbol("@a") => ([:Int], [], [], []) - ]; configuration) - @test testee(:(macro a(b::Int=c) end), [], [], [], [ - Symbol("@a") => ([:Int, :c], [], [], []) - ]; configuration) - @test testee(:(macro a(); b = c; return b end), [], [], [], [ - Symbol("@a") => ([:c], [], [], []) - ]; configuration) - @test test_expression_explorer(; - expr=:(@parent @child 10), - macrocalls=[Symbol("@parent"), Symbol("@child")], - configuration - ) - @test test_expression_explorer(; - expr=:(@parent begin @child 1 + @grandchild 10 end), - macrocalls=[Symbol("@parent"), Symbol("@child"), Symbol("@grandchild")], - configuration - ) - @test testee(macroexpand(Main, :(@noinline f(x) = x)), [], [], [], [ - Symbol("f") => ([], [], [], []) - ]; configuration) -end - - -@testset "Macros w/ Pluto" begin - - @test testee(:(@bind a b), [:b, :PlutoRunner, :Base, :Core], [:a], [[:PlutoRunner, :create_bond], [:Core, :applicable], [:Base, :get]], [], [Symbol("@bind")]; configuration) - @test testee(:(PlutoRunner.@bind a b), [:b, :PlutoRunner, :Base, :Core], [:a], [[:PlutoRunner, :create_bond], [:Core, :applicable], [:Base, :get]], [], [[:PlutoRunner, Symbol("@bind")]]; configuration) - @test_broken testee(:(Main.PlutoRunner.@bind a b), [:b, :PlutoRunner, :Base, :Core], [:a], [[:Base, :get], [:Core, :applicable], [:PlutoRunner, :create_bond], [:PlutoRunner, Symbol("@bind")]], [], verbose=false, configuration) - @test testee(:(let @bind a b end), [:b, :PlutoRunner, :Base, :Core], [:a], [[:PlutoRunner, :create_bond], [:Core, :applicable], [:Base, :get]], [], [Symbol("@bind")]; configuration) - - @test testee(:(`hey $(a = 1) $(b)`), [:b], [], [:cmd_gen], [], [Symbol("@cmd")]; configuration) - @test testee(:(md"hey $(@bind a b) $(a)"), [:b, :PlutoRunner, :Base, :Core], [:a], [[:PlutoRunner, :create_bond], [:Core, :applicable], [:Base, :get], :getindex], [], [Symbol("@md_str"), Symbol("@bind")]; configuration) - @test testee(:(md"hey $(a) $(@bind a b)"), [:a, :b, :PlutoRunner, :Base, :Core], [:a], [[:PlutoRunner, :create_bond], [:Core, :applicable], [:Base, :get], :getindex], [], [Symbol("@md_str"), Symbol("@bind")]; configuration) - - -end \ No newline at end of file diff --git a/test/ExpressionExplorer.jl b/test/ExpressionExplorer.jl index 4f51abd..5e6269a 100644 --- a/test/ExpressionExplorer.jl +++ b/test/ExpressionExplorer.jl @@ -736,3 +736,29 @@ end ] ) end + +@testset "Macros and heuristics w/o Pluto" begin + @test test_expression_explorer(; + expr=:(@macro import Pkg), + macrocalls=[Symbol("@macro")], + definitions=[], + ) + @test test_expression_explorer(; + expr=:(@macro Pkg.activate("..")), + macrocalls=[Symbol("@macro")], + references=[], + funccalls=[], + ) + @test test_expression_explorer(; + expr=:(@macro Pkg.add("Pluto.jl")), + macrocalls=[Symbol("@macro")], + references=[], + funccalls=[], + ) + @test test_expression_explorer(; + expr=:(@macro include("Firebasey.jl")), + macrocalls=[Symbol("@macro")], + funccalls=[], + ) +end + diff --git a/test/PlutoConfiguration.jl b/test/PlutoConfiguration.jl deleted file mode 100644 index b2b0c2c..0000000 --- a/test/PlutoConfiguration.jl +++ /dev/null @@ -1,141 +0,0 @@ -module PlutoConfigurationSetup - -using ..ExpressionExplorer -using ..ExpressionExplorer: ScopeState - -module PlutoRunner - -using Markdown - -function create_bond(args...) -end - - -initial_value_getter_ref = Ref(nothing) - - -macro bind(def, element) - if def isa Symbol - quote - local el = $(esc(element)) - global $(esc(def)) = Core.applicable(Base.get, el) ? Base.get(el) : $(initial_value_getter_ref)[](el) - PlutoRunner.create_bond(el, $(Meta.quot(def)), nothing) - end - else - :(throw(ArgumentError("""\nMacro example usage: \n\n\t@bind my_number html""\n\n"""))) - end -end - -end - - -import .PlutoRunner - - - - -struct PlutoConfiguration <: ExpressionExplorer.AbstractExpressionExplorerConfiguration -end - - - -""" -Uses `cell_precedence_heuristic` to determine if we need to include the contents of this macro in the symstate. -This helps with things like a Pkg.activate() that's in a macro, so Pluto still understands to disable nbpkg. -""" -function macro_has_special_heuristic_inside(; symstate::SymbolsState, expr::Expr)::Bool - blub = union( - symstate.references, - symstate.assignments, - (x.joined for x in symstate.funccalls), - ) - - yup = ["Pkg", "include"] - - return any(blub) do s - any(yup) do z - occursin(z, String(s)) - end - end -end - - - - - -const can_macroexpand_no_bind = Set(Symbol.(["@md_str", "Markdown.@md_str", "@gensym", "Base.@gensym", "@enum", "Base.@enum", "@assert", "Base.@assert", "@cmd"])) -const can_macroexpand = can_macroexpand_no_bind ∪ Set(Symbol.(["@bind", "PlutoRunner.@bind"])) - -""" -If the macro is **known to Pluto**, expand or 'mock expand' it, if not, return the expression. Macros from external packages are not expanded, this is done later in the pipeline. See https://github.com/fonsp/Pluto.jl/pull/1032 -""" -function maybe_macroexpand_pluto(ex::Expr; recursive::Bool=false, expand_bind::Bool=true) - result::Expr = if ex.head === :macrocall - funcname = ExpressionExplorer.split_funcname(ex.args[1]) - - if funcname.joined ∈ (expand_bind ? can_macroexpand : can_macroexpand_no_bind) - macroexpand(PlutoRunner, ex; recursive=false)::Expr - else - ex - end - else - ex - end - - if recursive - # Not using broadcasting because that is expensive compilation-wise for `result.args::Any`. - expanded = Any[] - for arg in result.args - ex = maybe_macroexpand_pluto(arg; recursive, expand_bind) - push!(expanded, ex) - end - return Expr(result.head, expanded...) - else - return result - end -end - -maybe_macroexpand_pluto(ex::Any; kwargs...) = ex - - - - - - - -function ExpressionExplorer.explore_macrocall!(ex::Expr, scopestate::ScopeState{PlutoConfiguration}) - # Early stopping, this expression will have to be re-explored once - # the macro is expanded in the notebook process. - macro_name = ExpressionExplorer.split_funcname(ex.args[1]) - symstate = SymbolsState(macrocalls = Set{FunctionName}([macro_name])) - - # Because it sure wouldn't break anything, - # I'm also going to blatantly assume that any macros referenced in here... - # will end up in the code after the macroexpansion 🤷‍♀️ - # "You should make a new function for that" they said, knowing I would take the lazy route. - for arg in ex.args[begin+1:end] - macro_symstate = ExpressionExplorer.explore!(arg, ScopeState(scopestate.configuration)) - - # Also, when this macro has something special inside like `Pkg.activate()`, - # we're going to treat it as normal code (so these heuristics trigger later) - # (Might want to also not let this to @eval macro, as an extra escape hatch if you - # really don't want pluto to see your Pkg.activate() call) - if arg isa Expr && macro_has_special_heuristic_inside(symstate = macro_symstate, expr = arg) - union!(symstate, macro_symstate) - else - union!(symstate, SymbolsState(macrocalls = macro_symstate.macrocalls)) - end - end - - # Some macros can be expanded on the server process - if macro_name.joined ∈ can_macroexpand - new_ex = maybe_macroexpand_pluto(ex) - union!(symstate, ExpressionExplorer.explore!(new_ex, scopestate)) - end - - return symstate -end - -end - - diff --git a/test/helpers.jl b/test/helpers.jl index e16a7fb..48226f7 100644 --- a/test/helpers.jl +++ b/test/helpers.jl @@ -51,11 +51,11 @@ julia> @test testee(:( true ``` " -function testee(expr::Any, expected_references, expected_definitions, expected_funccalls, expected_funcdefs, expected_macrocalls = []; verbose::Bool=true, configuration=ExpressionExplorer.DefaultConfiguration()) +function testee(expr::Any, expected_references, expected_definitions, expected_funccalls, expected_funcdefs, expected_macrocalls = []; verbose::Bool=true) expected = easy_symstate(expected_references, expected_definitions, expected_funccalls, expected_funcdefs, expected_macrocalls) original_hash = expr_hash(expr) - result = ExpressionExplorer.compute_symbolreferences(expr; configuration) + result = ExpressionExplorer.compute_symbolreferences(expr) # should not throw: ReactiveNode(result) @@ -105,8 +105,8 @@ expr_hash(x) = objectid(x) """ Like `testee` but actually a convenient syntax """ -function test_expression_explorer(; expr, references=[], definitions=[], funccalls=[], funcdefs=[], macrocalls=[], configuration::ExpressionExplorer.AbstractExpressionExplorerConfiguration=ExpressionExplorer.DefaultConfiguration()) - testee(expr, references, definitions, funccalls, funcdefs, macrocalls; configuration) +function test_expression_explorer(; expr, references=[], definitions=[], funccalls=[], funcdefs=[], macrocalls=[]) + testee(expr, references, definitions, funccalls, funcdefs, macrocalls) end function easy_symstate(expected_references, expected_definitions, expected_funccalls, expected_funcdefs, expected_macrocalls = []) diff --git a/test/runtests.jl b/test/runtests.jl index bc2e28a..cac67a6 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -1,6 +1,5 @@ include("helpers.jl") include("ExpressionExplorer.jl") -include("Configuration.jl") include("ReactiveNode.jl") include("UsingsImports.jl") include("Utils.jl")