From dbf35d5ff2601a9040a3c59b15fd533009c1824f Mon Sep 17 00:00:00 2001 From: Kristof Semjen Date: Fri, 24 May 2024 10:21:40 +0200 Subject: [PATCH 1/5] Attempts to solve #13593 Attempts to solve an issue that causes `Macro.escape/2` to crash when given certain tuples. Note that, `Macro.escape({:quote, [column: 10], [:foo]})` still produces : `{:{}, [], [:quote, [], [:foo]]}` which is arguably wrong. --- lib/elixir/src/elixir_quote.erl | 8 +++++--- lib/elixir/test/elixir/macro_test.exs | 5 +++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/elixir/src/elixir_quote.erl b/lib/elixir/src/elixir_quote.erl index 65a7049d7cc..676a630ee3d 100644 --- a/lib/elixir/src/elixir_quote.erl +++ b/lib/elixir/src/elixir_quote.erl @@ -538,8 +538,10 @@ update_last([H | T], F) -> [H | update_last(T, F)]. keyfind(Key, Meta) -> lists:keyfind(Key, 1, Meta). -keydelete(Key, Meta) -> - lists:keydelete(Key, 1, Meta). +keydelete(Key, Meta) when is_list(Meta) -> + lists:keydelete(Key, 1, Meta); +keydelete(_Key, Meta) -> + Meta. keystore(_Key, Meta, nil) -> Meta; keystore(Key, Meta, Value) -> @@ -571,4 +573,4 @@ annotate_def({'when', Meta, [Left, Right]}, Context) -> annotate_def({Fun, Meta, Args}, Context) -> {Fun, keystore(context, Meta, Context), Args}; annotate_def(Other, _Context) -> - Other. \ No newline at end of file + Other. diff --git a/lib/elixir/test/elixir/macro_test.exs b/lib/elixir/test/elixir/macro_test.exs index 0b4aecb8ee1..f26d7e5a56d 100644 --- a/lib/elixir/test/elixir/macro_test.exs +++ b/lib/elixir/test/elixir/macro_test.exs @@ -38,6 +38,11 @@ defmodule MacroTest do assert Macro.escape({:a, {1, 2, 3}, :c}) == {:{}, [], [:a, {:{}, [], [1, 2, 3]}, :c]} end + test "correctly escapes tuples where the first element is :quote" do + assert Macro.escape({:quote, :foo, [:bar]}) == {:{}, [], [:quote, :foo, [:bar]]} + assert Macro.escape({:quote, :foo, [:bar, :baz]}) == {:{}, [], [:quote, :foo, [:bar, :baz]]} + end + test "escapes maps" do assert Macro.escape(%{a: 1}) == {:%{}, [], [a: 1]} end From 690f45f7478d67fcbc9f7fa4828363cbbd699013 Mon Sep 17 00:00:00 2001 From: Kristof Semjen Date: Fri, 24 May 2024 10:50:38 +0200 Subject: [PATCH 2/5] Moves the _fix_ closer to the actual issue Moves the `is_list` test closer to the issue. --- lib/elixir/src/elixir_quote.erl | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/elixir/src/elixir_quote.erl b/lib/elixir/src/elixir_quote.erl index 676a630ee3d..24a949c1979 100644 --- a/lib/elixir/src/elixir_quote.erl +++ b/lib/elixir/src/elixir_quote.erl @@ -263,7 +263,7 @@ quote(Expr, Q) -> %% quote/unquote -do_quote({quote, Meta, [Arg]}, Q) -> +do_quote({quote, Meta, [Arg]}, Q) when is_list(Meta) -> TArg = do_quote(Arg, Q#elixir_quote{unquote=false}), NewMeta = case Q of @@ -273,7 +273,7 @@ do_quote({quote, Meta, [Arg]}, Q) -> {'{}', [], [quote, meta(NewMeta, Q), [TArg]]}; -do_quote({quote, Meta, [Opts, Arg]}, Q) -> +do_quote({quote, Meta, [Opts, Arg]}, Q) when is_list(Meta) -> TOpts = do_quote(Opts, Q), TArg = do_quote(Arg, Q#elixir_quote{unquote=false}), @@ -539,9 +539,7 @@ update_last([H | T], F) -> [H | update_last(T, F)]. keyfind(Key, Meta) -> lists:keyfind(Key, 1, Meta). keydelete(Key, Meta) when is_list(Meta) -> - lists:keydelete(Key, 1, Meta); -keydelete(_Key, Meta) -> - Meta. + lists:keydelete(Key, 1, Meta). keystore(_Key, Meta, nil) -> Meta; keystore(Key, Meta, Value) -> From aade20fc99de213ee47e2cc646af5fe4cdba73d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 24 May 2024 11:23:55 +0200 Subject: [PATCH 3/5] Update lib/elixir/test/elixir/macro_test.exs --- lib/elixir/test/elixir/macro_test.exs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/elixir/test/elixir/macro_test.exs b/lib/elixir/test/elixir/macro_test.exs index f26d7e5a56d..3d10dc7df31 100644 --- a/lib/elixir/test/elixir/macro_test.exs +++ b/lib/elixir/test/elixir/macro_test.exs @@ -36,9 +36,8 @@ defmodule MacroTest do assert Macro.escape({:a}) == {:{}, [], [:a]} assert Macro.escape({:a, :b, :c}) == {:{}, [], [:a, :b, :c]} assert Macro.escape({:a, {1, 2, 3}, :c}) == {:{}, [], [:a, {:{}, [], [1, 2, 3]}, :c]} - end - test "correctly escapes tuples where the first element is :quote" do + # False positives assert Macro.escape({:quote, :foo, [:bar]}) == {:{}, [], [:quote, :foo, [:bar]]} assert Macro.escape({:quote, :foo, [:bar, :baz]}) == {:{}, [], [:quote, :foo, [:bar, :baz]]} end From abaad0ca5802626957ec620b85ed923a21ed7db5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 24 May 2024 11:24:09 +0200 Subject: [PATCH 4/5] Update lib/elixir/test/elixir/macro_test.exs --- lib/elixir/test/elixir/macro_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/elixir/test/elixir/macro_test.exs b/lib/elixir/test/elixir/macro_test.exs index 3d10dc7df31..a1f697685a8 100644 --- a/lib/elixir/test/elixir/macro_test.exs +++ b/lib/elixir/test/elixir/macro_test.exs @@ -37,7 +37,7 @@ defmodule MacroTest do assert Macro.escape({:a, :b, :c}) == {:{}, [], [:a, :b, :c]} assert Macro.escape({:a, {1, 2, 3}, :c}) == {:{}, [], [:a, {:{}, [], [1, 2, 3]}, :c]} - # False positives + # False positives assert Macro.escape({:quote, :foo, [:bar]}) == {:{}, [], [:quote, :foo, [:bar]]} assert Macro.escape({:quote, :foo, [:bar, :baz]}) == {:{}, [], [:quote, :foo, [:bar, :baz]]} end From 693d62e02da86eb50bfa0ba19f3b556c903b2387 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 24 May 2024 11:26:14 +0200 Subject: [PATCH 5/5] Update elixir_quote.erl --- lib/elixir/src/elixir_quote.erl | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/elixir/src/elixir_quote.erl b/lib/elixir/src/elixir_quote.erl index 24a949c1979..9e60ede10bf 100644 --- a/lib/elixir/src/elixir_quote.erl +++ b/lib/elixir/src/elixir_quote.erl @@ -139,7 +139,7 @@ escape(Expr, Op, Unquote) -> unquote=Unquote }). -do_escape({Left, Meta, Right}, #elixir_quote{op=prune_metadata} = Q) -> +do_escape({Left, Meta, Right}, #elixir_quote{op=prune_metadata} = Q) when is_list(Meta) -> TM = [{K, V} || {K, V} <- Meta, (K == no_parens) orelse (K == line)], TL = do_quote(Left, Q), TR = do_quote(Right, Q), @@ -284,13 +284,13 @@ do_quote({quote, Meta, [Opts, Arg]}, Q) when is_list(Meta) -> {'{}', [], [quote, meta(NewMeta, Q), [TOpts, TArg]]}; -do_quote({unquote, _Meta, [Expr]}, #elixir_quote{unquote=true}) -> +do_quote({unquote, Meta, [Expr]}, #elixir_quote{unquote=true}) when is_list(Meta) -> Expr; %% Aliases do_quote({'__aliases__', Meta, [H | T]}, #elixir_quote{aliases_hygiene=(#{}=E)} = Q) - when is_atom(H), H /= 'Elixir' -> + when is_list(Meta), is_atom(H), H /= 'Elixir' -> Annotation = case elixir_aliases:expand(Meta, [H | T], E, true) of Atom when is_atom(Atom) -> Atom; @@ -312,16 +312,16 @@ do_quote({Name, Meta, nil}, #elixir_quote{op=add_context} = Q) %% Unquote -do_quote({{{'.', Meta, [Left, unquote]}, _, [Expr]}, _, Args}, #elixir_quote{unquote=true} = Q) -> +do_quote({{{'.', Meta, [Left, unquote]}, _, [Expr]}, _, Args}, #elixir_quote{unquote=true} = Q) when is_list(Meta) -> do_quote_call(Left, Meta, Expr, Args, Q); -do_quote({{'.', Meta, [Left, unquote]}, _, [Expr]}, #elixir_quote{unquote=true} = Q) -> +do_quote({{'.', Meta, [Left, unquote]}, _, [Expr]}, #elixir_quote{unquote=true} = Q) when is_list(Meta) -> do_quote_call(Left, Meta, Expr, nil, Q); %% Imports do_quote({'&', Meta, [{'/', _, [{F, _, C}, A]}] = Args}, - #elixir_quote{imports_hygiene=(#{}=E)} = Q) when is_atom(F), is_integer(A), is_atom(C) -> + #elixir_quote{imports_hygiene=(#{}=E)} = Q) when is_atom(F), is_integer(A), is_atom(C), is_list(Meta) -> NewMeta = case elixir_dispatch:find_import(Meta, F, A, E) of false -> @@ -538,7 +538,7 @@ update_last([H | T], F) -> [H | update_last(T, F)]. keyfind(Key, Meta) -> lists:keyfind(Key, 1, Meta). -keydelete(Key, Meta) when is_list(Meta) -> +keydelete(Key, Meta) -> lists:keydelete(Key, 1, Meta). keystore(_Key, Meta, nil) -> Meta;