Improve map unification (#10310)

Author: ericmj

lib/elixir/lib/map.ex

@@ -67,8 +67,10 @@ defmodule Map do
       iex> %{a: a} = %{:a => 1, "b" => 2, [:c, :e, :e] => 3}
       iex> a
       1
-      iex> %{:c => 3} = %{:a => 1, 2 => :b}
-      ** (MatchError) no match of right hand side value: %{2 => :b, :a => 1}
+
+  But this will raise a match error:
+
+      %{:c => 3} = %{:a => 1, 2 => :b}
 
   Variables can be used as map keys both when writing map literals as well as
   when matching:

lib/elixir/lib/module/types.ex

@@ -266,6 +266,8 @@ defmodule Module.Types do
     end
   end
 
+  defp missing_field(_, _), do: :error
+
   defp format_traces([], _simplify?) do
     {[], []}
   end

lib/elixir/lib/module/types/infer.ex

@@ -158,104 +158,132 @@ defmodule Module.Types.Infer do
     end
   end
 
-  defp unify_maps(source_pairs, target_pairs, %{context: :pattern} = stack, context) do
+  # * All required keys on each side need to match to the other side.
+  # * All optional keys on each side that do not match must be discarded.
+
+  defp unify_maps(source_pairs, target_pairs, stack, context) do
     source_pairs = expand_struct(source_pairs)
     target_pairs = expand_struct(target_pairs)
 
-    # Since maps in patterns only support literal keys (excluding maps)
-    # we can do exact type match without subtype checking
-
-    unique_right_pairs =
-      Enum.reject(target_pairs, fn {_kind, key, _value} ->
-        List.keyfind(source_pairs, key, 1)
-      end)
-
-    unique_pairs = source_pairs ++ unique_right_pairs
-
-    # Build union of all unique key-value pairs between the maps
-    result =
-      map_reduce_ok(unique_pairs, context, fn {source_kind, source_key, source_value}, context ->
-        case List.keyfind(target_pairs, source_key, 1) do
-          {target_kind, ^source_key, target_value} ->
-            case unify(source_value, target_value, stack, context) do
-              {:ok, value, context} ->
-                {:ok, {unify_kinds(source_kind, target_kind), source_key, value}, context}
-
-              {:error, reason} ->
-                {:error, reason}
-            end
-
-          nil ->
-            {:ok, {source_kind, source_key, source_value}, context}
-        end
-      end)
+    {source_required, source_optional} = split_pairs(source_pairs)
+    {target_required, target_optional} = split_pairs(target_pairs)
+
+    with {:ok, source_required_pairs, context} <-
+           unify_source_required(source_required, target_pairs, stack, context),
+         {:ok, target_required_pairs, context} <-
+           unify_target_required(target_required, source_pairs, stack, context),
+         {:ok, source_optional_pairs, context} <-
+           unify_source_optional(source_optional, target_optional, stack, context),
+         {:ok, target_optional_pairs, context} <-
+           unify_target_optional(target_optional, source_optional, stack, context),
+         pairs =
+           [
+             source_required_pairs,
+             target_required_pairs,
+             source_optional_pairs,
+             target_optional_pairs
+           ]
+           |> Enum.concat()
+           # Remove duplicate pairs from matching in both left and right directions
+           |> Enum.uniq()
+           |> simplify_struct() do
+      {:ok, {:map, pairs}, context}
+    else
+      {:error, :unify} ->
+        error({:unable_unify, {:map, source_pairs}, {:map, target_pairs}}, stack, context)
 
-    case result do
-      {:ok, pairs, context} -> {:ok, {:map, simplify_struct(pairs)}, context}
-      {:error, reason} -> {:error, reason}
+      {:error, context} ->
+        {:error, context}
     end
   end
 
-  defp unify_maps(source_pairs, target_pairs, %{context: :expr} = stack, context) do
-    source_pairs = expand_struct(source_pairs)
-    target_pairs = expand_struct(target_pairs)
+  defp unify_source_required(source_required, target_pairs, stack, context) do
+    map_reduce_ok(source_required, context, fn {source_key, source_value}, context ->
+      Enum.find_value(target_pairs, fn {target_kind, target_key, target_value} ->
+        with {:ok, key, context} <- unify(source_key, target_key, stack, context) do
+          case unify(source_value, target_value, stack, context) do
+            {:ok, value, context} ->
+              {:ok, {:required, key, value}, context}
 
-    result =
-      flat_map_reduce_ok(source_pairs, context, fn {source_kind, _, _} = source_pair, context ->
-        # Currently we only match on exact and dynamic types
-        # since those are the only we get from map.key
-        with :error <- exact_map_match(source_pair, target_pairs, stack, context),
-             :error <- dynamic_map_match(source_pair, target_pairs, stack, context) do
-          if source_kind == :optional do
-            {:ok, [], context}
-          else
-            source_map = {:map, simplify_struct(source_pairs)}
-            target_map = {:map, simplify_struct(target_pairs)}
-            error({:unable_unify, source_map, target_map}, stack, context)
+            {:error, _reason} ->
+              source_map = {:map, [{:required, source_key, source_value}]}
+              target_map = {:map, [{target_kind, target_key, target_value}]}
+              error({:unable_unify, source_map, target_map}, stack, context)
           end
+        else
+          {:error, _reason} -> nil
         end
-      end)
+      end) || {:error, :unify}
+    end)
+  end
 
-    case result do
-      {:ok, pairs, context} ->
-        pairs = Enum.uniq_by(pairs ++ target_pairs, fn {_kind, key, _value} -> key end)
-        {:ok, {:map, simplify_struct(pairs)}, context}
+  defp unify_target_required(target_required, source_pairs, stack, context) do
+    map_reduce_ok(target_required, context, fn {target_key, target_value}, context ->
+      Enum.find_value(source_pairs, fn {source_kind, source_key, source_value} ->
+        with {:ok, key, context} <- unify(source_key, target_key, stack, context) do
+          case unify(source_value, target_value, stack, context) do
+            {:ok, value, context} ->
+              {:ok, {:required, key, value}, context}
 
-      {:error, reason} ->
-        {:error, reason}
-    end
+            {:error, _reason} ->
+              source_map = {:map, [{source_kind, source_key, source_value}]}
+              target_map = {:map, [{:required, target_key, target_value}]}
+              error({:unable_unify, source_map, target_map}, stack, context)
+          end
+        else
+          {:error, _reason} -> nil
+        end
+      end) || {:error, :unify}
+    end)
   end
 
-  defp exact_map_match({source_kind, source_key, source_value}, target_pairs, stack, context) do
-    case List.keyfind(target_pairs, source_key, 1) do
-      {target_kind, ^source_key, target_value} ->
-        case unify(source_value, target_value, stack, context) do
-          {:ok, value, context} ->
-            {:ok, [{unify_kinds(source_kind, target_kind), source_key, value}], context}
+  defp unify_source_optional(source_optional, target_optional, stack, context) do
+    flat_map_reduce_ok(source_optional, context, fn {source_key, source_value}, context ->
+      Enum.find_value(target_optional, fn {target_key, target_value} ->
+        with {:ok, key, context} <- unify(source_key, target_key, stack, context) do
+          case unify(source_value, target_value, stack, context) do
+            {:ok, value, context} ->
+              {:ok, [{:optional, key, value}], context}
 
-          {:error, reason} ->
-            {:error, reason}
+            {:error, _reason} ->
+              source_map = {:map, [{:optional, source_key, source_value}]}
+              target_map = {:map, [{:optional, target_key, target_value}]}
+              error({:unable_unify, source_map, target_map}, stack, context)
+          end
+        else
+          _ -> nil
         end
-
-      nil ->
-        :error
-    end
+      end) || {:ok, [], context}
+    end)
   end
 
-  defp dynamic_map_match({source_kind, source_key, source_value}, target_pairs, stack, context) do
-    case dynamic_pair(target_pairs) do
-      {:ok, {_target_kind, target_value}} ->
-        case unify(source_value, target_value, stack, context) do
-          {:ok, value, context} ->
-            {:ok, [{source_kind, source_key, value}], context}
+  defp unify_target_optional(target_optional, source_optional, stack, context) do
+    flat_map_reduce_ok(target_optional, context, fn {target_key, target_value}, context ->
+      Enum.find_value(source_optional, fn {source_key, source_value} ->
+        with {:ok, key, context} <- unify(source_key, target_key, stack, context) do
+          case unify(source_value, target_value, stack, context) do
+            {:ok, value, context} ->
+              {:ok, [{:optional, key, value}], context}
 
-          {:error, reason} ->
-            {:error, reason}
+            {:error, _reason} ->
+              source_map = {:map, [{:optional, source_key, source_value}]}
+              target_map = {:map, [{:optional, target_key, target_value}]}
+              error({:unable_unify, source_map, target_map}, stack, context)
+          end
+        else
+          _ -> nil
         end
+      end) || {:ok, [], context}
+    end)
+  end
 
-      :error ->
-        :error
-    end
+  defp split_pairs(pairs) do
+    {required, optional} =
+      Enum.split_with(pairs, fn {kind, _key, _value} -> kind == :required end)
+
+    required = Enum.map(required, fn {_kind, key, value} -> {key, value} end)
+    optional = Enum.map(optional, fn {_kind, key, value} -> {key, value} end)
+    {required, optional}
   end
 
   @doc """
@@ -467,6 +495,8 @@ defmodule Module.Types.Infer do
   def unify_kinds(:optional, :optional), do: :optional
 
   # Collect relevant information from context and traces to report error
+  # TODO: We should do this lazily since in some cases unification will error
+  #       but we continue attempting unifying other types
   defp error({:unable_unify, left, right}, stack, context) do
     {fun, arity} = context.function
     line = get_meta(stack.last_expr)[:line]
@@ -491,13 +521,11 @@ defmodule Module.Types.Infer do
       |> Enum.uniq()
 
     Enum.flat_map(stack, fn var_index ->
-      case Map.fetch(context.traces, var_index) do
-        {:ok, traces} ->
-          expr_var = Map.fetch!(context.types_to_vars, var_index)
-          Enum.map(traces, &{expr_var, &1})
-
-        _other ->
-          []
+      with %{^var_index => traces} <- context.traces,
+           %{^var_index => expr_var} <- context.types_to_vars do
+        Enum.map(traces, &{expr_var, &1})
+      else
+        _other -> []
       end
     end)
   end
@@ -564,17 +592,11 @@ defmodule Module.Types.Infer do
     end
   end
 
+  # TODO: Resolve type variables if %{__struct__: var, ...}
   defp fetch_struct_pair(pairs) do
     case Enum.find(pairs, &match?({:required, {:atom, :__struct__}, {:atom, _}}, &1)) do
       {:required, {:atom, :__struct__}, {:atom, module}} -> {:ok, module}
       nil -> :error
     end
   end
-
-  defp dynamic_pair(pairs) do
-    case List.keyfind(pairs, :dynamic, 1) do
-      {kind, :dynamic, type} -> {:ok, {kind, type}}
-      nil -> :error
-    end
-  end
 end

lib/elixir/src/elixir_errors.erl

@@ -38,9 +38,13 @@ form_error(Meta, File, Module, Desc) ->
 
 -spec form_warn(list(), binary() | #{file := binary(), _ => _}, module(), any()) -> ok.
 form_warn(Meta, File, Module, Desc) when is_list(Meta), is_binary(File) ->
-  do_form_warn(Meta, File, #{}, Module:format_error(Desc));
+  form_warn(Meta, #{file => File}, Module, Desc);
 form_warn(Meta, #{file := File} = E, Module, Desc) when is_list(Meta) ->
-  do_form_warn(Meta, File, E, Module:format_error(Desc)).
+  % Skip warnings during bootstrap, they will be reported during recompilation
+  case elixir_config:get(bootstrap) of
+    true -> ok;
+    false -> do_form_warn(Meta, File, E, Module:format_error(Desc))
+  end.
 
 do_form_warn(Meta, GivenFile, E, Warning) ->
   [{file, File}, {line, Line}] = meta_location(Meta, GivenFile),
@@ -190,4 +194,4 @@ raise(Location, File, Kind, Message) when is_binary(File) ->
 raise(Kind, Message, Opts) when is_binary(Message) ->
   Stacktrace = try throw(ok) catch _:_:Stack -> Stack end,
   Exception = Kind:exception([{description, Message} | Opts]),
-  erlang:raise(error, Exception, tl(Stacktrace)).
+  erlang:raise(error, Exception, tl(Stacktrace)).