Do not change code invocation semantics in guards

Author: José Valim

lib/elixir/lib/kernel.ex

@@ -4570,25 +4570,33 @@ defmodule Kernel do
 
   defp define_guard(kind, guard, env) do
     case :elixir_utils.extract_guards(guard) do
-      {call, impl} when length(impl) < 2 ->
+      {call, [_, _ | _]} ->
+        raise ArgumentError,
+              "invalid syntax in defguard #{Macro.to_string(call)}, " <>
+                "only a single when clause is allowed"
+
+      {call, impls} ->
         case Macro.decompose_call(call) do
           {_name, args} ->
             validate_variable_only_args!(call, args)
 
-            quoted =
-              quote do
-                require Kernel.Utils
-                Kernel.Utils.defguard(unquote(args), unquote(impl))
-              end
+            case impls do
+              [] ->
+                define(kind, call, nil, env)
+
+              [guard] ->
+                quoted =
+                  quote do
+                    require Kernel.Utils
+                    Kernel.Utils.defguard(unquote(args), unquote(guard))
+                  end
 
-            define(kind, call, [do: quoted], env)
+                define(kind, call, [do: quoted], env)
+            end
 
           _invalid_definition ->
             raise ArgumentError, "invalid syntax in defguard #{Macro.to_string(call)}"
         end
-
-      {call, _multiple_impls} ->
-        raise ArgumentError, "invalid syntax in defguard #{Macro.to_string(call)}"
     end
   end
 

lib/elixir/lib/kernel/utils.ex

@@ -208,9 +208,9 @@ defmodule Kernel.Utils do
     end)
   end
 
-  # Finds every reference to `refs` in `expr` and wraps them in an unquote.
-  defp unquote_every_ref(expr, refs) do
-    Macro.postwalk(expr, fn
+  # Finds every reference to `refs` in `guard` and wraps them in an unquote.
+  defp unquote_every_ref(guard, refs) do
+    Macro.postwalk(guard, fn
       {ref, _meta, context} = var when is_atom(ref) and is_atom(context) ->
         case {ref, context} in refs do
           true -> literal_unquote(var)
@@ -222,10 +222,10 @@ defmodule Kernel.Utils do
     end)
   end
 
-  # Prefaces `expr` with unquoted versions of `refs`.
-  defp unquote_refs_once(expr, refs) do
-    {^expr, used_refs} =
-      Macro.postwalk(expr, [], fn
+  # Prefaces `guard` with unquoted versions of `refs`.
+  defp unquote_refs_once(guard, refs) do
+    {_, used_refs} =
+      Macro.postwalk(guard, [], fn
         {ref, _meta, context} = var, acc when is_atom(ref) and is_atom(context) ->
           case {ref, context} in refs and {ref, context} not in acc do
             true -> {var, [{ref, context} | acc]}
@@ -236,14 +236,17 @@ defmodule Kernel.Utils do
           {node, acc}
       end)
 
-    for {ref, context} <- :lists.reverse(used_refs) do
-      var = {ref, [], context}
-      quote do: unquote(var) = unquote(literal_unquote(var))
-    end ++ List.wrap(expr)
+    vars = for {ref, context} <- :lists.reverse(used_refs), do: {ref, [], context}
+    exprs = for var <- vars, do: literal_unquote(var)
+
+    quote do
+      {unquote_splicing(vars)} = {unquote_splicing(exprs)}
+      unquote(guard)
+    end
   end
 
   defp literal_quote(ast) do
-    {:quote, [], [[do: {:__block__, [], List.wrap(ast)}]]}
+    {:quote, [], [[do: ast]]}
   end
 
   defp literal_unquote(ast) do

lib/elixir/test/elixir/kernel/guard_test.exs

@@ -21,6 +21,7 @@ defmodule Kernel.GuardTest do
 
     defmodule Guards.In.Funs do
       defguard is_foo(atom) when atom == :foo
+      defguard is_equal(foo, bar) when foo == bar
 
       def is_foobar(atom) when is_foo(atom) do
         is_foo(atom)
@@ -33,6 +34,13 @@ defmodule Kernel.GuardTest do
       refute Guards.In.Funs.is_foo(:bar)
     end
 
+    test "guards do not change code evaluation semantics" do
+      require Guards.In.Funs
+      x = 1
+      assert Guards.In.Funs.is_equal(x = 2, x) == false
+      assert x == 2
+    end
+
     defmodule Macros.In.Guards do
       defmacro is_foo(atom) do
         quote do
@@ -365,9 +373,7 @@ defmodule Kernel.GuardTest do
             :erlang.+(:erlang.+(unquote(foo), unquote(bar)), unquote(baz))
           end
           false -> quote do
-            foo = unquote(foo)
-            bar = unquote(bar)
-            baz = unquote(baz)
+            {foo, bar, baz} = {unquote(foo), unquote(bar), unquote(baz)}
             :erlang.+(:erlang.+(foo, bar), baz)
           end
         end
@@ -387,8 +393,7 @@ defmodule Kernel.GuardTest do
             :erlang.+(unquote(foo), unquote(bar))
           end
           false -> quote do
-            foo = unquote(foo)
-            bar = unquote(bar)
+            {foo, bar} = {unquote(foo), unquote(bar)}
             :erlang.+(foo, bar)
           end
         end
@@ -410,9 +415,7 @@ defmodule Kernel.GuardTest do
             end
           false ->
             quote() do
-              foo = unquote(foo)
-              bar = unquote(bar)
-              baz = unquote(baz)
+              {foo, bar, baz} = {unquote(foo), unquote(bar), unquote(baz)}
               :erlang.+(:erlang.+(:erlang.+(foo, foo), bar), baz)
             end
         end

lib/elixir/test/elixir/kernel/warning_test.exs

@@ -1092,7 +1092,7 @@ defmodule Kernel.WarningTest do
   end
 
   test "unused variable in defguard" do
-    capture_err(fn ->
+    assert capture_err(fn ->
       Code.eval_string("""
       defmodule Sample do
         defguard foo(bar, baz) when bar
@@ -1129,7 +1129,7 @@ defmodule Kernel.WarningTest do
   end
 
   test "defguard overriding defmacro" do
-    capture_err(fn ->
+    assert capture_err(fn ->
       Code.eval_string("""
       defmodule Sample do
         defmacro foo(bar), do: bar == :bar
@@ -1142,7 +1142,7 @@ defmodule Kernel.WarningTest do
   end
 
   test "defmacro overriding defguard" do
-    capture_err(fn ->
+    assert capture_err(fn ->
       Code.eval_string("""
       defmodule Sample do
         defguard foo(baz) when baz == :baz
@@ -1154,40 +1154,14 @@ defmodule Kernel.WarningTest do
     purge(Sample)
   end
 
-  test "defguardp overriding defmacrop" do
-    capture_err(fn ->
-      Code.eval_string("""
-      defmodule Sample do
-        defmacrop foo(bar), do: bar == :bar
-        defguardp foo(baz) when baz == :baz
-      end
-      """)
-    end) =~ "this clause cannot match because a previous clause at line 2 always matches"
-  after
-    purge(Sample)
-  end
-
-  test "defmacrop overriding defguardp" do
-    capture_err(fn ->
-      Code.eval_string("""
-      defmodule Sample do
-      defguardp foo(baz) when baz == :baz
-        defmacrop foo(bar), do: bar == :bar
-      end
-      """)
-    end) =~ "this clause cannot match because a previous clause at line 2 always matches"
-  after
-    purge(Sample)
-  end
-
   test "defguard needs an implementation" do
-    capture_err(fn ->
+    assert capture_err(fn ->
       Code.eval_string("""
       defmodule Sample do
         defguard foo(bar)
       end
       """)
-    end) =~ "implementation not provided for predefined defguard"
+    end) =~ "implementation not provided for predefined defmacro foo/1"
   after
     purge(Sample)
   end