From 7a1a3ab32bafbd2dbd79dfcf59219d140e490bd5 Mon Sep 17 00:00:00 2001 From: sabiwara Date: Thu, 16 Jan 2025 08:28:57 +0900 Subject: [PATCH 1/7] Prevent crash on zero arity def in protocol --- lib/elixir/lib/protocol.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/elixir/lib/protocol.ex b/lib/elixir/lib/protocol.ex index 1e4c3911ad6..36342ab6410 100644 --- a/lib/elixir/lib/protocol.ex +++ b/lib/elixir/lib/protocol.ex @@ -666,7 +666,7 @@ defmodule Protocol do end new_signatures = - for {{fun, arity}, :def, _, _} <- definitions do + for {{fun, arity}, :def, _, _} when arity > 0 <- definitions do rest = List.duplicate(Descr.term(), arity - 1) {{fun, arity}, {:strong, nil, [{[domain | rest], Descr.dynamic()}]}} end From 609f7cde69f924f1d23c2f8f70392e98a63dd0a8 Mon Sep 17 00:00:00 2001 From: sabiwara Date: Thu, 16 Jan 2025 20:43:06 +0900 Subject: [PATCH 2/7] Check protocol functions only --- lib/elixir/lib/protocol.ex | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/elixir/lib/protocol.ex b/lib/elixir/lib/protocol.ex index 36342ab6410..6c2f8faa785 100644 --- a/lib/elixir/lib/protocol.ex +++ b/lib/elixir/lib/protocol.ex @@ -665,10 +665,13 @@ defmodule Protocol do end end + fun_arities = :sets.from_list(protocol.__protocol__(:functions), version: 2) + new_signatures = - for {{fun, arity}, :def, _, _} when arity > 0 <- definitions do + for {{_fun, arity} = fun_arity, :def, _, _} <- definitions, + :sets.is_element(fun_arity, fun_arities) do rest = List.duplicate(Descr.term(), arity - 1) - {{fun, arity}, {:strong, nil, [{[domain | rest], Descr.dynamic()}]}} + {fun_arity, {:strong, nil, [{[domain | rest], Descr.dynamic()}]}} end [ From 6ff5011893d42ec20789607f6dadbd426b34fa16 Mon Sep 17 00:00:00 2001 From: sabiwara Date: Thu, 16 Jan 2025 22:43:02 +0900 Subject: [PATCH 3/7] Read from __protocol__ clause return --- lib/elixir/lib/protocol.ex | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/elixir/lib/protocol.ex b/lib/elixir/lib/protocol.ex index 6c2f8faa785..15c6db590a8 100644 --- a/lib/elixir/lib/protocol.ex +++ b/lib/elixir/lib/protocol.ex @@ -614,10 +614,12 @@ defmodule Protocol do {impl_for!, definitions} = List.keytake(definitions, {:impl_for!, 1}, 0) {struct_impl_for, definitions} = List.keytake(definitions, {:struct_impl_for, 1}, 0) + protocol_funs = get_protocol_functions(protocol_def) + protocol_def = change_protocol(protocol_def, types) impl_for = change_impl_for(impl_for, protocol, types) struct_impl_for = change_struct_impl_for(struct_impl_for, protocol, types, structs) - new_signatures = new_signatures(definitions, protocol, types) + new_signatures = new_signatures(definitions, protocol_funs, protocol, types) definitions = [protocol_def, impl_for, impl_for!, struct_impl_for] ++ definitions signatures = Enum.into(new_signatures, signatures) @@ -628,7 +630,7 @@ defmodule Protocol do end end - defp new_signatures(definitions, protocol, types) do + defp new_signatures(definitions, protocol_funs, protocol, types) do alias Module.Types.Descr clauses = @@ -665,11 +667,9 @@ defmodule Protocol do end end - fun_arities = :sets.from_list(protocol.__protocol__(:functions), version: 2) - new_signatures = for {{_fun, arity} = fun_arity, :def, _, _} <- definitions, - :sets.is_element(fun_arity, fun_arities) do + fun_arity in protocol_funs do rest = List.duplicate(Descr.term(), arity - 1) {fun_arity, {:strong, nil, [{[domain | rest], Descr.dynamic()}]}} end @@ -680,6 +680,13 @@ defmodule Protocol do ] ++ new_signatures end + defp get_protocol_functions({_name, _kind, _meta, clauses}) do + Enum.find_value(clauses, fn + {_meta, [:functions], [], clauses} -> clauses + _ -> nil + end) + end + defp change_protocol({_name, _kind, meta, clauses}, types) do clauses = Enum.map(clauses, fn From e9879e63073bf55f68c5e30676214af7309690bb Mon Sep 17 00:00:00 2001 From: Jean Klingler Date: Fri, 17 Jan 2025 10:33:43 +0900 Subject: [PATCH 4/7] Raise when functions are not found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Valim --- lib/elixir/lib/protocol.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/elixir/lib/protocol.ex b/lib/elixir/lib/protocol.ex index 15c6db590a8..df6d9df1632 100644 --- a/lib/elixir/lib/protocol.ex +++ b/lib/elixir/lib/protocol.ex @@ -684,7 +684,7 @@ defmodule Protocol do Enum.find_value(clauses, fn {_meta, [:functions], [], clauses} -> clauses _ -> nil - end) + end) || raise "could not find protocol functions" end defp change_protocol({_name, _kind, meta, clauses}, types) do From ba959084cd2e3bf7244ed008993d80b2b0a37c2f Mon Sep 17 00:00:00 2001 From: sabiwara Date: Fri, 17 Jan 2025 11:08:23 +0900 Subject: [PATCH 5/7] Add rest with regular functions --- .../fixtures/consolidation/extra_def.ex | 5 +++++ .../elixir/protocol/consolidation_test.exs | 19 ++++++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 lib/elixir/test/elixir/fixtures/consolidation/extra_def.ex diff --git a/lib/elixir/test/elixir/fixtures/consolidation/extra_def.ex b/lib/elixir/test/elixir/fixtures/consolidation/extra_def.ex new file mode 100644 index 00000000000..00769cb2d60 --- /dev/null +++ b/lib/elixir/test/elixir/fixtures/consolidation/extra_def.ex @@ -0,0 +1,5 @@ +defprotocol Protocol.ConsolidationTest.ExtraDef do + def protocol_fun(term) + + Kernel.def(regular_fun(term), do: term + 1) +end diff --git a/lib/elixir/test/elixir/protocol/consolidation_test.exs b/lib/elixir/test/elixir/protocol/consolidation_test.exs index 64f35f88e6a..b666a0727e8 100644 --- a/lib/elixir/test/elixir/protocol/consolidation_test.exs +++ b/lib/elixir/test/elixir/protocol/consolidation_test.exs @@ -8,7 +8,7 @@ Kernel.ParallelCompiler.compile_to_path(files, path, return_diagnostics: true) defmodule Protocol.ConsolidationTest do use ExUnit.Case, async: true - alias Protocol.ConsolidationTest.{Sample, WithAny, NoImpl} + alias Protocol.ConsolidationTest.{Sample, WithAny, NoImpl, ExtraDef} defimpl WithAny, for: Map do def ok(map, _opts) do @@ -65,6 +65,14 @@ defmodule Protocol.ConsolidationTest do defp no_impl_binary, do: unquote(binary) + # No Any + :code.purge(ExtraDef) + :code.delete(ExtraDef) + {:ok, binary} = Protocol.consolidate(ExtraDef, []) + :code.load_binary(ExtraDef, ~c"protocol_test.exs", binary) + + defp extra_def_binary, do: unquote(binary) + test "consolidated?/1" do assert Protocol.consolidated?(WithAny) refute Protocol.consolidated?(Enumerable) @@ -234,6 +242,15 @@ defmodule Protocol.ConsolidationTest do assert %{{:ok, 1} => %{sig: {:strong, nil, clauses}}} = exports assert clauses == [{[none()], dynamic()}] end + + test "handles regular function definitions" do + exports = exports(extra_def_binary()) + + assert %{{:regular_fun, 1} => %{sig: :none}} = exports + + assert %{{:protocol_fun, 1} => %{sig: {:strong, nil, clauses}}} = exports + assert clauses == [{[none()], dynamic()}] + end end test "consolidation errors on missing BEAM files" do From 9adad463b95a828d233739707468daf3bcad2ad1 Mon Sep 17 00:00:00 2001 From: sabiwara Date: Fri, 17 Jan 2025 20:43:04 +0900 Subject: [PATCH 6/7] Re-use Sample fixture --- .../elixir/fixtures/consolidation/extra_def.ex | 5 ----- .../test/elixir/fixtures/consolidation/sample.ex | 3 +++ .../test/elixir/protocol/consolidation_test.exs | 15 ++------------- 3 files changed, 5 insertions(+), 18 deletions(-) delete mode 100644 lib/elixir/test/elixir/fixtures/consolidation/extra_def.ex diff --git a/lib/elixir/test/elixir/fixtures/consolidation/extra_def.ex b/lib/elixir/test/elixir/fixtures/consolidation/extra_def.ex deleted file mode 100644 index 00769cb2d60..00000000000 --- a/lib/elixir/test/elixir/fixtures/consolidation/extra_def.ex +++ /dev/null @@ -1,5 +0,0 @@ -defprotocol Protocol.ConsolidationTest.ExtraDef do - def protocol_fun(term) - - Kernel.def(regular_fun(term), do: term + 1) -end diff --git a/lib/elixir/test/elixir/fixtures/consolidation/sample.ex b/lib/elixir/test/elixir/fixtures/consolidation/sample.ex index ee7edea3fbc..2f708dff523 100644 --- a/lib/elixir/test/elixir/fixtures/consolidation/sample.ex +++ b/lib/elixir/test/elixir/fixtures/consolidation/sample.ex @@ -4,4 +4,7 @@ defprotocol Protocol.ConsolidationTest.Sample do @deprecated "Reason" @spec ok(t) :: boolean def ok(term) + + # Not a protocol function + Kernel.def(regular_fun(term), do: term + 1) end diff --git a/lib/elixir/test/elixir/protocol/consolidation_test.exs b/lib/elixir/test/elixir/protocol/consolidation_test.exs index b666a0727e8..9a209961521 100644 --- a/lib/elixir/test/elixir/protocol/consolidation_test.exs +++ b/lib/elixir/test/elixir/protocol/consolidation_test.exs @@ -8,7 +8,7 @@ Kernel.ParallelCompiler.compile_to_path(files, path, return_diagnostics: true) defmodule Protocol.ConsolidationTest do use ExUnit.Case, async: true - alias Protocol.ConsolidationTest.{Sample, WithAny, NoImpl, ExtraDef} + alias Protocol.ConsolidationTest.{Sample, WithAny, NoImpl} defimpl WithAny, for: Map do def ok(map, _opts) do @@ -65,14 +65,6 @@ defmodule Protocol.ConsolidationTest do defp no_impl_binary, do: unquote(binary) - # No Any - :code.purge(ExtraDef) - :code.delete(ExtraDef) - {:ok, binary} = Protocol.consolidate(ExtraDef, []) - :code.load_binary(ExtraDef, ~c"protocol_test.exs", binary) - - defp extra_def_binary, do: unquote(binary) - test "consolidated?/1" do assert Protocol.consolidated?(WithAny) refute Protocol.consolidated?(Enumerable) @@ -244,12 +236,9 @@ defmodule Protocol.ConsolidationTest do end test "handles regular function definitions" do - exports = exports(extra_def_binary()) + exports = exports(sample_binary()) assert %{{:regular_fun, 1} => %{sig: :none}} = exports - - assert %{{:protocol_fun, 1} => %{sig: {:strong, nil, clauses}}} = exports - assert clauses == [{[none()], dynamic()}] end end From 1644fe4894b9dcc6e4a16ee682470b34162bdd62 Mon Sep 17 00:00:00 2001 From: Jean Klingler Date: Fri, 17 Jan 2025 20:47:20 +0900 Subject: [PATCH 7/7] Add more info to comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: José Valim --- lib/elixir/test/elixir/fixtures/consolidation/sample.ex | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/elixir/test/elixir/fixtures/consolidation/sample.ex b/lib/elixir/test/elixir/fixtures/consolidation/sample.ex index 2f708dff523..b1530e20a52 100644 --- a/lib/elixir/test/elixir/fixtures/consolidation/sample.ex +++ b/lib/elixir/test/elixir/fixtures/consolidation/sample.ex @@ -5,6 +5,8 @@ defprotocol Protocol.ConsolidationTest.Sample do @spec ok(t) :: boolean def ok(term) - # Not a protocol function + # Not a protocol function. While this is not "officially" supported, + # it does happen in practice, so we need to make sure we preserve + # its signature. Kernel.def(regular_fun(term), do: term + 1) end