From af102e160ebf3538b9ffad0cd6869a0d837656d5 Mon Sep 17 00:00:00 2001 From: sabiwara Date: Thu, 21 Sep 2023 21:11:44 +0900 Subject: [PATCH 1/3] Translate signed literals with sign in matches --- lib/elixir/lib/module/types/pattern.ex | 12 ++++++++++++ lib/elixir/src/elixir_rewrite.erl | 4 ++-- lib/elixir/test/elixir/kernel/expansion_test.exs | 13 +++++++++++-- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/lib/elixir/lib/module/types/pattern.ex b/lib/elixir/lib/module/types/pattern.ex index 36d2d25a1c2..572ff690de7 100644 --- a/lib/elixir/lib/module/types/pattern.ex +++ b/lib/elixir/lib/module/types/pattern.ex @@ -654,6 +654,18 @@ defmodule Module.Types.Pattern do {:ok, :float, context} end + # +12 / -12 + defp of_shared({{:., _, [:erlang, op]}, _, [literal]}, _stack, context, _fun) + when is_integer(literal) and op in [:+, :-] do + {:ok, :integer, context} + end + + # +1.2 / -1.2 + defp of_shared({{:., _, [:erlang, op]}, _, [literal]}, _stack, context, _fun) + when is_float(literal) and op in [:+, :-] do + {:ok, :float, context} + end + # "..." defp of_shared(literal, _stack, context, _fun) when is_binary(literal) do {:ok, :binary, context} diff --git a/lib/elixir/src/elixir_rewrite.erl b/lib/elixir/src/elixir_rewrite.erl index 8f67e833ea7..c99bd5401dc 100644 --- a/lib/elixir/src/elixir_rewrite.erl +++ b/lib/elixir/src/elixir_rewrite.erl @@ -302,8 +302,8 @@ increment(Meta, Other) -> %% The allowed operations are very limited. %% The Kernel operators are already inlined by now, we only need to %% care about Erlang ones. -match_rewrite(erlang, _, '+', _, [Arg]) when is_number(Arg) -> {ok, Arg}; -match_rewrite(erlang, _, '-', _, [Arg]) when is_number(Arg) -> {ok, -Arg}; +match_rewrite(erlang, DotMeta, '+', Meta, [Arg]) when is_number(Arg) -> {ok, {{'.', DotMeta, [erlang, '+']}, Meta, [Arg]}}; +match_rewrite(erlang, DotMeta, '-', Meta, [Arg]) when is_number(Arg) -> {ok, {{'.', DotMeta, [erlang, '-']}, Meta, [Arg]}}; match_rewrite(erlang, _, '++', Meta, [Left, Right]) -> try {ok, static_append(Left, Right, Meta)} catch impossible -> {error, {invalid_match_append, Left}} diff --git a/lib/elixir/test/elixir/kernel/expansion_test.exs b/lib/elixir/test/elixir/kernel/expansion_test.exs index f7042df3a78..9a780eb1549 100644 --- a/lib/elixir/test/elixir/kernel/expansion_test.exs +++ b/lib/elixir/test/elixir/kernel/expansion_test.exs @@ -715,8 +715,17 @@ defmodule Kernel.ExpansionTest do expand(quote(do: [1] ++ 2 ++ [3] = [1, 2, 3])) end) - assert {:=, _, [-1, {{:., _, [:erlang, :-]}, _, [1]}]} = expand(quote(do: -1 = -1)) - assert {:=, _, [1, {{:., _, [:erlang, :+]}, _, [1]}]} = expand(quote(do: +1 = +1)) + assert {:=, _, [{{:., _, [:erlang, :-]}, _, [1]}, {{:., _, [:erlang, :-]}, _, [1]}]} = + expand(quote(do: -1 = -1)) + + assert {:=, _, [{{:., _, [:erlang, :+]}, _, [1]}, {{:., _, [:erlang, :+]}, _, [1]}]} = + expand(quote(do: +1 = +1)) + + assert {:=, _, [{{:., _, [:erlang, :+]}, _, [1.0]}, {{:., _, [:erlang, :+]}, _, [1.0]}]} = + expand(quote(do: +1.0 = +1.0)) + + assert {:=, _, [{{:., _, [:erlang, :-]}, _, [1.0]}, {{:., _, [:erlang, :-]}, _, [1.0]}]} = + expand(quote(do: -1.0 = -1.0)) assert {:=, _, [[{:|, _, [1, [{:|, _, [2, 3]}]]}], [1, 2, 3]]} = expand(quote(do: [1] ++ [2] ++ 3 = [1, 2, 3])) From 49efd3637405d3709b7e65c97252b7a492635335 Mon Sep 17 00:00:00 2001 From: sabiwara Date: Fri, 22 Sep 2023 09:17:08 +0900 Subject: [PATCH 2/3] Add warning when matching on 0.0 --- lib/elixir/src/elixir_expand.erl | 7 ++++++ .../test/elixir/kernel/expansion_test.exs | 16 +++++++++----- .../test/elixir/module/types/pattern_test.exs | 22 ++++++++++++++++++- lib/elixir/test/erlang/control_test.erl | 6 +++++ 4 files changed, 44 insertions(+), 7 deletions(-) diff --git a/lib/elixir/src/elixir_expand.erl b/lib/elixir/src/elixir_expand.erl index 62b1f7c5583..d00bf104c9d 100644 --- a/lib/elixir/src/elixir_expand.erl +++ b/lib/elixir/src/elixir_expand.erl @@ -456,6 +456,11 @@ expand(Pid, S, E) when is_pid(Pid) -> {Pid, E} end; +expand(Zero, S, #{context := match} = E) when is_float(Zero), Zero == 0.0 -> + elixir_errors:file_warn([], E, ?MODULE, invalid_match_on_zero_float), + Escaped = {{'.', [], [erlang, '+']}, [], [0.0]}, + {Escaped, S, E}; + expand(Other, S, E) when is_number(Other); is_atom(Other); is_binary(Other) -> {Other, S, E}; @@ -1163,6 +1168,8 @@ guard_context(_) -> "guards". format_error({remote_nullary_no_parens, Expr}) -> String = 'Elixir.String':replace_suffix('Elixir.Macro':to_string(Expr), <<"()">>, <<>>), io_lib:format("parentheses are required for function calls with no arguments, got: ~ts", [String]); +format_error(invalid_match_on_zero_float) -> + "pattern matching on 0.0 is equivalent to matching only on +0.0 from Erlang/OTP 27+. Instead you must match on +0.0 or -0.0"; format_error({useless_literal, Term}) -> io_lib:format("code block contains unused literal ~ts " "(remove the literal or assign it to _ to avoid warnings)", diff --git a/lib/elixir/test/elixir/kernel/expansion_test.exs b/lib/elixir/test/elixir/kernel/expansion_test.exs index 9a780eb1549..67696994b65 100644 --- a/lib/elixir/test/elixir/kernel/expansion_test.exs +++ b/lib/elixir/test/elixir/kernel/expansion_test.exs @@ -468,6 +468,16 @@ defmodule Kernel.ExpansionTest do end end + describe "floats" do + test "cannot be 0.0 inside match" do + assert capture_io(:stderr, fn -> expand(quote(do: 0.0 = 0.0)) end) =~ + "pattern matching on 0.0 is equivalent to matching only on +0.0 from Erlang/OTP 27+" + + assert {:=, [], [{{:., _, [:erlang, :+]}, _, [+0.0]}, +0.0]} = expand(quote(do: +0.0 = 0.0)) + assert {:=, [], [{{:., _, [:erlang, :-]}, _, [+0.0]}, +0.0]} = expand(quote(do: -0.0 = 0.0)) + end + end + describe "tuples" do test "expanded as arguments" do assert expand(quote(do: {after_expansion = 1, a})) == quote(do: {after_expansion = 1, a()}) @@ -721,12 +731,6 @@ defmodule Kernel.ExpansionTest do assert {:=, _, [{{:., _, [:erlang, :+]}, _, [1]}, {{:., _, [:erlang, :+]}, _, [1]}]} = expand(quote(do: +1 = +1)) - assert {:=, _, [{{:., _, [:erlang, :+]}, _, [1.0]}, {{:., _, [:erlang, :+]}, _, [1.0]}]} = - expand(quote(do: +1.0 = +1.0)) - - assert {:=, _, [{{:., _, [:erlang, :-]}, _, [1.0]}, {{:., _, [:erlang, :-]}, _, [1.0]}]} = - expand(quote(do: -1.0 = -1.0)) - assert {:=, _, [[{:|, _, [1, [{:|, _, [2, 3]}]]}], [1, 2, 3]]} = expand(quote(do: [1] ++ [2] ++ 3 = [1, 2, 3])) end diff --git a/lib/elixir/test/elixir/module/types/pattern_test.exs b/lib/elixir/test/elixir/module/types/pattern_test.exs index d495ce9de8c..70e7ca108d6 100644 --- a/lib/elixir/test/elixir/module/types/pattern_test.exs +++ b/lib/elixir/test/elixir/module/types/pattern_test.exs @@ -15,6 +15,20 @@ defmodule Module.Types.PatternTest do end end + defmacrop quoted_pattern_with_diagnostics(patterns) do + {ast, diagnostics} = Code.with_diagnostics(fn -> expand_head(patterns, true) end) + + quote do + {patterns, true} = unquote(Macro.escape(ast)) + + result = + Pattern.of_pattern(patterns, new_stack(), new_context()) + |> lift_result() + + {result, unquote(Macro.escape(diagnostics))} + end + end + defmacrop quoted_head(patterns, guards \\ []) do quote do {patterns, guards} = unquote(Macro.escape(expand_head(patterns, guards))) @@ -76,8 +90,14 @@ defmodule Module.Types.PatternTest do assert quoted_pattern(false) == {:ok, {:atom, false}} assert quoted_pattern(:foo) == {:ok, {:atom, :foo}} assert quoted_pattern(0) == {:ok, :integer} - assert quoted_pattern(0.0) == {:ok, :float} + assert quoted_pattern(+0.0) == {:ok, :float} + assert quoted_pattern(-0.0) == {:ok, :float} assert quoted_pattern("foo") == {:ok, :binary} + + assert {{:ok, :float}, [diagnostic]} = quoted_pattern_with_diagnostics(0.0) + + assert diagnostic.message =~ + "pattern matching on 0.0 is equivalent to matching only on +0.0" end test "list" do diff --git a/lib/elixir/test/erlang/control_test.erl b/lib/elixir/test/erlang/control_test.erl index fbe853bd2d3..7f3337f7880 100644 --- a/lib/elixir/test/erlang/control_test.erl +++ b/lib/elixir/test/erlang/control_test.erl @@ -12,6 +12,12 @@ cond_line_test() -> {clause, 3, _, _, _}] } = to_erl("cond do\n 1 -> :ok\n 2 -> :ok\nend"). +float_match_test() -> + {'case', _, _, + [{clause, _, [{op, _, '+', {float, _, 0.0}}], [], [{atom, _, pos}]}, + {clause, _, [{op, _, '-', {float, _, 0.0}}], [], [{atom, _, neg}]}] + } = to_erl("case X do\n +0.0 -> :pos\n -0.0 -> :neg\nend"). + % Optimized optimized_if_test() -> From 6ac066462817f6e5a9041d25a58cb734ccf148dd Mon Sep 17 00:00:00 2001 From: sabiwara Date: Fri, 22 Sep 2023 22:07:26 +0900 Subject: [PATCH 3/3] Use literals as AST --- lib/elixir/lib/module/types/pattern.ex | 12 ------------ lib/elixir/src/elixir_erl.erl | 8 ++++++++ lib/elixir/src/elixir_expand.erl | 3 +-- lib/elixir/src/elixir_rewrite.erl | 4 ++-- lib/elixir/test/elixir/kernel/expansion_test.exs | 8 ++++---- 5 files changed, 15 insertions(+), 20 deletions(-) diff --git a/lib/elixir/lib/module/types/pattern.ex b/lib/elixir/lib/module/types/pattern.ex index 572ff690de7..36d2d25a1c2 100644 --- a/lib/elixir/lib/module/types/pattern.ex +++ b/lib/elixir/lib/module/types/pattern.ex @@ -654,18 +654,6 @@ defmodule Module.Types.Pattern do {:ok, :float, context} end - # +12 / -12 - defp of_shared({{:., _, [:erlang, op]}, _, [literal]}, _stack, context, _fun) - when is_integer(literal) and op in [:+, :-] do - {:ok, :integer, context} - end - - # +1.2 / -1.2 - defp of_shared({{:., _, [:erlang, op]}, _, [literal]}, _stack, context, _fun) - when is_float(literal) and op in [:+, :-] do - {:ok, :float, context} - end - # "..." defp of_shared(literal, _stack, context, _fun) when is_binary(literal) do {:ok, :binary, context} diff --git a/lib/elixir/src/elixir_erl.erl b/lib/elixir/src/elixir_erl.erl index 41a519bf0a4..2a6e04c7213 100644 --- a/lib/elixir/src/elixir_erl.erl +++ b/lib/elixir/src/elixir_erl.erl @@ -74,6 +74,14 @@ elixir_to_erl(Tree, Ann) when is_atom(Tree) -> {atom, Ann, Tree}; elixir_to_erl(Tree, Ann) when is_integer(Tree) -> {integer, Ann, Tree}; +elixir_to_erl(Tree, Ann) when is_float(Tree), Tree == 0.0 -> + % 0.0 needs to be rewritten as the AST for +0.0 in matches + Op = + case <> of + <<1:1,_:63>> -> '-'; + _ -> '+' + end, + {op, Ann, Op, {float, Ann, 0.0}}; elixir_to_erl(Tree, Ann) when is_float(Tree) -> {float, Ann, Tree}; elixir_to_erl(Tree, Ann) when is_binary(Tree) -> diff --git a/lib/elixir/src/elixir_expand.erl b/lib/elixir/src/elixir_expand.erl index d00bf104c9d..c72d2124362 100644 --- a/lib/elixir/src/elixir_expand.erl +++ b/lib/elixir/src/elixir_expand.erl @@ -458,8 +458,7 @@ expand(Pid, S, E) when is_pid(Pid) -> expand(Zero, S, #{context := match} = E) when is_float(Zero), Zero == 0.0 -> elixir_errors:file_warn([], E, ?MODULE, invalid_match_on_zero_float), - Escaped = {{'.', [], [erlang, '+']}, [], [0.0]}, - {Escaped, S, E}; + {Zero, S, E}; expand(Other, S, E) when is_number(Other); is_atom(Other); is_binary(Other) -> {Other, S, E}; diff --git a/lib/elixir/src/elixir_rewrite.erl b/lib/elixir/src/elixir_rewrite.erl index c99bd5401dc..8f67e833ea7 100644 --- a/lib/elixir/src/elixir_rewrite.erl +++ b/lib/elixir/src/elixir_rewrite.erl @@ -302,8 +302,8 @@ increment(Meta, Other) -> %% The allowed operations are very limited. %% The Kernel operators are already inlined by now, we only need to %% care about Erlang ones. -match_rewrite(erlang, DotMeta, '+', Meta, [Arg]) when is_number(Arg) -> {ok, {{'.', DotMeta, [erlang, '+']}, Meta, [Arg]}}; -match_rewrite(erlang, DotMeta, '-', Meta, [Arg]) when is_number(Arg) -> {ok, {{'.', DotMeta, [erlang, '-']}, Meta, [Arg]}}; +match_rewrite(erlang, _, '+', _, [Arg]) when is_number(Arg) -> {ok, Arg}; +match_rewrite(erlang, _, '-', _, [Arg]) when is_number(Arg) -> {ok, -Arg}; match_rewrite(erlang, _, '++', Meta, [Left, Right]) -> try {ok, static_append(Left, Right, Meta)} catch impossible -> {error, {invalid_match_append, Left}} diff --git a/lib/elixir/test/elixir/kernel/expansion_test.exs b/lib/elixir/test/elixir/kernel/expansion_test.exs index 67696994b65..8e16612f2df 100644 --- a/lib/elixir/test/elixir/kernel/expansion_test.exs +++ b/lib/elixir/test/elixir/kernel/expansion_test.exs @@ -473,8 +473,8 @@ defmodule Kernel.ExpansionTest do assert capture_io(:stderr, fn -> expand(quote(do: 0.0 = 0.0)) end) =~ "pattern matching on 0.0 is equivalent to matching only on +0.0 from Erlang/OTP 27+" - assert {:=, [], [{{:., _, [:erlang, :+]}, _, [+0.0]}, +0.0]} = expand(quote(do: +0.0 = 0.0)) - assert {:=, [], [{{:., _, [:erlang, :-]}, _, [+0.0]}, +0.0]} = expand(quote(do: -0.0 = 0.0)) + assert {:=, [], [+0.0, +0.0]} = expand(quote(do: +0.0 = 0.0)) + assert {:=, [], [-0.0, +0.0]} = expand(quote(do: -0.0 = 0.0)) end end @@ -725,10 +725,10 @@ defmodule Kernel.ExpansionTest do expand(quote(do: [1] ++ 2 ++ [3] = [1, 2, 3])) end) - assert {:=, _, [{{:., _, [:erlang, :-]}, _, [1]}, {{:., _, [:erlang, :-]}, _, [1]}]} = + assert {:=, _, [-1, {{:., _, [:erlang, :-]}, _, [1]}]} = expand(quote(do: -1 = -1)) - assert {:=, _, [{{:., _, [:erlang, :+]}, _, [1]}, {{:., _, [:erlang, :+]}, _, [1]}]} = + assert {:=, _, [1, {{:., _, [:erlang, :+]}, _, [1]}]} = expand(quote(do: +1 = +1)) assert {:=, _, [[{:|, _, [1, [{:|, _, [2, 3]}]]}], [1, 2, 3]]} =