Optimize previous handling by using nested unions instead of nested differences

Partially addresses #15129.

Author: josevalim

lib/elixir/lib/module/types.ex

@@ -322,7 +322,7 @@ defmodule Module.Types do
     base_info = {:def, kind, fun, expected}
 
     {_, _, _, mapping, clauses_types, clauses_context} =
-      Enum.reduce(clauses, {0, 0, [], [], [], context}, fn
+      Enum.reduce(clauses, {0, 0, Pattern.init_previous(), [], [], context}, fn
         {meta, args, guards, body}, {index, total, previous, mapping, inferred, acc_context} ->
           fresh_context = fresh_context(acc_context)
           info = {base_info, args, guards}

lib/elixir/lib/module/types/expr.ex

@@ -728,7 +728,7 @@ defmodule Module.Types.Expr do
     %{failed: failed?} = original
 
     {result, _previous, context} =
-      Enum.reduce(clauses, {acc, [], original}, fn
+      Enum.reduce(clauses, {acc, Pattern.init_previous(), original}, fn
         {:->, meta, [head, body]}, {acc, previous, context} ->
           {failed?, context} = reset_failed(context, failed?)
           {patterns, guards} = extract_head(head)

lib/elixir/lib/module/types/pattern.ex

@@ -8,6 +8,63 @@ defmodule Module.Types.Pattern do
   alias Module.Types.{Apply, Of}
   import Module.Types.{Helpers, Descr}
 
+  @doc """
+  Returns the initial value for previous clause information.
+  """
+  def init_previous do
+    {[], none()}
+  end
+
+  defp empty_previous?({list, _descr}), do: list == []
+
+  defp previous_subtype?(_, {[], _}), do: false
+  defp previous_subtype?([], _), do: true
+  defp previous_subtype?([type], {_, descr}), do: subtype?(type, descr)
+  defp previous_subtype?(args, {_, descr}), do: subtype?(args_to_domain(args), descr)
+
+  defp concat_previous([], previous),
+    do: previous
+
+  defp concat_previous([type], {list, descr}),
+    do: {[[type] | list], union(type, descr)}
+
+  defp concat_previous(types, {list, descr}),
+    do: {[types | list], union(args_to_domain(types), descr)}
+
+  defp of_pattern_previous(types, {[], _}, _trees, _pattern_info, _tag, _stack, _context) do
+    {:ok, types}
+  end
+
+  defp of_pattern_previous(types, {_, descr}, trees, pattern_info, tag, stack, context) do
+    types =
+      case types do
+        [type] ->
+          [difference(type, descr)]
+
+        [_ | _] ->
+          types
+          |> args_to_domain()
+          |> difference(descr)
+          |> domain_to_flat_args(types)
+      end
+
+    if index = Enum.find_index(types, &empty?/1) do
+      {_, _, pattern} = Enum.fetch!(trees, index)
+      context = badpattern_error(pattern, index, tag, stack, context)
+      {:error, error_vars(pattern_info, context)}
+    else
+      {:ok, types}
+    end
+  end
+
+  defp previous_to_string({previous, _}) do
+    Enum.map_join(previous, "\n    ", fn types ->
+      types
+      |> Enum.map_join(", ", &to_quoted_string/1)
+      |> indent(4)
+    end)
+  end
+
   @doc """
   Refine the dependencies of variables represented by version.
   """
@@ -129,16 +186,17 @@ defmodule Module.Types.Pattern do
   Every step we deduct previous clauses from the current ones and,
   in case of failures, we try to break down the root cause.
   """
-  def of_head(patterns, guards, expected, previous, tag, meta, stack, original_context) do
+  def of_head(patterns, guards, expected, previous, tag, meta, stack, original) do
     stack = %{stack | meta: meta}
 
     {trees, precise?, context} =
-      of_precise_head(patterns, guards, expected, previous, tag, stack, original_context)
+      of_precise_head(patterns, guards, expected, previous, tag, stack, original)
 
-    if context.failed and previous != [] and Keyword.get(meta, :generated, false) != true do
+    if context.failed and not empty_previous?(previous) and
+         Keyword.get(meta, :generated, false) != true do
       # If it failed, let's try to break it down to a better error message.
       # First we check if it fails without previous, if it doesn't, check if it is redundant.
-      case of_precise_head(patterns, guards, expected, [], tag, stack, original_context) do
+      case of_precise_head(patterns, guards, expected, init_previous(), tag, stack, original) do
         {other_trees, _, %{failed: true} = other_context} ->
           {other_trees, previous, other_context}
 
@@ -150,7 +208,7 @@ defmodule Module.Types.Pattern do
               |> upper_bound()
             end)
 
-          if args_subtype?(args_types, previous) do
+          if previous_subtype?(args_types, previous) do
             warning = {:redundant, tag, expected, args_types, previous, other_context}
             {other_trees, previous, warn(__MODULE__, warning, meta, stack, other_context)}
           else
@@ -166,12 +224,12 @@ defmodule Module.Types.Pattern do
         end)
 
       cond do
-        args_subtype?(args_types, previous) ->
+        previous_subtype?(args_types, previous) ->
           warning = {:redundant, tag, expected, args_types, previous, context}
           {trees, previous, warn(__MODULE__, warning, meta, stack, context)}
 
         precise? ->
-          {trees, [args_types | previous], context}
+          {trees, concat_previous(args_types, previous), context}
 
         true ->
           {trees, previous, context}
@@ -192,22 +250,6 @@ defmodule Module.Types.Pattern do
     end
   end
 
-  defp args_subtype?(_, []),
-    do: false
-
-  defp args_subtype?([], _),
-    do: true
-
-  defp args_subtype?([type], previous),
-    do: subtype?(type, Enum.reduce(previous, none(), &union(&2, hd(&1))))
-
-  defp args_subtype?(args, previous) do
-    subtype?(
-      args_to_domain(args),
-      Enum.reduce(previous, none(), &union(&2, args_to_domain(&1)))
-    )
-  end
-
   @doc """
   Computes the domain from the pattern tree and expected types.
 
@@ -335,31 +377,6 @@ defmodule Module.Types.Pattern do
     {:ok, Enum.reverse(acc)}
   end
 
-  defp of_pattern_previous(types, [], _trees, _pattern_info, _tag, _stack, _context) do
-    {:ok, types}
-  end
-
-  defp of_pattern_previous(types, previous, trees, pattern_info, tag, stack, context) do
-    types =
-      case types do
-        [type] ->
-          [Enum.reduce(previous, type, &difference(&2, hd(&1)))]
-
-        [_ | _] ->
-          previous
-          |> Enum.reduce(args_to_domain(types), &difference(&2, args_to_domain(&1)))
-          |> domain_to_flat_args(types)
-      end
-
-    if index = Enum.find_index(types, &empty?/1) do
-      {_, _, pattern} = Enum.fetch!(trees, index)
-      context = badpattern_error(pattern, index, tag, stack, context)
-      {:error, error_vars(pattern_info, context)}
-    else
-      {:ok, types}
-    end
-  end
-
   defp of_pattern_refine(types, pattern_info, tag, stack, context) do
     pattern_info
     |> Enum.reverse()
@@ -1490,7 +1507,7 @@ defmodule Module.Types.Pattern do
       else
         _ ->
           with {_op, _meta, expr, type} <- info,
-               true <- args_subtype?(expected, previous) do
+               true <- previous_subtype?(expected, previous) do
             """
             the following clause cannot match because the previous clauses already matched all possible values:
 
@@ -1690,12 +1707,4 @@ defmodule Module.Types.Pattern do
     |> Enum.map_join(", ", &to_quoted_string/1)
     |> indent(4)
   end
-
-  defp previous_to_string(previous) do
-    Enum.map_join(previous, "\n    ", fn types ->
-      types
-      |> Enum.map_join(", ", &to_quoted_string/1)
-      |> indent(4)
-    end)
-  end
 end

lib/elixir/test/elixir/module/types/integration_test.exs

@@ -850,6 +850,43 @@ defmodule Module.Types.IntegrationTest do
   end
 
   describe "performance regressions" do
+    test "redundant clause checking on structs with many fields" do
+      files = %{
+        "big_struct.ex" => """
+        defmodule BigStruct do
+          defstruct [:f1, :f2, :f3, :f4, :f5, :f6, :f7, :f8, :f9, :f10,
+                     :f11, :f12, :f13, :f14, :f15, :f16, :f17, :f18, :f19, :f20,
+                     :f21, :f22, :f23, :f24, :f25, :f26, :f27, :f28, :f29, :f30,
+                     :f31, :f32, :f33, :value, :schema]
+          def cast(%__MODULE__{value: nil, schema: %{k1: _}}), do: :ok
+          def cast(%__MODULE__{value: nil, schema: %{k2: _}}), do: :ok
+          def cast(%__MODULE__{value: nil, schema: %{k3: _}}), do: :ok
+          def cast(%__MODULE__{value: nil, schema: %{k4: _}}), do: :ok
+          def cast(%__MODULE__{value: nil, schema: %{k5: _}}), do: :ok
+          def cast(%__MODULE__{value: nil, schema: %{k6: _}}), do: :ok
+          def cast(%__MODULE__{value: nil, schema: %{k7: _}}), do: :ok
+          def cast(%__MODULE__{value: nil, schema: %{k8: _}}), do: :ok
+          def cast(%__MODULE__{value: nil, schema: %{k9: _}}), do: :ok
+          def cast(%__MODULE__{value: nil, schema: %{k10: _}}), do: :ok
+          def cast(%__MODULE__{value: nil, schema: %{k11: _}}), do: :ok
+          def cast(%__MODULE__{value: nil, schema: %{k12: _}}), do: :ok
+          def cast(%__MODULE__{value: nil, schema: %{k13: _}}), do: :ok
+          def cast(%__MODULE__{value: nil, schema: %{k14: _}}), do: :ok
+          def cast(%__MODULE__{value: nil, schema: %{k15: _}}), do: :ok
+          def cast(%__MODULE__{value: nil, schema: %{k16: _}}), do: :ok
+          def cast(%__MODULE__{value: nil, schema: %{k17: _}}), do: :ok
+          def cast(%__MODULE__{value: nil, schema: %{k18: _}}), do: :ok
+          def cast(%__MODULE__{value: nil, schema: %{k19: _}}), do: :ok
+          def cast(%__MODULE__{value: nil, schema: %{k20: _}}), do: :ok
+          # This different clause avoids optimizations from kick in many cases
+          def cast(%__MODULE__{schema: %{target_key: x}}), do: x
+        end
+        """
+      }
+
+      assert_no_warnings(files)
+    end
+
     test "unions and intersections of open maps" do
       files = %{
         "large_head.ex" => """

lib/elixir/test/elixir/module/types/type_helper.exs

@@ -153,19 +153,23 @@ defmodule TypeHelper do
   def __precise__?(patterns, guards) do
     stack = new_stack(:static)
     expected = Enum.map(patterns, fn _ -> Descr.dynamic() end)
+    previous = Pattern.init_previous()
+    tag = {:fn, patterns}
 
-    {_trees, previous, _context} =
-      Pattern.of_head(patterns, guards, expected, [], {:fn, patterns}, [], stack, new_context())
+    {_trees, {previous, _}, _context} =
+      Pattern.of_head(patterns, guards, expected, previous, tag, [], stack, new_context())
 
     previous != []
   end
 
   def __typecheck__(mode, patterns, guards, body) do
     stack = new_stack(mode)
     expected = Enum.map(patterns, fn _ -> Descr.dynamic() end)
+    previous = Pattern.init_previous()
+    tag = {:fn, patterns}
 
     {_trees, _precise?, context} =
-      Pattern.of_head(patterns, guards, expected, [], {:fn, patterns}, [], stack, new_context())
+      Pattern.of_head(patterns, guards, expected, previous, tag, [], stack, new_context())
 
     Expr.of_expr(body, Descr.term(), :ok, stack, context)
   end