search

JuliaServices / Match.jl

Advanced Pattern Matching for Julia
https://juliaservices.github.io/Match.jl/latest/
Other
240 stars 22 forks source link

Fix deprecated use of union() on arrays #54

Closed epatters closed 5 years ago

epatters commented 6 years ago

This usage of union() is deprecated in Julia v0.7 and removed in Julia v1.0.

cpressey commented 5 years ago

Unless I'm mistaken, this fix addresses issue #53 ? (which I believe I ran up against recently, while trying to update some older code that uses Match.jl, to Julia v1.0.)

briochemc commented 5 years ago

Well it does not solve my problem actually... I get an error if the output uses a function with no argument. I'm not even sure this is related at all to this PR. Should I raise a new issue? MWE: 

julia> f() = "no argument"
f (generic function with 1 method)

julia> f(x) = "1 argument"
f (generic function with 2 methods)

julia> g(x) = @match x begin # if `f` has an argument, this works:
           1 => x
           2 => f(x)
       end
g (generic function with 1 method)

julia> g(x) = @match x begin # if `f` has no argument, this does not work:
           1 => x
           2 => f()
       end
ERROR: LoadError: MethodError: no method matching union()
Closest candidates are:
  union(::Pkg.Pkg2.Pkg2Types.VersionSet, ::Pkg.Pkg2.Pkg2Types.VersionSet) at /Users/osx/buildbot/slave/package_osx64/build/usr/share/julia/stdlib/v1.0/Pkg/src/Pkg2/types.jl:130
  union(::Pkg.Types.VersionSpec, ::Pkg.Types.VersionSpec) at /Users/osx/buildbot/slave/package_osx64/build/usr/share/julia/stdlib/v1.0/Pkg/src/versions.jl:223
  union(::BitSet, ::Any...) at bitset.jl:272
  ...
Stacktrace:
 [1] getvars(::Array{Any,1}) at /Users/benoitpasquier/.julia/packages/Match/M8WAy/src/matchutils.jl:76
 [2] getvars(::Expr) at /Users/benoitpasquier/.julia/packages/Match/M8WAy/src/matchutils.jl:71
 [3] gen_match_expr(::Symbol, ::Expr, ::Symbol, ::Bool) at /Users/benoitpasquier/.julia/packages/Match/M8WAy/src/matchmacro.jl:365
 [4] gen_match_expr(::Symbol, ::Expr, ::Symbol) at /Users/benoitpasquier/.julia/packages/Match/M8WAy/src/matchmacro.jl:348
 [5] @match(::LineNumberNode, ::Module, ::Any, ::Any) at /Users/benoitpasquier/.julia/packages/Match/M8WAy/src/matchmacro.jl:434
in expression starting at REPL[6]:1

[EDIT] : I used the wrong branch of the code for this MWE. The PR works for me! :)

epatters commented 5 years ago

My understanding is that the new code should have the same behavior as the original code on older versions of Julia. Please correct me if I'm wrong.

I don't whether this will fix #53. I do know that this fixed an error I encountered when porting some of my old code to Julia v1.0. Unfortunately, the test suite for this library does not seem to exercise the line in question at all.

cpressey commented 5 years ago

@briochemc Can you double-check that you've got the correct branch (epatters:union-deprecated) checked out? According to this GitHub search there is only one instance of union in the Match.jl repo, and this patch removes it -- but your error message still says no method matching union() -- so I strongly suspect you're testing with a different version of the code.

briochemc commented 5 years ago

I strongly suspect you're testing with a different version of the code.

You are totally right I just assumed your modified line was on the master branch :) Let me try this right now!

[EDIT]: Yes you were correct this fixed my issue!

briochemc commented 5 years ago

@kmsquire any reason this has not been merged yet?

kmsquire commented 5 years ago

Sorry for not merging sooner. I'll release a new version in a little while.