From dc12d878608e7a785c82c9f032e0600254c131e7 Mon Sep 17 00:00:00 2001 From: sabiwara Date: Tue, 16 Sep 2025 14:10:22 +0900 Subject: [PATCH 1/7] Bugfix: Macro.escape/1 properly escapes meta in :quote tuples Close https://github.com/elixir-lang/elixir/issues/14771 --- lib/elixir/src/elixir_quote.erl | 22 ++++++++++------------ lib/elixir/test/elixir/macro_test.exs | 5 +++++ 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/lib/elixir/src/elixir_quote.erl b/lib/elixir/src/elixir_quote.erl index 095e402625..46cc3b0955 100644 --- a/lib/elixir/src/elixir_quote.erl +++ b/lib/elixir/src/elixir_quote.erl @@ -340,23 +340,13 @@ quote(Expr, Q) -> do_quote({quote, Meta, [Arg]}, Q) when is_list(Meta) -> TArg = do_quote(Arg, Q#elixir_quote{unquote=false}), - NewMeta = case Q of - #elixir_quote{op=add_context, context=Context} -> keystore(context, Meta, Context); - _ -> Meta - end, - - {'{}', [], [quote, meta(NewMeta, Q), [TArg]]}; + {'{}', [], [quote, quote_meta(Meta, Q), [TArg]]}; 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}), - NewMeta = case Q of - #elixir_quote{op=add_context, context=Context} -> keystore(context, Meta, Context); - _ -> Meta - end, - - {'{}', [], [quote, meta(NewMeta, Q), [TOpts, TArg]]}; + {'{}', [], [quote, quote_meta(Meta, Q), [TOpts, TArg]]}; % do_quote({unquote, Meta, [Expr]}, #elixir_quote{unquote=true, shallow_validate=Validate}) when is_list(Meta) -> @@ -598,6 +588,14 @@ argument_error(Message) -> meta(Meta, Q) -> generated(keep(keydelete(column, Meta), Q), Q). +quote_meta(Meta, Q) -> + Meta1 = do_quote(Meta, Q), + Meta2 = case Q of + #elixir_quote{op=add_context, context=Context} -> keystore(context, Meta1, Context); + _ -> Meta1 + end, + meta(Meta2, Q). + generated(Meta, #elixir_quote{generated=true}) -> [{generated, true} | Meta]; generated(Meta, #elixir_quote{generated=false}) -> Meta. diff --git a/lib/elixir/test/elixir/macro_test.exs b/lib/elixir/test/elixir/macro_test.exs index feaeebe3c7..0c09b31887 100644 --- a/lib/elixir/test/elixir/macro_test.exs +++ b/lib/elixir/test/elixir/macro_test.exs @@ -146,6 +146,11 @@ defmodule MacroTest do assert Macro.escape({:quote, [], [[do: :foo]]}) == {:{}, [], [:quote, [], [[do: :foo]]]} end + test "escapes the content of :quote tuples" do + assert Macro.escape({:quote, [%{}], [{}]}) == + {:{}, [], [:quote, [{:%{}, [], []}], [{:{}, [], []}]]} + end + test "escape container when a reference cannot be escaped" do assert_raise ArgumentError, ~r"contains a reference", fn -> Macro.escape(%{re_pattern: {:re_pattern, 0, 0, 0, make_ref()}}) From 58e1369253d83b4e845ba25f4e335bff2035d6a7 Mon Sep 17 00:00:00 2001 From: sabiwara Date: Tue, 16 Sep 2025 16:48:20 +0900 Subject: [PATCH 2/7] escape directly calls do_escape if unquote is false --- lib/elixir/src/elixir_quote.erl | 44 +++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/lib/elixir/src/elixir_quote.erl b/lib/elixir/src/elixir_quote.erl index 46cc3b0955..6ad69eb27d 100644 --- a/lib/elixir/src/elixir_quote.erl +++ b/lib/elixir/src/elixir_quote.erl @@ -145,21 +145,27 @@ do_tuple_linify(Fun, Meta, Left, Right, Var, Gen) -> %% Escapes the given expression. It is similar to quote, but %% lines are kept and hygiene mechanisms are disabled. escape(Expr, Op, Unquote) -> - do_quote(Expr, #elixir_quote{ + Q = #elixir_quote{ line=true, file=nil, op=Op, unquote=Unquote - }). + }, + case Unquote of + true -> do_quote(Expr, Q); + false -> do_escape(Expr, Q) + end. 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) orelse (K == delimiter)], - TL = do_quote(Left, Q), - TR = do_quote(Right, Q), + TL = do_escape(Left, Q), + TR = do_escape(Right, Q), {'{}', [], [TL, TM, TR]}; +do_escape({Left, Right}, Q) -> + {do_escape(Left, Q), do_escape(Right, Q)}; do_escape(Tuple, Q) when is_tuple(Tuple) -> - TT = do_quote(tuple_to_list(Tuple), Q), + TT = do_escape(tuple_to_list(Tuple), Q), {'{}', [], TT}; do_escape(BitString, _) when is_bitstring(BitString) -> @@ -193,7 +199,7 @@ do_escape([], _) -> []; do_escape([H | T], #elixir_quote{unquote=false} = Q) -> - do_quote_simple_list(T, do_quote(H, Q), Q); + do_quote_simple_list(T, do_escape(H, Q), Q); do_escape([H | T], Q) -> %% The improper case is inefficient, but improper lists are rare. @@ -203,7 +209,7 @@ do_escape([H | T], Q) -> _:_ -> {L, R} = reverse_improper(T, [H]), TL = do_quote_splice(L, Q, [], []), - TR = do_quote(R, Q), + TR = do_escape(R, Q), update_last(TL, fun(X) -> {'|', [], [X, TR]} end) end; @@ -232,7 +238,7 @@ escape_map_key_value(K, V, Map, Q) -> ('Elixir.Kernel':inspect(MaybeRef, []))/binary, ") and therefore it cannot be escaped ", "(it must be defined within a function instead). ", (bad_escape_hint())/binary>>); true -> - {do_quote(K, Q), do_quote(V, Q)} + {do_escape(K, Q), do_escape(V, Q)} end. find_tuple_ref(Tuple, Index) when Index > tuple_size(Tuple) -> nil; @@ -340,13 +346,23 @@ quote(Expr, Q) -> do_quote({quote, Meta, [Arg]}, Q) when is_list(Meta) -> TArg = do_quote(Arg, Q#elixir_quote{unquote=false}), - {'{}', [], [quote, quote_meta(Meta, Q), [TArg]]}; + NewMeta = case Q of + #elixir_quote{op=add_context, context=Context} -> keystore(context, Meta, Context); + _ -> Meta + end, + + {'{}', [], [quote, meta(NewMeta, Q), [TArg]]}; 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}), - {'{}', [], [quote, quote_meta(Meta, Q), [TOpts, TArg]]}; + NewMeta = case Q of + #elixir_quote{op=add_context, context=Context} -> keystore(context, Meta, Context); + _ -> Meta + end, + + {'{}', [], [quote, meta(NewMeta, Q), [TOpts, TArg]]}; % do_quote({unquote, Meta, [Expr]}, #elixir_quote{unquote=true, shallow_validate=Validate}) when is_list(Meta) -> @@ -588,14 +604,6 @@ argument_error(Message) -> meta(Meta, Q) -> generated(keep(keydelete(column, Meta), Q), Q). -quote_meta(Meta, Q) -> - Meta1 = do_quote(Meta, Q), - Meta2 = case Q of - #elixir_quote{op=add_context, context=Context} -> keystore(context, Meta1, Context); - _ -> Meta1 - end, - meta(Meta2, Q). - generated(Meta, #elixir_quote{generated=true}) -> [{generated, true} | Meta]; generated(Meta, #elixir_quote{generated=false}) -> Meta. From ff5c1d29795e5ed6174bb3815b0a5e9c4b691153 Mon Sep 17 00:00:00 2001 From: sabiwara Date: Tue, 16 Sep 2025 17:10:03 +0900 Subject: [PATCH 3/7] Change op types to :escape | :escape_and_prune --- lib/elixir/lib/kernel.ex | 4 ++-- lib/elixir/lib/kernel/utils.ex | 4 ++-- lib/elixir/lib/macro.ex | 2 +- lib/elixir/src/elixir_quote.erl | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/elixir/lib/kernel.ex b/lib/elixir/lib/kernel.ex index c652a0099a..c2476ea290 100644 --- a/lib/elixir/lib/kernel.ex +++ b/lib/elixir/lib/kernel.ex @@ -5185,7 +5185,7 @@ defmodule Kernel do quote(do: Kernel.LexicalTracker.read_cache(unquote(pid), unquote(integer))) %{} -> - :elixir_quote.escape(block, :none, false) + :elixir_quote.escape(block, :escape, false) end versioned_vars = env.versioned_vars @@ -5465,7 +5465,7 @@ defmodule Kernel do store = case unquoted_expr or unquoted_call do true -> - :elixir_quote.escape({call, expr}, :none, true) + :elixir_quote.escape({call, expr}, :escape, true) false -> key = :erlang.unique_integer() diff --git a/lib/elixir/lib/kernel/utils.ex b/lib/elixir/lib/kernel/utils.ex index c126ef10cf..1850fd8ce9 100644 --- a/lib/elixir/lib/kernel/utils.ex +++ b/lib/elixir/lib/kernel/utils.ex @@ -126,7 +126,7 @@ defmodule Kernel.Utils do key == :__struct__ and raise(ArgumentError, "cannot set :__struct__ in struct definition") try do - :elixir_quote.escape(val, :none, false) + :elixir_quote.escape(val, :escape, false) rescue e in [ArgumentError] -> raise ArgumentError, "invalid value for struct field #{key}, " <> Exception.message(e) @@ -171,7 +171,7 @@ defmodule Kernel.Utils do :lists.foreach(foreach, enforce_keys) struct = :maps.from_list([__struct__: module] ++ fields) - escaped_struct = :elixir_quote.escape(struct, :none, false) + escaped_struct = :elixir_quote.escape(struct, :escape, false) body = case bootstrapped? do diff --git a/lib/elixir/lib/macro.ex b/lib/elixir/lib/macro.ex index 2bb45d2a46..b4ac496aad 100644 --- a/lib/elixir/lib/macro.ex +++ b/lib/elixir/lib/macro.ex @@ -912,7 +912,7 @@ defmodule Macro do @spec escape(term, escape_opts) :: t() def escape(expr, opts \\ []) do unquote = Keyword.get(opts, :unquote, false) - kind = if Keyword.get(opts, :prune_metadata, false), do: :prune_metadata, else: :none + kind = if Keyword.get(opts, :prune_metadata, false), do: :escape_and_prune, else: :escape generated = Keyword.get(opts, :generated, false) case :elixir_quote.escape(expr, kind, unquote) do diff --git a/lib/elixir/src/elixir_quote.erl b/lib/elixir/src/elixir_quote.erl index 6ad69eb27d..3e65b5feaf 100644 --- a/lib/elixir/src/elixir_quote.erl +++ b/lib/elixir/src/elixir_quote.erl @@ -18,7 +18,7 @@ line=false, file=nil, context=nil, - op=none, % none | prune_metadata | add_context + op=none, % none | escape | escape_and_prune | add_context aliases_hygiene=nil, imports_hygiene=nil, unquote=true, @@ -156,7 +156,7 @@ escape(Expr, Op, Unquote) -> false -> do_escape(Expr, Q) end. -do_escape({Left, Meta, Right}, #elixir_quote{op=prune_metadata} = Q) when is_list(Meta) -> +do_escape({Left, Meta, Right}, #elixir_quote{op=escape_and_prune} = Q) when is_list(Meta) -> TM = [{K, V} || {K, V} <- Meta, (K == no_parens) orelse (K == line) orelse (K == delimiter)], TL = do_escape(Left, Q), TR = do_escape(Right, Q), From f054dd28f8dc7f323861dce32d95e6cdef4c1929 Mon Sep 17 00:00:00 2001 From: sabiwara Date: Tue, 16 Sep 2025 17:25:25 +0900 Subject: [PATCH 4/7] Attempt of skipping the clauses - fails to bootstrap --- lib/elixir/src/elixir_quote.erl | 4 ++-- lib/elixir/test/elixir/macro_test.exs | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/elixir/src/elixir_quote.erl b/lib/elixir/src/elixir_quote.erl index 3e65b5feaf..5995abc9ee 100644 --- a/lib/elixir/src/elixir_quote.erl +++ b/lib/elixir/src/elixir_quote.erl @@ -343,7 +343,7 @@ quote(Expr, Q) -> %% quote/unquote -do_quote({quote, Meta, [Arg]}, Q) when is_list(Meta) -> +do_quote({quote, Meta, [Arg]}, #elixir_quote{op=Op} = Q) when is_list(Meta), Op /= escape, Op /= escape_and_prune -> TArg = do_quote(Arg, Q#elixir_quote{unquote=false}), NewMeta = case Q of @@ -353,7 +353,7 @@ do_quote({quote, Meta, [Arg]}, Q) when is_list(Meta) -> {'{}', [], [quote, meta(NewMeta, Q), [TArg]]}; -do_quote({quote, Meta, [Opts, Arg]}, Q) when is_list(Meta) -> +do_quote({quote, Meta, [Opts, Arg]}, #elixir_quote{op=Op} = Q) when is_list(Meta), Op /= escape, Op /= escape_and_prune -> TOpts = do_quote(Opts, Q), TArg = do_quote(Arg, Q#elixir_quote{unquote=false}), diff --git a/lib/elixir/test/elixir/macro_test.exs b/lib/elixir/test/elixir/macro_test.exs index 0c09b31887..318a241af2 100644 --- a/lib/elixir/test/elixir/macro_test.exs +++ b/lib/elixir/test/elixir/macro_test.exs @@ -149,6 +149,9 @@ defmodule MacroTest do test "escapes the content of :quote tuples" do assert Macro.escape({:quote, [%{}], [{}]}) == {:{}, [], [:quote, [{:%{}, [], []}], [{:{}, [], []}]]} + + assert Macro.escape({:quote, [%{}], [{}]}, unquote: true) == + {:{}, [], [:quote, [{:%{}, [], []}], [{:{}, [], []}]]} end test "escape container when a reference cannot be escaped" do From c9f2b3e07184837d28323a9988baac3d0293000e Mon Sep 17 00:00:00 2001 From: sabiwara Date: Tue, 16 Sep 2025 17:35:02 +0900 Subject: [PATCH 5/7] Replace none -> escape, add_context -> quote --- lib/elixir/src/elixir_quote.erl | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/lib/elixir/src/elixir_quote.erl b/lib/elixir/src/elixir_quote.erl index 5995abc9ee..ba1196db63 100644 --- a/lib/elixir/src/elixir_quote.erl +++ b/lib/elixir/src/elixir_quote.erl @@ -18,7 +18,7 @@ line=false, file=nil, context=nil, - op=none, % none | escape | escape_and_prune | add_context + op=escape, % escape | escape_and_prune | quote aliases_hygiene=nil, imports_hygiene=nil, unquote=true, @@ -267,7 +267,7 @@ build(Meta, Line, File, Context, Unquote, Generated, E) -> validate_runtime(generated, Generated), Q = #elixir_quote{ - op=add_context, + op=quote, aliases_hygiene=E, imports_hygiene=E, line=VLine, @@ -343,25 +343,15 @@ quote(Expr, Q) -> %% quote/unquote -do_quote({quote, Meta, [Arg]}, #elixir_quote{op=Op} = Q) when is_list(Meta), Op /= escape, Op /= escape_and_prune -> +do_quote({quote, Meta, [Arg]}, #elixir_quote{op=quote, context=Context} = Q) when is_list(Meta) -> TArg = do_quote(Arg, Q#elixir_quote{unquote=false}), - - NewMeta = case Q of - #elixir_quote{op=add_context, context=Context} -> keystore(context, Meta, Context); - _ -> Meta - end, - + NewMeta = keystore(context, Meta, Context), {'{}', [], [quote, meta(NewMeta, Q), [TArg]]}; -do_quote({quote, Meta, [Opts, Arg]}, #elixir_quote{op=Op} = Q) when is_list(Meta), Op /= escape, Op /= escape_and_prune -> +do_quote({quote, Meta, [Opts, Arg]}, #elixir_quote{op=quote, context=Context} = Q) when is_list(Meta) -> TOpts = do_quote(Opts, Q), TArg = do_quote(Arg, Q#elixir_quote{unquote=false}), - - NewMeta = case Q of - #elixir_quote{op=add_context, context=Context} -> keystore(context, Meta, Context); - _ -> Meta - end, - + NewMeta = keystore(context, Meta, Context), {'{}', [], [quote, meta(NewMeta, Q), [TOpts, TArg]]}; % @@ -385,7 +375,7 @@ do_quote({'__aliases__', Meta, [H | T]}, #elixir_quote{aliases_hygiene=(#{}=E)} %% Vars -do_quote({Name, Meta, nil}, #elixir_quote{op=add_context} = Q) +do_quote({Name, Meta, nil}, #elixir_quote{op=quote} = Q) when is_atom(Name), is_list(Meta) -> ImportMeta = case Q#elixir_quote.imports_hygiene of nil -> Meta; @@ -441,7 +431,7 @@ do_quote({Left, Right}, Q) -> %% Everything else -do_quote(Other, #elixir_quote{op=Op} = Q) when Op =/= add_context -> +do_quote(Other, #elixir_quote{op=Op} = Q) when Op =/= quote -> do_escape(Other, Q); do_quote({_, _, _} = Tuple, Q) -> From 8ea7ad895b11d9d1e3df07d9f090e41e3747ef99 Mon Sep 17 00:00:00 2001 From: sabiwara Date: Tue, 16 Sep 2025 17:55:38 +0900 Subject: [PATCH 6/7] Revert to previous version - not called when escaping with unquote: false --- lib/elixir/src/elixir_quote.erl | 18 ++++++++++++++---- lib/elixir/test/elixir/macro_test.exs | 3 --- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/lib/elixir/src/elixir_quote.erl b/lib/elixir/src/elixir_quote.erl index ba1196db63..c68c485bd3 100644 --- a/lib/elixir/src/elixir_quote.erl +++ b/lib/elixir/src/elixir_quote.erl @@ -343,15 +343,25 @@ quote(Expr, Q) -> %% quote/unquote -do_quote({quote, Meta, [Arg]}, #elixir_quote{op=quote, context=Context} = Q) when is_list(Meta) -> +do_quote({quote, Meta, [Arg]}, Q) when is_list(Meta) -> TArg = do_quote(Arg, Q#elixir_quote{unquote=false}), - NewMeta = keystore(context, Meta, Context), + + NewMeta = case Q of + #elixir_quote{op=quote, context=Context} -> keystore(context, Meta, Context); + _ -> Meta + end, + {'{}', [], [quote, meta(NewMeta, Q), [TArg]]}; -do_quote({quote, Meta, [Opts, Arg]}, #elixir_quote{op=quote, context=Context} = Q) when is_list(Meta) -> +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}), - NewMeta = keystore(context, Meta, Context), + + NewMeta = case Q of + #elixir_quote{op=quote, context=Context} -> keystore(context, Meta, Context); + _ -> Meta + end, + {'{}', [], [quote, meta(NewMeta, Q), [TOpts, TArg]]}; % diff --git a/lib/elixir/test/elixir/macro_test.exs b/lib/elixir/test/elixir/macro_test.exs index 318a241af2..0c09b31887 100644 --- a/lib/elixir/test/elixir/macro_test.exs +++ b/lib/elixir/test/elixir/macro_test.exs @@ -149,9 +149,6 @@ defmodule MacroTest do test "escapes the content of :quote tuples" do assert Macro.escape({:quote, [%{}], [{}]}) == {:{}, [], [:quote, [{:%{}, [], []}], [{:{}, [], []}]]} - - assert Macro.escape({:quote, [%{}], [{}]}, unquote: true) == - {:{}, [], [:quote, [{:%{}, [], []}], [{:{}, [], []}]]} end test "escape container when a reference cannot be escaped" do From a96d1ee8a666d85724d80885b7d14acd50f3df07 Mon Sep 17 00:00:00 2001 From: sabiwara Date: Tue, 16 Sep 2025 18:33:41 +0900 Subject: [PATCH 7/7] Add a comment about unquote: true pitfall --- lib/elixir/lib/macro.ex | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/elixir/lib/macro.ex b/lib/elixir/lib/macro.ex index b4ac496aad..ed31255c4a 100644 --- a/lib/elixir/lib/macro.ex +++ b/lib/elixir/lib/macro.ex @@ -803,7 +803,9 @@ defmodule Macro do * `:unquote` - when `true`, this function leaves `unquote/1` and `unquote_splicing/1` expressions unescaped, effectively unquoting the contents on escape. This option is useful only when escaping - ASTs which may have quoted fragments in them. Defaults to `false`. + ASTs which may have quoted fragments in them. Note this option + will give a special meaning to `quote`/`unquote` nodes, which need + to be valid AST before escaping. Defaults to `false`. * `:prune_metadata` - when `true`, removes most metadata from escaped AST nodes. Note this option changes the semantics of escaped code and