Do not expand interpolation twice, closes #9709

Author: José Valim

lib/elixir/src/elixir_bitstring.erl

@@ -132,17 +132,16 @@ compute_alignment(_, _, _) -> unknown.
 
 %% Expands the expression of a bitstring, that is, the LHS of :: or
 %% an argument of the bitstring (such as "foo" in "<<foo>>").
+%% If we are inside a match/guard, we inline interpolations explicitly,
+%% otherwise they are inlined by elixir_rewrite.erl.
 
-expand_expr(Meta, {{'.', M1, [Mod, to_string]}, M2, [Arg]}, Fun, E)
-    when Mod == 'Elixir.Kernel'; Mod == 'Elixir.String.Chars' ->
+expand_expr(_Meta, {{'.', _, [Mod, to_string]}, _, [Arg]} = AST, Fun, {#{context := Context}, _} = E)
+    when Context /= nil, (Mod == 'Elixir.Kernel') orelse (Mod == 'Elixir.String.Chars') ->
   case Fun(Arg, E) of
     {EBin, EE} when is_binary(EBin) -> {EBin, EE};
-    _ -> do_expand_expr(Meta, {{'.', M1, ['Elixir.String.Chars', to_string]}, M2, [Arg]}, Fun, E)
+    _ -> Fun(AST, E) % Let it raise
   end;
 expand_expr(Meta, Component, Fun, E) ->
-  do_expand_expr(Meta, Component, Fun, E).
-
-do_expand_expr(Meta, Component, Fun, E) ->
   case Fun(Component, E) of
     {EComponent, {ErrorE, _}} when is_list(EComponent); is_atom(EComponent) ->
       form_error(Meta, ErrorE, ?MODULE, {invalid_literal, EComponent});

lib/elixir/src/elixir_erl_pass.erl

@@ -528,29 +528,20 @@ extract_bit_type({Other, _, []}, Acc) ->
 %% Optimizations that are specific to Erlang and change
 %% the format of the AST.
 
-translate_remote('Elixir.Access' = Mod, get, Meta, [Container, Value], S) ->
-  Ann = ?ann(Meta),
-  {TArgs, SA} = translate_args([Container, Value, nil], S),
-  {?remote(Ann, Mod, get, TArgs), SA};
 translate_remote('Elixir.String.Chars', to_string, Meta, [Arg], S) ->
-  case is_always_string(Arg) of
-    true ->
-      translate(Arg, S);
-    false ->
-      {TArg, TS} = translate(Arg, S),
-      {VarName, VS} = elixir_erl_var:build('_', TS),
-
-      Generated = ?ann(?generated(Meta)),
-      Var = {var, Generated, VarName},
-      Guard = ?remote(Generated, erlang, is_binary, [Var]),
-      Slow = ?remote(Generated, 'Elixir.String.Chars', to_string, [Var]),
-      Fast = Var,
-
-      {{'case', Generated, TArg, [
-        {clause, Generated, [Var], [[Guard]], [Fast]},
-        {clause, Generated, [Var], [], [Slow]}
-      ]}, VS}
-  end;
+  {TArg, TS} = translate(Arg, S),
+  {VarName, VS} = elixir_erl_var:build('_', TS),
+
+  Generated = ?ann(?generated(Meta)),
+  Var = {var, Generated, VarName},
+  Guard = ?remote(Generated, erlang, is_binary, [Var]),
+  Slow = ?remote(Generated, 'Elixir.String.Chars', to_string, [Var]),
+  Fast = Var,
+
+  {{'case', Generated, TArg, [
+    {clause, Generated, [Var], [[Guard]], [Fast]},
+    {clause, Generated, [Var], [], [Slow]}
+  ]}, VS};
 translate_remote(maps, put, Meta, [Key, Value, Map], S) ->
   Ann = ?ann(Meta),
 
@@ -600,20 +591,6 @@ translate_remote(Left, Right, Meta, Args, S) ->
       {{call, Ann, {remote, Ann, TLeft, TRight}, TArgs}, SA}
   end.
 
-is_always_string({{'.', _, [Module, Function]}, _, Args}) ->
-  is_always_string(Module, Function, length(Args));
-%% Binary literals were already excluded in earlier passes.
-is_always_string(_Ast) ->
-  false.
-
-is_always_string('Elixir.Enum', join, _) -> true;
-is_always_string('Elixir.Enum', map_join, _) -> true;
-is_always_string('Elixir.Kernel', inspect, _) -> true;
-is_always_string('Elixir.Macro', to_string, _) -> true;
-is_always_string('Elixir.String.Chars', to_string, _) -> true;
-is_always_string('Elixir.Path', join, _) -> true;
-is_always_string(_Module, _Function, _Args) -> false.
-
 generate_struct_name_guard([{map_field_exact, Ann, {atom, _, '__struct__'} = Key, Var} | Rest], Acc, S0) ->
   {ModuleVarName, S1} = elixir_erl_var:build('_', S0),
   Generated = erl_anno:set_generated(true, Ann),

lib/elixir/src/elixir_rewrite.erl

@@ -223,10 +223,11 @@ inner_inline(_, _, _, _) -> false.
 %% as they may change the number of arguments. However, they
 %% don't add new code (such as case statements), at best they
 %% perform dead code removal.
-rewrite(?string_chars, _, to_string, _, [String]) when is_binary(String) ->
-  String;
-rewrite(?string_chars, _, to_string, _, [{{'.', _, [?kernel, inspect]}, _, _} = Call]) ->
-  Call;
+rewrite(?string_chars, DotMeta, to_string, Meta, [Arg]) ->
+  case is_always_string(Arg) of
+    true -> Arg;
+    false -> {{'.', DotMeta, [?string_chars, to_string]}, Meta, [Arg]}
+  end;
 rewrite(Receiver, DotMeta, Right, Meta, Args) ->
   {EReceiver, ERight, EArgs} = inner_rewrite(ex_to_erl, DotMeta, Receiver, Right, Args),
   {{'.', DotMeta, [EReceiver, ERight]}, Meta, EArgs}.
@@ -333,3 +334,16 @@ format_error({invalid_match, Receiver, Right, Arity}) ->
 format_error({invalid_match_append, Arg}) ->
   io_lib:format("invalid argument for ++ operator inside a match, expected a literal proper list, got: ~ts",
                 ['Elixir.Macro':to_string(Arg)]).
+
+is_always_string({{'.', _, [Module, Function]}, _, Args}) ->
+  is_always_string(Module, Function, length(Args));
+is_always_string(Ast) ->
+  is_binary(Ast).
+
+is_always_string('Elixir.Enum', join, _) -> true;
+is_always_string('Elixir.Enum', map_join, _) -> true;
+is_always_string('Elixir.Kernel', inspect, _) -> true;
+is_always_string('Elixir.Macro', to_string, _) -> true;
+is_always_string('Elixir.String.Chars', to_string, _) -> true;
+is_always_string('Elixir.Path', join, _) -> true;
+is_always_string(_Module, _Function, _Args) -> false.

lib/elixir/test/elixir/kernel/expansion_test.exs

@@ -3,6 +3,11 @@ Code.require_file("../test_helper.exs", __DIR__)
 defmodule Kernel.ExpansionTarget do
   defmacro seventeen, do: 17
   defmacro bar, do: "bar"
+
+  defmacro message_hello(arg) do
+    send(self(), :hello)
+    arg
+  end
 end
 
 defmodule Kernel.ExpansionTest do
@@ -2149,6 +2154,14 @@ defmodule Kernel.ExpansionTest do
     test "inlines binaries inside interpolation" do
       import Kernel.ExpansionTarget
 
+      # Check expansion happens only once
+      assert expand(quote(do: "foo#{message_hello("bar")}")) |> clean_meta([:alignment]) ==
+               quote(do: <<"foo"::binary(), "bar"::binary()>>)
+
+      assert_received :hello
+      refute_received :hello
+
+      # And it also works in match
       assert expand(quote(do: "foo#{bar()}" = "foobar")) |> clean_meta([:alignment]) ==
                quote(do: <<"foo"::binary(), "bar"::binary()>> = "foobar")
     end