Skip to content

Commit

Permalink
Avoid nesting of capture inside macros
Browse files Browse the repository at this point in the history
Unfortunately this makes it so the unused capture
warnings emit false positives, so this particular
warning was removed.

Closes #13609.
  • Loading branch information
josevalim committed Jun 1, 2024
1 parent a7bf120 commit 53c93b9
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 31 deletions.
6 changes: 2 additions & 4 deletions lib/elixir/lib/macro.ex
Original file line number Diff line number Diff line change
Expand Up @@ -495,10 +495,8 @@ defmodule Macro do
"""
@doc since: "1.11.3"
@spec generate_unique_arguments(0, context :: atom) :: []
@spec generate_unique_arguments(pos_integer, context) :: [
{atom, [counter: integer], context},
...
]
@spec generate_unique_arguments(pos_integer, context) ::
[{atom, [counter: integer], context}, ...]
when context: atom
def generate_unique_arguments(amount, context),
do: generate_arguments(amount, context, &unique_var/2)
Expand Down
6 changes: 3 additions & 3 deletions lib/elixir/src/elixir_env.erl
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ reset_unused_vars(#elixir_ex{unused={_Unused, Version}} = S) ->

check_unused_vars(#elixir_ex{unused={Unused, _Version}}, E) ->
[elixir_errors:file_warn(calculate_span(Meta, Name), E, ?MODULE, {unused_var, Name, Overridden}) ||
{{{Name, nil}, _}, {Meta, Overridden}} <- maps:to_list(Unused), is_unused_var(Name)],
{{{Name, _Kind}, _Count}, {Meta, Overridden}} <- maps:to_list(Unused), is_unused_var(Name)],
E.

calculate_span(Meta, Name) ->
Expand Down Expand Up @@ -120,8 +120,8 @@ merge_and_check_unused_vars(Current, Unused, ClauseUnused, E) ->
Acc
end;

({{Name, Kind}, _Count}, {Meta, Overridden}, Acc) ->
case (Kind == nil) andalso is_unused_var(Name) of
({{Name, _Kind}, _Count}, {Meta, Overridden}, Acc) ->
case is_unused_var(Name) of
true ->
Warn = {unused_var, Name, Overridden},
elixir_errors:file_warn(Meta, E, ?MODULE, Warn);
Expand Down
6 changes: 3 additions & 3 deletions lib/elixir/src/elixir_expand.erl
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ expand({Name, Meta, Kind}, S, #{context := match} = E) when is_atom(Name), is_at
%% Variable was already overridden
#{Pair := VarVersion} when VarVersion >= PrematchVersion ->
maybe_warn_underscored_var_repeat(Meta, Name, Kind, E),
NewUnused = var_used(Meta, Pair, VarVersion, Unused),
NewUnused = var_used(Pair, Meta, VarVersion, Unused),
Var = {Name, [{version, VarVersion} | Meta], Kind},
{Var, S#elixir_ex{unused={NewUnused, Version}}, E};

Expand Down Expand Up @@ -396,7 +396,7 @@ expand({Name, Meta, Kind}, S, E) when is_atom(Name), is_atom(Kind) ->
{ok, PairVersion} ->
maybe_warn_underscored_var_access(Meta, Name, Kind, E),
Var = {Name, [{version, PairVersion} | Meta], Kind},
{Var, S#elixir_ex{unused={var_used(Meta, Pair, PairVersion, Unused), Version}}, E};
{Var, S#elixir_ex{unused={var_used(Pair, Meta, PairVersion, Unused), Version}}, E};

Error ->
case lists:keyfind(if_undefined, 1, Meta) of
Expand Down Expand Up @@ -656,7 +656,7 @@ var_unused({_, Kind} = Pair, Meta, Version, Unused, Override) ->
false -> Unused
end.

var_used(Meta, {_, Kind} = Pair, Version, Unused) ->
var_used({_, Kind} = Pair, Meta, Version, Unused) ->
KeepUnused = lists:keymember(keep_unused, 1, Meta),

if
Expand Down
12 changes: 9 additions & 3 deletions lib/elixir/src/elixir_fn.erl
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,18 @@ validate(Meta, [{Pos, _} | _], Expected, E) ->
validate(_Meta, [], _Pos, _E) ->
[].

escape({'&', Meta, [Pos]}, _E, Dict) when is_integer(Pos), Pos > 0 ->
escape({'&', Meta, [Pos]}, E, Dict) when is_integer(Pos), Pos > 0 ->
% Using a nil context here to emit warnings when variable is unused.
% This might pollute user space but is unlikely because variables
% named :"&1" are not valid syntax.
Var = {list_to_atom([$& | integer_to_list(Pos)]), Meta, nil},
{Var, orddict:store(Pos, Var, Dict)};
case orddict:find(Pos, Dict) of
{ok, Var} ->
{Var, Dict};
error ->
Next = elixir_module:next_counter(?key(E, module)),
Var = {capture, [{counter, Next} | Meta], nil},
{Var, orddict:store(Pos, Var, Dict)}
end;
escape({'&', Meta, [Pos]}, E, _Dict) when is_integer(Pos) ->
file_error(Meta, E, ?MODULE, {invalid_arity_for_capture, Pos});
escape({'&', Meta, _} = Arg, E, _Dict) ->
Expand Down
4 changes: 2 additions & 2 deletions lib/elixir/test/elixir/kernel/expansion_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -1134,9 +1134,9 @@ defmodule Kernel.ExpansionTest do
end

test "keeps position meta on & variables" do
assert expand(Code.string_to_quoted!("& &1")) ==
assert expand(Code.string_to_quoted!("& &1")) |> clean_meta([:counter]) ==
{:fn, [{:line, 1}],
[{:->, [{:line, 1}], [[{:"&1", [line: 1], nil}], {:"&1", [line: 1], nil}]}]}
[{:->, [{:line, 1}], [[{:capture, [line: 1], nil}], {:capture, [line: 1], nil}]}]}
end

test "expands remotes" do
Expand Down
16 changes: 11 additions & 5 deletions lib/elixir/test/elixir/kernel/fn_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,6 @@ defmodule Kernel.FnTest do
assert is_function(&and/2)
end

test "capture precedence in cons" do
assert [(&IO.puts/1) | &IO.puts/2] == [(&IO.puts/1) | &IO.puts/2]
end

test "capture with variable module" do
mod = List
assert (&mod.flatten(&1)).([1, [2], 3]) == [1, 2, 3]
Expand Down Expand Up @@ -140,13 +136,23 @@ defmodule Kernel.FnTest do
assert (&(!is_atom(&1))).(:foo) == false
end

test "capture other" do
test "capture with function call" do
assert (& &1).(:ok) == :ok

fun = fn a, b -> a + b end
assert (&fun.(&1, 2)).(1) == 3
end

defmacro c(x) do
quote do
&(unquote(x) <> &1)
end
end

test "capture within capture through macro" do
assert (&c(&1).("b")).("a") == "ab"
end

defp atom?(atom) when is_atom(atom), do: true
defp atom?(_), do: false

Expand Down
11 changes: 0 additions & 11 deletions lib/elixir/test/elixir/kernel/warning_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -463,17 +463,6 @@ defmodule Kernel.WarningTest do
fn x -> match?(x, :value) end
"""
)

assert_warn_eval(
[
"nofile:1",
"variable \"&1\" is unused (this might happen when using a capture argument as a pattern)",
"variable \"&1\" is unused (this might happen when using a capture argument as a pattern)"
],
"""
&match?(&1, :value)
"""
)
end

test "useless literal" do
Expand Down

0 comments on commit 53c93b9

Please sign in to comment.