Closed jrevels closed 8 years ago
A bit ugly but harmless
From JuliaDiff/ForwardDiff.jl#56, @vtjnash said:
+ is a symbol and has scope, or you wouldn't have needed eval. usually the language semantics manage it for you (for example, functions contain a reference to the module that created it). however, current_module() is probably the wrong scope, since it will cause confusing and unexpected behavior when using it in combination with other packages (and managing to trick the current heuristic does not make it correct, just that the heuristic was not designed to catch this particular situation).
For my own education, can you give an example of this change leading to unexpected behavior? I tried to make an example myself but ended up getting the same results both before and after this PR, it makes me think I'm checking for the wrong thing...
It's less obvious with functions like +
, since they tend to be the same everywhere. Consider instead the following pseudo-code:
using Calculus
f() = simplify("sin(0)")
module One
sin(x) = 2
Base.f() # returns 2
fOne() = Base.f()
fOne() # returns 2
end
module Two
sin(x) = 3
Base.f() # returns 3
end
Base.f() # returns 0
One.fOne() # returns 0
Thanks for sharing -your example is actually uncannily similar to what I tried (even down to using sin
as the test function)! I think my mistake was that I kept extending the same function f
instead of defining new functions. Anyway, here's my example, tweaked to show the same thing:
julia> module A
using Calculus
f() = Calculus.simplify(:(sin(0)))
end
A
julia> A.f()
0.0
julia> module B
using A
sin(x) = "B's sin was called"
fB() = A.f()
println(fB())
end
B's sin was called
B
julia> B.fB() # expect "B's sin was called"
0.0 # oh noes
See JuliaDiff/ForwardDiff.jl#56