Defer generating labels in the automaton to prevent duplicate labels …

…being generated (#105)

* implify the code a bit before fixing #102
* Fix #102 by deferring the creation of a label until code generation.
* Extract method for checking for a macro.
* Bump version number.

Project.toml

@@ -1,6 +1,6 @@
 name = "Match"
 uuid = "7eb4fadd-790c-5f42-8a69-bfa0b872bfbf"
-version = "2.1.0"
+version = "2.1.1"
 authors = ["Neal Gafter <[email protected]>", "Kevin Squire <[email protected]>"]
 
 [deps]

src/lowering.jl

@@ -10,7 +10,7 @@ function assignments(assigned::ImmutableDict{Symbol, Symbol})
 end
 
 function code(e::BoundExpression)
-    value = Expr(:block, e.location, e.source)
+    value = Expr(:block, e.location, code_for_expression(source(e)))
     assignments = Expr(:block, (:($k = $v) for (k, v) in e.assignments)...)
     return Expr(:let, assignments, value)
 end
@@ -31,15 +31,16 @@ end
 function code(bound_pattern::BoundTypeTestPattern, binder::BinderContext)
     # We assert that the type is invariant.  Because this mutates binder.assertions,
     # you must take the value of binder.assertions after all calls to the generated code.
-    if bound_pattern.source != bound_pattern.type && !(bound_pattern.source in binder.asserted_types)
-        test = :($(bound_pattern.type) == $(bound_pattern.source))
+    src = source(bound_pattern)
+    if src != bound_pattern.type && !(src in binder.asserted_types)
+        test = :($(bound_pattern.type) == $src)
         thrown = :($throw($AssertionError($string($(string(bound_pattern.location.file)),
             ":", $(bound_pattern.location.line),
-            ": The type syntax `::", $(string(bound_pattern.source)), "` bound to type ",
+            ": The type syntax `::", $(string(src)), "` bound to type ",
             $string($(bound_pattern.type)), " at macro expansion time but ",
-             $(bound_pattern.source), " later."))))
+             $src, " later."))))
         push!(binder.assertions, Expr(:block, bound_pattern.location, :($test || $thrown)))
-        push!(binder.asserted_types, bound_pattern.source)
+        push!(binder.asserted_types, )
     end
     :($(bound_pattern.input) isa $(bound_pattern.type))
 end

src/match_cases_opt.jl

@@ -301,10 +301,12 @@ function remove(action::BoundIsMatchTestPattern, action_result::Bool, pattern::B
     # As a special case, if the input variable is of type Bool, then we know that true and false
     # are the only values it can hold.
     type = get!(() -> Any, binder.types, action.input)
-    if type == Bool && action.bound_expression.source isa Bool && pattern.bound_expression.source isa Bool
-        @assert action.bound_expression.source != pattern.bound_expression.source # because we already checked for equality
+    action_src = source(action.bound_expression)
+    pattern_src = source(pattern.bound_expression)
+    if type == Bool && action_src isa Bool && pattern_src isa Bool
+        @assert action_src != pattern_src # because we already checked for equality
         # If the one succeeded, then the other one fails
-        return BoundBoolPattern(loc(pattern), source(pattern), !action_result)
+        return BoundBoolPattern(loc(pattern), pattern_src, !action_result)
     end
 
     return pattern

src/match_return.jl

@@ -76,42 +76,89 @@ end
 # in which `MatchFailure` is a possible result.
 #
 function adjust_case_for_return_macro(__module__, location, pattern, result, predeclared_temps)
-    value = gensym("value")
-    label = gensym("label")
     found_early_exit::Bool = false
+
+    # Check for the presence of early exit macros @match_return and @match_fail
     function adjust_top(p)
-        is_expr(p, :macrocall) || return p
-        if length(p.args) == 3 &&
-            (p.args[1] == :var"@match_return"  || p.args[1] == Expr(:., Symbol(string(@__MODULE__)), QuoteNode(:var"@match_return")))
+        if is_macro(p, :var"@match_return", 3)
             # :(@match_return e) -> :($value = $e; @goto $label)
             found_early_exit = true
-            return Expr(:block, p.args[2], :($value = $(p.args[3])), :(@goto $label))
-        elseif length(p.args) == 2 &&
-            (p.args[1] == :var"@match_fail"  || p.args[1] == Expr(:., Symbol(string(@__MODULE__)), QuoteNode(:var"@match_fail")))
+            # expansion of the result will be done later by ExpressionRequiringAdjustmentForReturnMacro 
+            return p
+        elseif is_macro(p, :var"@match_fail", 2)
             # :(@match_fail) -> :($value = $MatchFaulure; @goto $label)
             found_early_exit = true
-            return Expr(:block, p.args[2], :($value = $MatchFailure), :(@goto $label))
-        elseif length(p.args) == 4 &&
-            (p.args[1] == :var"@match" || p.args[1] == Expr(:., Symbol(string(@__MODULE__)), QuoteNode(:var"@match")) ||
-             p.args[1] == :var"@match" || p.args[1] == Expr(:., Symbol(string(@__MODULE__)), QuoteNode(:var"@match")))
+            # expansion of the result will be done later by ExpressionRequiringAdjustmentForReturnMacro 
+            return p
+        elseif is_macro(p, :var"@match", 4)
             # Nested uses of @match should be treated as independent
             return macroexpand(__module__, p)
-        else
+        elseif is_expr(p, :macrocall)
             # It is possible for a macro to expand into @match_fail, so only expand one step.
             return adjust_top(macroexpand(__module__, p; recursive = false))
+        else
+            return p
         end
     end
 
     rewritten_result = MacroTools.prewalk(adjust_top, result)
     if found_early_exit
         # Since we found an early exit, we need to predeclare the temp to ensure
         # it is in scope both for where it is written and in the constructed where clause.
-        push!(predeclared_temps, value)
-        where_expr = Expr(:block, location, :($value = $rewritten_result), :(@label $label), :($value !== $MatchFailure))
+        value_symbol = gensym("value")
+        push!(predeclared_temps, value_symbol)
+
+        # Defer generation of the label and branch, so we get a unique label in case it needs to be generated more than once.
+        where_expr = where_expression_requiring_adjustment_for_return_macro(value_symbol, location, rewritten_result)
         new_pattern = :($pattern where $where_expr)
-        new_result = value
+        new_result = value_symbol
         (new_pattern, new_result)
     else
         (pattern, rewritten_result)
     end
 end
+
+function is_macro(x, name::Symbol, arity::Int)
+    return is_expr(x, :macrocall) && ## length(x.args) == arity &&
+        (x.args[1] == name ||
+         x.args[1] == Expr(:., Symbol(string(@__MODULE__)), QuoteNode(name)))
+end
+
+const marker_for_where_expression_requiring_adjustment_for_return_macro =
+    :where_expression_requiring_adjustment_for_return_macro
+
+# We defer generating the code for the where clause with the label, because
+# the state machine may require it to be generated more than once, and each
+# generated version must use a fresh new label to avoid a duplicate label
+# in the generated code.
+function where_expression_requiring_adjustment_for_return_macro(
+    value_symbol::Symbol,
+    location,
+    expression_containing_macro)
+    Expr(marker_for_where_expression_requiring_adjustment_for_return_macro,
+         value_symbol, location, expression_containing_macro)
+end
+
+# See doc for where_expression_requiring_adjustment_for_return_macro, above
+code_for_expression(x) = x
+function code_for_expression(x::Expr)
+    x.head == marker_for_where_expression_requiring_adjustment_for_return_macro || return x
+    value_symbol, location, expression_containing_macro = x.args
+    label = gensym("early_label")
+
+    function adjust_top(result)
+        if is_macro(result, :var"@match_return", 3)
+            # :(@match_return e) -> :($value = $e; @goto $label)
+            return Expr(:block, result.args[2], :($value_symbol = $(result.args[3])), :(@goto $label))
+        elseif is_macro(result, :var"@match_fail", 2)
+            # :(@match_fail) -> :($value = $MatchFaulure; @goto $label)
+            return Expr(:block, result.args[2], :($value_symbol = $MatchFailure), :(@goto $label))
+        else
+            return result 
+        end
+    end
+
+    rewritten_result = MacroTools.prewalk(adjust_top, expression_containing_macro)
+    Expr(:block, location, :($value_symbol = $rewritten_result),
+         :(@label $label), :($value_symbol !== $MatchFailure))
+end

test/match_return.jl

@@ -126,4 +126,20 @@ end
     end) == 2
 end
 
+foo(x) = x == 9
+@testset "Test for presence of bug https://github.com/JuliaServices/Match.jl/issues/102" begin
+    @test (@match Foo(1, 2) begin
+        Foo(_, _) where (foo(1) && foo(2)) => 15
+        Foo(_, _) where foo(7) =>
+            begin
+                if foo(9)
+                    @match_return 16
+                end
+                17
+            end
+        Foo(_, _) where (foo(1) && foo(3)) => 18
+        _ => 43
+    end) == 43
+end
+
 end