thautwarm / MLStyle.jl

Julia functional programming infrastructures and metaprogramming facilities
https://thautwarm.github.io/MLStyle.jl/latest/
MIT License
402 stars 38 forks source link

Error message for non-exhaustive patterns #52

Open cscherrer opened 5 years ago

cscherrer commented 5 years ago

Below might be a common problem with people new to pattern matching (or rusty with it like me, apparently). It would be really nice to have a clear error message that the problem here is with a non-exhaustive pattern match :)

julia> @match :(sin(x)) quote
           :($f($(args...))) => args
       end
ERROR: LoadError: MethodError: no method matching length(::Nothing)
Closest candidates are:
  length(::Core.SimpleVector) at essentials.jl:561
  length(::Base.MethodList) at reflection.jl:801
  length(::Core.MethodTable) at reflection.jl:875
  ...
Stacktrace:
 [1] _similar_for(::UnitRange{Int64}, ::Type{Any}, ::Nothing, ::Base.HasLength) at ./array.jl:517
 [2] _collect(::UnitRange{Int64}, ::Nothing, ::Base.HasEltype, ::Base.HasLength) at ./array.jl:550
 [3] collect(::Nothing) at ./array.jl:544
 [4] (::getfield(MLStyle.MatchCore, Symbol("##50#52")){Expr,Symbol,Module})(::LineNumberNode) at /home/chad/.julia/packages/MLStyle/ksIXg/src/MatchCore.jl:249
 [5] (::getfield(MLStyle.Toolz, Symbol("##1#2")){getfield(MLStyle.MatchCore, Symbol("##50#52")){Expr,Symbol,Module}})(::MLStyle.MatchCore.config) at /home/chad/.julia/packages/MLStyle/ksIXg/src/Internal/Toolz.jl:25
 [6] $(::getfield(MLStyle.Toolz, Symbol("##1#2")){getfield(MLStyle.MatchCore, Symbol("##50#52")){Expr,Symbol,Module}}, ::MLStyle.MatchCore.config) at /home/chad/.julia/packages/MLStyle/ksIXg/src/Internal/Toolz.jl:8
 [7] (::getfield(MLStyle.Toolz, Symbol("##1#2")){getfield(MLStyle.MatchCore, Symbol("##48#49")){Expr,Module}})(::MLStyle.MatchCore.config) at /home/chad/.julia/packages/MLStyle/ksIXg/src/Internal/Toolz.jl:25
 [8] $(::getfield(MLStyle.Toolz, Symbol("##1#2")){getfield(MLStyle.MatchCore, Symbol("##48#49")){Expr,Module}}, ::MLStyle.MatchCore.config) at /home/chad/.julia/packages/MLStyle/ksIXg/src/Internal/Toolz.jl:8
 [9] gen_match(::Expr, ::Expr, ::Module) at /home/chad/.julia/packages/MLStyle/ksIXg/src/MatchCore.jl:220
 [10] @match(::LineNumberNode, ::Module, ::Any, ::Any) at /home/chad/.julia/packages/MLStyle/ksIXg/src/MatchCore.jl:238
in expression starting at REPL[62]:1
thautwarm commented 5 years ago

Hi! Thank you for your feedback here! I have to say sorry for now the error message does make some mess. Some people has requested a better error reporting, as a result, I have raised the priority of this task.

This might take some time, but I promise it'll be done until the mid-May.

Note: Refactoring and improving error messages might cause some changes of MLStyle's low level implementation. Don't use def_pattern to define your own patterns if you don't want to get a compatibility issue in the future, and anything noted in the documents won't be influenced.

thautwarm commented 5 years ago

Would you like to try the master branch out? I've improved many of the error reporting sites.

cscherrer commented 5 years ago

Thanks for the quick response on this! I expect to spend some more time working with it this weekend

thautwarm commented 5 years ago

With pleasure! Thanks for spending time with MLStyle!

cscherrer commented 5 years ago

MLStyle thinks this pattern is non-exhaustive:

julia> using MLStyle
julia> function canonicalize(expr :: Expr)
           @match expr begin
               :($x |> $f)           => :($f($x)) 
               Expr(:do, $g($x), $f) => :($g($f, $x)) 
               x                     => x
           end
       end
ERROR: LoadError: InternalException("Non-exhaustive pattern found!")
Stacktrace:
 [1] mk_gapp_pattern(::Symbol, ::Array{Any,1}, ::Expr, ::Array{Any,1}, ::Module) at /home/chad/.julia/packages/MLStyle/ksIXg/src/Pervasives.jl:502
 [2] (::getfield(MLStyle.Infras, Symbol("##9#11")))(::Symbol, ::Expr, ::Module) at /home/chad/.julia/packages/MLStyle/ksIXg/src/Infras.jl:127
 [3] mk_pattern(::Symbol, ::Expr, ::Module) at /home/chad/.julia/packages/MLStyle/ksIXg/src/MatchCore.jl:277
 [4] (::getfield(MLStyle.Pervasives, Symbol("##135#142")){Symbol,Array{Any,1},Module,Symbol,Symbol,Symbol})(::Expr) at /home/chad/.julia/packages/MLStyle/ksIXg/src/Pervasives.jl:386
 [5] foreach(::getfield(MLStyle.Pervasives, Symbol("##135#142")){Symbol,Array{Any,1},Module,Symbol,Symbol,Symbol}, ::Array{Any,1}) at ./abstractarray.jl:1866
 [6] ordered_seq_match(::Symbol, ::Array{Any,1}, ::Module) at /home/chad/.julia/packages/MLStyle/ksIXg/src/Pervasives.jl:361
 [7] (::getfield(MLStyle.Pervasives, Symbol("##101#106")))(::Symbol, ::Type, ::Array{Any,1}, ::Module) at /home/chad/.julia/packages/MLStyle/ksIXg/src/Pervasives.jl:275
 [8] mk_app_pattern(::Symbol, ::Symbol, ::Array{Any,1}, ::Module) at /home/chad/.julia/packages/MLStyle/ksIXg/src/Infras.jl:93
 [9] (::getfield(MLStyle.Infras, Symbol("##9#11")))(::Symbol, ::Expr, ::Module) at /home/chad/.julia/packages/MLStyle/ksIXg/src/Infras.jl:127
 [10] mk_pattern(::Symbol, ::Expr, ::Module) at /home/chad/.julia/packages/MLStyle/ksIXg/src/MatchCore.jl:277
 [11] (::getfield(MLStyle.MatchCore, Symbol("##51#53")){Symbol,Symbol,Module})(::Tuple{LineNumberNode,Expr,Expr}, ::Expr) at /home/chad/.julia/packages/MLStyle/ksIXg/src/MatchCore.jl:252
 [12] mapfoldr_impl(::typeof(identity), ::getfield(MLStyle.MatchCore, Symbol("##51#53")){Symbol,Symbol,Module}, ::NamedTuple{(:init,),Tuple{Expr}}, ::Array{Any,1}, ::Int64) at ./reduce.jl:105
 [13] #mapfoldr#189(::Base.Iterators.Pairs{Symbol,Expr,Tuple{Symbol},NamedTuple{(:init,),Tuple{Expr}}}, ::Function, ::Function, ::Function, ::Array{Any,1}) at ./reduce.jl:125
 [14] #mapfoldr at ./none:0 [inlined]
 [15] #foldr#190 at ./reduce.jl:144 [inlined]
 [16] (::getfield(Base, Symbol("#kw##foldr")))(::NamedTuple{(:init,),Tuple{Expr}}, ::typeof(foldr), ::Function, ::Array{Any,1}) at ./none:0
 [17] (::getfield(MLStyle.MatchCore, Symbol("##50#52")){Symbol,Symbol,Module})(::LineNumberNode) at /home/chad/.julia/packages/MLStyle/ksIXg/src/MatchCore.jl:250
 [18] (::getfield(MLStyle.Toolz, Symbol("##1#2")){getfield(MLStyle.MatchCore, Symbol("##50#52")){Symbol,Symbol,Module}})(::MLStyle.MatchCore.config) at /home/chad/.julia/packages/MLStyle/ksIXg/src/Internal/Toolz.jl:25
 [19] $(::getfield(MLStyle.Toolz, Symbol("##1#2")){getfield(MLStyle.MatchCore, Symbol("##50#52")){Symbol,Symbol,Module}}, ::MLStyle.MatchCore.config) at /home/chad/.julia/packages/MLStyle/ksIXg/src/Internal/Toolz.jl:8
 [20] (::getfield(MLStyle.Toolz, Symbol("##1#2")){getfield(MLStyle.MatchCore, Symbol("##48#49")){Symbol,Module}})(::MLStyle.MatchCore.config) at /home/chad/.julia/packages/MLStyle/ksIXg/src/Internal/Toolz.jl:25
 [21] $(::getfield(MLStyle.Toolz, Symbol("##1#2")){getfield(MLStyle.MatchCore, Symbol("##48#49")){Symbol,Module}}, ::MLStyle.MatchCore.config) at /home/chad/.julia/packages/MLStyle/ksIXg/src/Internal/Toolz.jl:8
 [22] gen_match(::Symbol, ::Expr, ::Module) at /home/chad/.julia/packages/MLStyle/ksIXg/src/MatchCore.jl:220
 [23] @match(::LineNumberNode, ::Module, ::Any, ::Any) at /home/chad/.julia/packages/MLStyle/ksIXg/src/MatchCore.jl:238
in expression starting at REPL[58]:3
cscherrer commented 5 years ago

Oops I got it, should have been

function canonicalize(expr :: Expr)
    @match expr begin
        :($x |> $f)                 => :($f($x)) 
        Expr(:do, :($g($x)), :($f)) => :($g($f, $x)) 
        x                           => x
    end
end

So the above error should really be "$ outside expression" or something

thautwarm commented 5 years ago

Oh, thanks for this feedback, and we're to solve it sooner!

thautwarm commented 5 years ago

Actually there's a possibility that users have defined their own $ patterns outside expressions, although it must be rare and not recommended. Also, as there is no $ pattern in MLStyle's built-in patterns, I think the error message should be something like "No pattern $g. Are you using $ outside expression unexpectedly?"

thautwarm commented 5 years ago

Oh, it's positioned as a bug now, I'll get it done at once. But the last release might not come out very recently.

thautwarm commented 5 years ago

@cscherrer Hi Chad, now the error message becomes

LoadError: "Deconstructor cannot be an expression like \$(Expr(:\$, :g))."

with corresponding position info.

It could be explained with that $g(...) is not a deconstructor. The current problem is that string(Expr(:$, :g)) is "\$(Expr(:\$, :g))" instead of "$g", so it still gets a bit confusing.

thautwarm commented 5 years ago

Hi Chad, is ERROR: LoadError: Deconstructor cannot be an expression like $(Expr(:$, :g)). okay?

thautwarm commented 5 years ago

Planning to release v0.3.1 for introducing some new features during my recent talk.