MLStyle.jl icon indicating copy to clipboard operation
MLStyle.jl copied to clipboard

Matching enum

Open jtrakk opened this issue 4 years ago • 6 comments

Sometimes I want to match on an enum.

using MLStyle

@enum Var left middle right

@match right begin
    left => 0
    middle => 1
    right => 2
end

This gives the result 0, but I want 2. As documented in pattern matching for Julia enums I can define

using MLStyle.AbstractPatterns: literal
MLStyle.is_enum(::Var) = true
MLStyle.pattern_uncall(e::Var, _, _, _, _) = literal(e)

to get the right answer. But I'm nervous about using this. If I forget to define these methods, or define them in the wrong place so they're not executed yet, I'll get the wrong answer.

I would prefer to get either the right answer or an error rather than the wrong answer. What would be a good solution here?

jtrakk avatar Jun 10 '21 20:06 jtrakk

A possible option is to report you that the global variable left is used as a capture pattern.

using MLStyle

MLStyle_AllowShadow = false
@enum Var left middle right

@match right begin
    left => 0
    middle => 1
    right => 2
end

┌ Error: global variables used as capture patterns: left, middle, right.
└ @ Main xxx:xxx

Could this suffice?

thautwarm avatar Jun 26 '21 13:06 thautwarm

I'm an unsophisticated user -- I don't understand why it gives 0 in my example or what MLStyle_AllowShadow means. I just want to make sure that I don't get the wrong answer accidentally.

  1. If the suggestion is that a user like me would write MLStyle_AllowShadow = false, I don't like that solution, because if I forget to write it then I will get the wrong answer. If I remembered that I need to write something, I would just copy-paste
using MLStyle.AbstractPatterns: literal
MLStyle.is_enum(::Var) = true
MLStyle.pattern_uncall(e::Var, _, _, _, _) = literal(e)

instead so I get the right answer instead of an error.

  1. On the other hand, if I could just get the error by default, I would be happy with that solution.

  2. I guess the reason I have to copy-paste those lines into my code is so that MLStyle.jl macros don't need any MLStyle.jl functions at runtime? For my use cases I wouldn't mind invoking your functions at runtime if it means I wouldn't have to copy-paste any code. But maybe that violates the MLStyle.jl philosophy and would need to be a separate package.

jtrakk avatar Jun 26 '21 18:06 jtrakk

Thanks for this explanation. I understand your concerns, but unfortunately it's hard to achieve what you prefer. In your example, you use left as a pattern, but MLStyle cannot distinguish whether you expect to bind a variable left or compare the global variable left with your match target. MLStyle takes the way that all symbols in a pattern are default to be variable binding, namely capture patterns. The only exception occurs when is_enum(left) == true. If you want to treat left directly as a pattern, it's yet another protocol so that you need tell MLStyle about it.

thautwarm avatar Jul 04 '21 17:07 thautwarm

Could MLStyle do something like this?

MLStyle.is_enum(::T) where {T<:Enum} = true
MLStyle.pattern_uncall(e::T, _, _, _, _) where {T<:Enum} = literal(e)

It seems to work in this case

julia> @match right begin
           left => 0
           middle => 1
           right => 2
       end
2

I think I understand your explanation. For other types that aren't known to MLStyle, like SumTypes.jl, it doesn't know whether I mean the existing name or a new name, and the "AllowShadow = false` suggestion would disambiguate. I think https://github.com/thautwarm/MLStyle.jl/issues/127 would also avoid this accident, maybe that is a solution?

jtrakk avatar Jul 04 '21 19:07 jtrakk

MLStyle 0.5 will support Base.Enum as a builtin pattern.

thautwarm avatar Jul 19 '22 04:07 thautwarm

This just bit me, I didn't realise this silently gave the wrong results for enums. If you can't make matching on an enum error, might I suggest adding the warning you have in the README to https://thautwarm.github.io/MLStyle.jl/latest/index.html as well as I went to check there first and didn't look at the README.

Zentrik avatar Dec 19 '23 15:12 Zentrik