Ensure no dialyzer warnings after protocol consolidation, closes #7107

Author: José Valim

lib/elixir/lib/protocol.ex

@@ -366,24 +366,20 @@ defmodule Protocol do
     abstract_types = :erl_parse.abstract(:lists.usort(types))
 
     clauses =
-      :lists.map(
-        fn
-          {:clause, l, [{:atom, _, :consolidated?}], [], [{:atom, _, _}]} ->
-            {:clause, l, [{:atom, 0, :consolidated?}], [], [{:atom, 0, true}]}
+      Enum.map(clauses, fn
+        {:clause, l, [{:atom, _, :consolidated?}], [], [{:atom, _, _}]} ->
+          {:clause, l, [{:atom, 0, :consolidated?}], [], [{:atom, 0, true}]}
 
-          {:clause, l, [{:atom, _, :impls}], [], [{:atom, _, _}]} ->
-            tuple = {:tuple, 0, [{:atom, 0, :consolidated}, abstract_types]}
-            {:clause, l, [{:atom, 0, :impls}], [], [tuple]}
+        {:clause, l, [{:atom, _, :impls}], [], [{:atom, _, _}]} ->
+          tuple = {:tuple, 0, [{:atom, 0, :consolidated}, abstract_types]}
+          {:clause, l, [{:atom, 0, :impls}], [], [tuple]}
 
-          {:clause, _, _, _, _} = c ->
-            c
-        end,
-        clauses
-      )
+        {:clause, _, _, _, _} = c ->
+          c
+      end)
 
-    change_impl_for(tail, protocol, types, structs, true, [
-      {:function, line, :__protocol__, 1, clauses} | acc
-    ])
+    acc = [{:function, line, :__protocol__, 1, clauses} | acc]
+    change_impl_for(tail, protocol, types, structs, true, acc)
   end
 
   defp change_impl_for(
@@ -404,9 +400,8 @@ defmodule Protocol do
     clauses =
       [struct_clause_for(line) | clauses] ++ [fallback_clause_for(fallback, protocol, line)]
 
-    change_impl_for(tail, protocol, types, structs, protocol?, [
-      {:function, line, :impl_for, 1, clauses} | acc
-    ])
+    acc = [{:function, line, :impl_for, 1, clauses} | acc]
+    change_impl_for(tail, protocol, types, structs, protocol?, acc)
   end
 
   defp change_impl_for(
@@ -549,6 +544,13 @@ defmodule Protocol do
 
   defp after_defprotocol do
     quote bind_quoted: [builtin: __builtin__()] do
+      any_impl_for =
+        if @fallback_to_any do
+          quote do: unquote(__MODULE__.Any).__impl__(:target)
+        else
+          nil
+        end
+
       @doc false
       @spec impl_for(term) :: atom | nil
       Kernel.def(impl_for(data))
@@ -567,9 +569,10 @@ defmodule Protocol do
           target = Module.concat(__MODULE__, mod)
 
           Kernel.def impl_for(data) when :erlang.unquote(guard)(data) do
-            case impl_for?(unquote(target)) do
+            case Code.ensure_compiled?(unquote(target)) and
+                   function_exported?(unquote(target), :__impl__, 1) do
               true -> unquote(target).__impl__(:target)
-              false -> any_impl_for()
+              false -> unquote(any_impl_for)
             end
           end
         end,
@@ -583,39 +586,33 @@ defmodule Protocol do
       # since it relies on Dialyzer not being smart enough to conclude that all
       # opaque types will get the any_impl_for/0 implementation.
       Kernel.def impl_for(_) do
-        any_impl_for()
+        unquote(any_impl_for)
       end
 
       @doc false
-      @spec impl_for!(term) :: atom | no_return
-      Kernel.def impl_for!(data) do
-        impl_for(data) || raise(Protocol.UndefinedError, protocol: __MODULE__, value: data)
-      end
-
-      # Internal handler for Any
-      if @fallback_to_any do
-        Kernel.defp(any_impl_for(), do: __MODULE__.Any.__impl__(:target))
+      @spec impl_for!(term) :: atom
+      if any_impl_for do
+        Kernel.def impl_for!(data) do
+          impl_for(data)
+        end
       else
-        Kernel.defp(any_impl_for(), do: nil)
+        Kernel.def impl_for!(data) do
+          impl_for(data) || raise(Protocol.UndefinedError, protocol: __MODULE__, value: data)
+        end
       end
 
       # Internal handler for Structs
       Kernel.defp struct_impl_for(struct) do
         target = Module.concat(__MODULE__, struct)
 
-        case impl_for?(target) do
+        case Code.ensure_compiled?(target) and function_exported?(target, :__impl__, 1) do
           true -> target.__impl__(:target)
-          false -> any_impl_for()
+          false -> unquote(any_impl_for)
         end
       end
 
-      # Check if compilation is available internally
-      Kernel.defp impl_for?(target) do
-        Code.ensure_compiled?(target) and function_exported?(target, :__impl__, 1)
-      end
-
-      # Inline any and struct implementations
-      @compile {:inline, any_impl_for: 0, struct_impl_for: 1, impl_for?: 1}
+      # Inline struct implementation for performance
+      @compile {:inline, struct_impl_for: 1}
 
       unless Kernel.Typespec.defines_type?(__MODULE__, :t, 0) do
         @type t :: term

lib/elixir/test/elixir/fixtures/dialyzer/protocol_opaque.ex

@@ -6,6 +6,7 @@ defmodule Dialyzer.ProtocolOpaque do
 end
 
 defprotocol Dialyzer.ProtocolOpaque.Entity do
+  @fallback_to_any true
   def speak(entity)
 end
 
@@ -20,3 +21,9 @@ defmodule Dialyzer.ProtocolOpaque.Duck do
     def speak(%Dialyzer.ProtocolOpaque.Duck{}), do: "Quack!"
   end
 end
+
+defimpl Dialyzer.ProtocolOpaque.Entity, for: Any do
+  def speak(_any) do
+    "I can be anything"
+  end
+end

lib/elixir/test/elixir/kernel/dialyzer_test.exs

@@ -36,8 +36,8 @@ defmodule Kernel.DialyzerTest do
     # Set up a per-test temporary directory, so we can run these with async: true.
     # We use the test's line number as the directory name, so they won't conflict.
     dir =
-      context[:base_dir]
-      |> Path.join("line#{context[:line]}")
+      context.base_dir
+      |> Path.join("line#{context.line}")
       |> String.to_charlist()
 
     File.mkdir_p!(dir)
@@ -47,10 +47,8 @@ defmodule Kernel.DialyzerTest do
       |> Path.join("plt")
       |> String.to_charlist()
 
-    File.cp!(context[:base_plt], plt)
-
+    File.cp!(context.base_plt, plt)
     dialyzer = [analysis_type: :succ_typings, check_plt: false, files_rec: [dir], plts: [plt]]
-
     {:ok, [outdir: dir, dialyzer: dialyzer]}
   end
 
@@ -88,9 +86,17 @@ defmodule Kernel.DialyzerTest do
   end
 
   test "no warnings on protocol calls with opaque types", context do
-    copy_beam!(context, Dialyzer.ProtocolOpaque)
-    copy_beam!(context, Dialyzer.ProtocolOpaque.Entity)
-    copy_beam!(context, Dialyzer.ProtocolOpaque.Duck)
+    alias Dialyzer.ProtocolOpaque
+
+    copy_beam!(context, ProtocolOpaque)
+    copy_beam!(context, ProtocolOpaque.Entity)
+    copy_beam!(context, ProtocolOpaque.Duck)
+    assert_dialyze_no_warnings!(context)
+
+    # Also ensure no warnings after consolidation.
+    Code.prepend_path(context.base_dir)
+    {:ok, binary} = Protocol.consolidate(ProtocolOpaque.Entity, [ProtocolOpaque.Duck, Any])
+    File.write!(Path.join(context.outdir, "#{ProtocolOpaque.Entity}.beam"), binary)
     assert_dialyze_no_warnings!(context)
   end
 
@@ -118,11 +124,11 @@ defmodule Kernel.DialyzerTest do
 
   defp copy_beam!(context, module) do
     name = "#{module}.beam"
-    File.cp!(Path.join(context[:base_dir], name), Path.join(context[:outdir], name))
+    File.cp!(Path.join(context.base_dir, name), Path.join(context.outdir, name))
   end
 
   defp assert_dialyze_no_warnings!(context) do
-    case dialyzer_run(context[:dialyzer]) do
+    case dialyzer_run(context.dialyzer) do
       [] ->
         :ok