Fix regression when case is used inside cond head

Author: José Valim

lib/elixir/src/elixir_clauses.erl

@@ -79,7 +79,7 @@ do_clauses(Meta, DecoupledClauses, Return, S) ->
   % Transform tree just passing the variables counter forward
   % and storing variables defined inside each clause.
   Transformer = fun(X, {SAcc, VAcc}) ->
-    {TX, TS} = each_clause(X, Return, SAcc),
+    {TX, TS} = each_clause(Meta, X, Return, SAcc),
     {TX, {elixir_scope:mergec(S, TS), [TS#elixir_scope.export_vars|VAcc]}}
   end,
 
@@ -139,15 +139,27 @@ expand_clauses(_Line, [], [], _FinalVars, Acc, S) ->
 
 % Handle each key/value clause pair and translate them accordingly.
 
-each_clause({match, Meta, [Condition], Expr}, Return, S) ->
+each_clause(Export, {match, Meta, [Condition], Expr}, Return, S) ->
+  Fun = wrap_export_fun(Export, fun elixir_translator:translate_args/2),
   {Arg, Guards} = extract_guards(Condition),
-  clause(?line(Meta), fun elixir_translator:translate_args/2, [Arg], Expr, Guards, Return, S);
+  clause(?line(Meta), Fun, [Arg], Expr, Guards, Return, S);
 
-each_clause({expr, Meta, [Condition], Expr}, Return, S) ->
-  {TCondition, SC} = elixir_translator:translate(Condition, S),
+each_clause(Export, {expr, Meta, [Condition], Expr}, Return, S) ->
+  {TCondition, SC} = (wrap_export_fun(Export, fun elixir_translator:translate/2))(Condition, S),
   {TExpr, SB} = elixir_translator:translate_block(Expr, Return, SC),
   {{clause, ?line(Meta), [TCondition], [], unblock(TExpr)}, SB}.
 
+wrap_export_fun(Meta, Fun) ->
+  case lists:keyfind(export_head, 1, Meta) of
+    {export_head, true} ->
+      Fun;
+    _ ->
+      fun(Args, S) ->
+        {TArgs, TS} = Fun(Args, S),
+        {TArgs, TS#elixir_scope{export_vars = S#elixir_scope.export_vars}}
+      end
+  end.
+
 % Check if the given expression is a match tuple.
 % This is a small optimization to allow us to change
 % existing assignments instead of creating new ones every time.
@@ -176,7 +188,7 @@ normalize_vars(Key, Value, #elixir_scope{vars=Vars,export_vars=ClauseVars} = S)
   VS = S#elixir_scope{
     vars=orddict:store(Key, Value, Vars),
     export_vars=orddict:store(Key, Value, ClauseVars)
- },
+  },
 
   Expr = case orddict:find(Key, Vars) of
     {ok, {PreValue, _}} -> {var, 0, PreValue};

lib/elixir/src/elixir_def.erl

@@ -168,16 +168,8 @@ translate_definition(Kind, Line, Module, Name, Args, Guards, Body, E) when is_in
 
   Body == nil andalso check_args_for_bodyless_clause(Line, EArgs, E),
 
-  %% Macros receive a special argument on invocation. Notice it does
-  %% not affect the arity of the stored function, but the clause
-  %% already contains it.
-  EAllArgs = case is_macro(Kind) of
-    true  -> [{'_@CALLER', [{line,Line}], nil}|EArgs];
-    false -> EArgs
-  end,
-
   S = elixir_env:env_to_scope(E),
-  {Unpacked, Defaults} = elixir_def_defaults:unpack(Kind, Name, EAllArgs, S),
+  {Unpacked, Defaults} = elixir_def_defaults:unpack(Kind, Name, EArgs, S),
   {Clauses, Super} = translate_clause(Body, Line, Kind, Unpacked, EGuards, EBody, S),
 
   run_on_definition_callbacks(Kind, Line, Module, Name, EArgs, EGuards, EBody, E),
@@ -192,22 +184,30 @@ translate_clause(_, Line, Kind, Args, Guards, Body, S) ->
   {TClause, TS} = elixir_clauses:clause(Line,
     fun elixir_translator:translate_args/2, Args, Body, Guards, true, S),
 
-  %% Set __CALLER__ if used
-  FClause = case is_macro(Kind) andalso TS#elixir_scope.caller of
-    true  ->
-      FBody = {'match', Line,
-        {'var', Line, '__CALLER__'},
-        elixir_utils:erl_call(Line, elixir_env, linify, [{var, Line, '_@CALLER'}])
-     },
-      setelement(5, TClause, [FBody|element(5, TClause)]);
-    false -> TClause
+  FClause = case is_macro(Kind) of
+    true ->
+      FArgs = {var, Line, '_@CALLER'},
+      MClause = setelement(3, TClause, [FArgs|element(3, TClause)]),
+
+      case TS#elixir_scope.caller of
+        true  ->
+          FBody = {'match', Line,
+            {'var', Line, '__CALLER__'},
+            elixir_utils:erl_call(Line, elixir_env, linify, [{var, Line, '_@CALLER'}])
+          },
+          setelement(5, MClause, [FBody|element(5, TClause)]);
+        false ->
+          MClause
+      end;
+    false ->
+      TClause
   end,
 
   {[FClause], TS#elixir_scope.super}.
 
-expr_from_body(_Line, nil)            -> nil;
+expr_from_body(_Line, nil)          -> nil;
 expr_from_body(_Line, [{do, Expr}]) -> Expr;
-expr_from_body(Line, Else)            -> {'try', [{line,Line}], [Else]}.
+expr_from_body(Line, Else)          -> {'try', [{line,Line}], [Else]}.
 
 is_macro(defmacro)  -> true;
 is_macro(defmacrop) -> true;

lib/elixir/src/elixir_def_defaults.erl

@@ -21,17 +21,19 @@ unpack(Kind, Name, Args, S) ->
 %% Unpack default from given args.
 %% Returns the given arguments without their default
 %% clauses and a list of clauses for the default calls.
-unpack_each(Kind, Name, [{'\\\\', Line, [Expr, _]}|T] = List, Acc, Clauses, S) ->
-  Base = build_match(Acc, Line, []),
-  {Args, Invoke} = extract_defaults(List, Line, length(Base), [], []),
+unpack_each(Kind, Name, [{'\\\\', DefMeta, [Expr, _]}|T] = List, Acc, Clauses, S) ->
+  Base = wrap_kind(Kind, build_match(Acc, [])),
+  {Args, Invoke} = extract_defaults(List, length(Base), [], []),
 
   {DefArgs, SA}  = elixir_clauses:match(fun elixir_translator:translate_args/2, Base ++ Args, S),
   {DefInvoke, _} = elixir_translator:translate_args(Base ++ Invoke, SA),
 
+  Line = ?line(DefMeta),
+
   Call = {call, Line,
     {atom, Line, name_for_kind(Kind, Name)},
     DefInvoke
- },
+  },
 
   Clause = {clause, Line, DefArgs, [], [Call]},
   unpack_each(Kind, Name, T, [Expr|Acc], [Clause|Clauses], S);
@@ -44,25 +46,28 @@ unpack_each(_Kind, _Name, [], Acc, Clauses, _S) ->
 
 % Extract default values from args following the current default clause.
 
-extract_defaults([{'\\\\', _, [_Expr, Default]}|T], Line, Counter, NewArgs, NewInvoke) ->
-  extract_defaults(T, Line, Counter, NewArgs, [Default|NewInvoke]);
+extract_defaults([{'\\\\', _, [_Expr, Default]}|T], Counter, NewArgs, NewInvoke) ->
+  extract_defaults(T, Counter, NewArgs, [Default|NewInvoke]);
 
-extract_defaults([_|T], Line, Counter, NewArgs, NewInvoke) ->
-  H = {elixir_utils:atom_concat(["_@D", Counter]), Line, nil},
-  extract_defaults(T, Line, Counter + 1, [H|NewArgs], [H|NewInvoke]);
+extract_defaults([_|T], Counter, NewArgs, NewInvoke) ->
+  H = {elixir_utils:atom_concat(["x", Counter]), [], nil},
+  extract_defaults(T, Counter + 1, [H|NewArgs], [H|NewInvoke]);
 
-extract_defaults([], _Line, _Counter, NewArgs, NewInvoke) ->
+extract_defaults([], _Counter, NewArgs, NewInvoke) ->
   {lists:reverse(NewArgs), lists:reverse(NewInvoke)}.
 
 % Build matches for all the previous argument until the current default clause.
 
-build_match([], _Line, Acc) -> Acc;
+build_match([], Acc) -> Acc;
 
-build_match([_|T], Line, Acc) ->
-  Var = {elixir_utils:atom_concat(["_@D", length(T)]), Line, nil},
-  build_match(T, Line, [Var|Acc]).
+build_match([_|T], Acc) ->
+  Var = {elixir_utils:atom_concat(["x", length(T)]), [], nil},
+  build_match(T, [Var|Acc]).
 
 % Given the invoked function name based on the kind
 
+wrap_kind(Kind, Args) when Kind == defmacro; Kind == defmacrop -> [{c, [], nil}|Args];
+wrap_kind(_Kind, Args) -> Args.
+
 name_for_kind(Kind, Name) when Kind == defmacro; Kind == defmacrop -> elixir_utils:macro_name(Name);
 name_for_kind(_Kind, Name) -> Name.

lib/elixir/src/elixir_exp.erl

@@ -283,7 +283,7 @@ expand({'_', _, Kind} = Var, E) when is_atom(Kind) ->
 expand({Name, Meta, Kind} = Var, #{context := match, export_vars := Export} = E) when is_atom(Name), is_atom(Kind) ->
   Pair      = {Name, var_kind(Meta, Kind)},
   NewVars   = ordsets:add_element(Pair, ?m(E, vars)),
-  NewExport = case (Export /= nil) andalso (lists:keyfind(export, 1, Meta) /= {export, false}) of
+  NewExport = case (Export /= nil) of
     true  -> ordsets:add_element(Pair, Export);
     false -> Export
   end,

lib/elixir/src/elixir_exp_clauses.erl

@@ -171,17 +171,6 @@ normalize_rescue(Other) ->
 
 %% Expansion helpers
 
-export_vars({Left, Meta, Right}) when is_atom(Left), is_list(Meta), is_atom(Right) ->
-  {Left, [{export,false}|Meta], Right};
-export_vars({Left, Meta, Right}) ->
-  {export_vars(Left), Meta, export_vars(Right)};
-export_vars({Left, Right}) ->
-  {export_vars(Left), export_vars(Right)};
-export_vars(List) when is_list(List) ->
-  [export_vars(X) || X <- List];
-export_vars(Other) ->
-  Other.
-
 %% Returns a function that expands arguments
 %% considering we have at maximum one entry.
 expand_one(Meta, Kind, Key, Fun) ->
@@ -197,8 +186,13 @@ expand_one(Meta, Kind, Key, Fun) ->
 expand_with_export(Meta, Kind, Fun, {Key, Clauses}, Acc, E) when is_list(Clauses) ->
   EFun =
     case lists:keyfind(export_head, 1, Meta) of
-      {export_head, true} -> Fun;
-      _ -> fun(ExportArgs, ExportE) -> Fun(export_vars(ExportArgs), ExportE) end
+      {export_head, true} ->
+        Fun;
+      _ ->
+        fun(Args, #{export_vars := ExportVars} = EE) ->
+          {FArgs, FE} = Fun(Args, EE),
+          {FArgs, FE#{export_vars := ExportVars}}
+        end
     end,
   Transformer = fun(Clause, Vars) ->
     {EClause, EC} = clause(Meta, Kind, EFun, Clause, E),

lib/elixir/src/elixir_scope.erl

@@ -27,29 +27,27 @@ translate_var(Meta, Name, Kind, S) when is_atom(Kind); is_integer(Kind) ->
         true ->
           {{var, Line, Current}, S};
         false ->
-          %% If the variable is not exported, we use a counter name.
-          %% The same if the variable already exists or we are in a
-          %% noname context.
-          Private = (lists:keyfind(export, 1, Meta) == {export, false}),
-
+          %% We attempt to give vars a nice name because we
+          %% still use the unused vars warnings from erl_lint.
+          %%
+          %% Once we move the warning to Elixir compiler, we
+          %% can name vars as _@COUNTER.
           {NewVar, Counter, NS} =
             if
               Kind /= nil ->
                 build_var('_', S);
-              Private orelse Exists orelse S#elixir_scope.noname ->
-                build_var(Name, S);
               true ->
-                {Name, 0, S}
+                build_var(Name, S)
             end,
 
           FS = NS#elixir_scope{
             vars=orddict:store(Tuple, {NewVar, Counter}, Vars),
             match_vars=ordsets:add_element(Tuple, MatchVars),
             export_vars=case S#elixir_scope.export_vars of
-              EV when Private; EV == nil -> EV;
-              EV -> orddict:store(Tuple, {NewVar, Counter}, EV)
+              nil -> nil;
+              EV  -> orddict:store(Tuple, {NewVar, Counter}, EV)
             end
-         },
+          },
 
           {{var, Line, NewVar}, FS}
       end;

lib/elixir/src/elixir_translator.erl

@@ -392,7 +392,7 @@ build_truthy_clause(Meta, Condition, Body) ->
     true ->
       {{'->', Meta, [[true], Body]}, false};
     false ->
-      Var  = {'cond', [{export, false}], 'Elixir'},
+      Var  = {'cond', [], 'Elixir'},
       Head = {'when', [], [Var,
         {'__op__', [], [
           'andalso',

lib/elixir/test/elixir/kernel/case_test.exs

@@ -37,6 +37,17 @@ defmodule Kernel.CaseTest do
     refute 1.0 in [1, 2, 3], "not in assertion"
   end
 
+  test :in_cond_clause do
+    assert (cond do
+      format() && (f = format()) ->
+        f
+      true ->
+        :text
+    end) == :html
+  end
+
+  defp format, do: :html
+
   defp vars_case(x, vx) do
     case x > 400 do
       true ->