🐶 Fix #41

Project.toml

@@ -2,7 +2,7 @@ name = "Pluto"
 uuid = "c3e4b0f8-55cb-11ea-2926-15256bba5781"
 license = "MIT"
 authors = ["Fons van der Plas <fonsvdplas@gmail.com>", "Mikołaj Bochenski <iceflamecode@gmail.com>"]
-version = "0.5.11"
+version = "0.5.12"
 
 [deps]
 Distributed = "8ba89e20-285c-5b6f-9357-94700520ee1b"

src/react/ExploreExpression.jl

@@ -24,15 +24,21 @@ mutable struct ScopeState
     inglobalscope::Bool
     exposedglobals::Set{Symbol}
     hiddenglobals::Set{Symbol}
+    definedfuncs::Set{Symbol}
 end
 
 function union(a::Dict{Symbol,SymbolsState}, b::Dict{Symbol,SymbolsState})
+    # TODO: optimise: reuse `a` as `c`
     c = Dict{Symbol,SymbolsState}()
     for (k, v) in a
         c[k] = v
     end
     for (k, v) in b
-        c[k] = v
+        if haskey(c, k)
+            c[k] = c[k] ∪ v
+        else
+            c[k] = v
+        end
     end
     c
 end
@@ -50,7 +56,7 @@ function ==(a::SymbolsState, b::SymbolsState)
 end
 
 function will_assign_global(assignee::Symbol, scopestate::ScopeState)::Bool
-    (scopestate.inglobalscope || assignee in scopestate.exposedglobals) && !(assignee in scopestate.hiddenglobals)
+    (scopestate.inglobalscope || assignee ∈ scopestate.exposedglobals) && (assignee ∉ scopestate.hiddenglobals || assignee ∈ scopestate.definedfuncs)
 end
 
 function get_global_assignees(assignee_exprs, scopestate::ScopeState)::Set{Symbol}
@@ -199,12 +205,8 @@ function explore!(ex::Expr, scopestate::ScopeState)::SymbolsState
         # Because we are entering a new scope, we create a copy of the current scope state, and run it through the expressions.
         innerscopestate = deepcopy(scopestate)
         innerscopestate.inglobalscope = false
-        for a in ex.args
-            innersymstate = explore!(a, innerscopestate)
-            symstate = symstate ∪ innersymstate
-        end
 
-        return symstate
+        return mapfoldl(a -> explore!(a, innerscopestate), ∪, ex.args, init=symstate)
     elseif ex.head == :struct
         # Creates local scope
 
@@ -238,21 +240,28 @@ function explore!(ex::Expr, scopestate::ScopeState)::SymbolsState
             ex.args[1]
         end
 
-        funcname = assignee = funcroot.args[1]
+        # is either `f` (Symbol) or `f::String` (Expression)
+        funcname_expr = assignee = funcroot.args[1]
 
         # is either [funcname] or []
-        global_assignees = get_global_assignees([funcname], scopestate)
+        global_assignees = get_global_assignees([funcname_expr], scopestate)
+
 
         # Because we are entering a new scope, we create a copy of the current scope state, and run it through the expressions.
-
+        
         innerscopestate = deepcopy(scopestate)
         innerscopestate.hiddenglobals = union(innerscopestate.hiddenglobals, extractfunctionarguments(funcroot))
         innerscopestate.inglobalscope = false
-
+        
         innersymstate = explore!(Expr(:block, ex.args[2:end]...), innerscopestate)
+
+        if !isempty(global_assignees)
+            funcname = first(global_assignees)
 
-        if funcname in global_assignees
             symstate.funcdefs[funcname] = innersymstate
+            push!(scopestate.hiddenglobals, funcname)
+            push!(scopestate.definedfuncs, funcname)
+            push!(symstate.assignments, funcname)
         else
             # The function is not defined globally. However, the function can still modify the global scope or reference globals, e.g.
 
@@ -267,9 +276,6 @@ function explore!(ex::Expr, scopestate::ScopeState)::SymbolsState
             symstate = symstate ∪ innersymstate
         end
 
-        scopestate.hiddenglobals = union(scopestate.hiddenglobals, global_assignees)
-        symstate.assignments = union(symstate.assignments, global_assignees)
-
         return symstate
     elseif ex.head == :(->)
         # Creates local scope
@@ -358,12 +364,7 @@ function explore!(ex::Expr, scopestate::ScopeState)::SymbolsState
             symstate.assignments = union(symstate.assignments, exposed)
         end
 
-        for a in recursers
-            innersymstate = explore!(a, scopestate)
-            symstate = symstate ∪ innersymstate
-        end
-
-        return symstate
+        return mapfoldl(a -> explore!(a, scopestate), ∪, recursers, init=symstate)
     elseif ex.head == :using || ex.head == :import
         if scopestate.inglobalscope
             imports = if ex.args[1].head == :(:)
@@ -404,18 +405,24 @@ function explore!(ex::Expr, scopestate::ScopeState)::SymbolsState
 
         # Does not create scope (probably)
 
-        for a in ex.args
-            innersymstate = explore!(a, scopestate)
-            symstate = symstate ∪ innersymstate
-        end
-
-        return symstate
+        return mapfoldl(a -> explore!(a, scopestate), ∪, ex.args, init=symstate)
     end
 end
 
 
 function compute_symbolreferences(ex)::SymbolsState
-    explore!(ex, ScopeState(true, Set{Symbol}(), Set{Symbol}()))
+    symstate = explore!(ex, ScopeState(true, Set{Symbol}(), Set{Symbol}(), Set{Symbol}()))
+
+    # We do something special to account for recursive functions:
+    # If a function `f` calls a function `g`, and both are defined inside this cell, the reference to `g` inside the symstate of `f` will be deleted.
+    # the motivitation is that normally, an assignment (or function definition) will add that symbol to a list of 'hidden globals' - any future references to that symbol will be ignored. i.e. the _local definition hides a global_.
+    # In the case of functions, you can reference functions and variables that do not yet exist, and so they won't be in the list of hidden symbols when the function definition is analysed. 
+    # Of course, our method will fail if a referenced function is defined both inside the cell **and** in another cell. However, this will lead to a MultipleDefinitionError before anything bad happens.
+    for (func, inner_symstate) in symstate.funcdefs
+        inner_symstate.references = setdiff(inner_symstate.references, keys(symstate.funcdefs))
+        inner_symstate.funccalls = setdiff(inner_symstate.funccalls, keys(symstate.funcdefs))
+    end
+    symstate
 end
 
 # TODO: this can be done during the `explore` recursion

src/react/Run.jl

@@ -12,9 +12,13 @@ function run_reactive!(notebook::Notebook, cells::Array{Cell, 1})
 
 	# update the cache using the new code and compute the new topology
 	for cell in cells
-		update_cache!(notebook, cell)
+		start_cache!(notebook, cell)
 	end
 	update_funcdefs!(notebook)
+	for cell in cells
+		finish_cache!(notebook, cell)
+	end
+
 
 	new_topology = dependent_cells(notebook, union(cells, keys(old_topology.errable)))
 	to_run = setdiff(union(new_topology.runnable, old_topology.runnable), keys(new_topology.errable)) # TODO: think if old error cell order matters
@@ -100,16 +104,25 @@ function run_single!(notebook::Notebook, cell::Cell)::Bool
 end
 
 "Update a single cell's cache - parsed code etc"
-function update_cache!(notebook::Notebook, cell::Cell)
+function start_cache!(notebook::Notebook, cell::Cell)
 	cell.parsedcode = Meta.parse(cell.code, raise=false)
 	cell.module_usings = ExploreExpression.compute_usings(cell.parsedcode)
 	cell.symstate = try
 		ExploreExpression.compute_symbolreferences(cell.parsedcode)
 	catch ex
 		@error "Expression explorer failed on: " cell.code
 		showerror(stderr, ex, stacktrace(backtrace()))
-		SymbolsState()
+		ExploreExpression.SymbolsState()
+	end
+end
+
+"Account for globals referenced in function calls by including `SymbolsState`s from called functions in the cell itself."
+function finish_cache!(notebook::Notebook, cell::Cell)
+	calls = all_recursed_calls!(notebook, cell.symstate)
+	calls = union(calls, keys(cell.symstate.funcdefs)) # _assume_ that all defined functions are called inside the cell. 
+	filter!(calls) do call
+		call in keys(notebook.combined_funcdefs)
 	end
-	cell.symstate.references = all_references(notebook, cell) # account for globals referenced in function calls
-	cell.symstate.assignments = all_assignments(notebook, cell) # account for globals assigned to in function calls
+	cell.symstate.references = union(cell.symstate.references, (notebook.combined_funcdefs[func].references for func in calls)...)
+	cell.symstate.assignments = union(cell.symstate.assignments, (notebook.combined_funcdefs[func].assignments for func in calls)...)
 end

test/ExploreExpression.jl

@@ -112,6 +112,9 @@ end
         @test testee(:(function f(x) x = x / 3; x end), [], [:f], [], [
             :f => ([:/], [], [:/])
         ])
+        @test testee(:(function f(x) a end; function f(x, y) b end), [], [:f], [], [
+            :f => ([:a, :b], [], [])
+        ])
         @test testee(:(f(x, y = a + 1) = x * y * z), [], [:f], [], [
             :f => ([:*, :z], [], [:*])
         ])