Use compatibility checks as much as possible

Author: josevalim

lib/elixir/lib/module/types/descr.ex

@@ -1885,6 +1885,7 @@ defmodule Module.Types.Descr do
         end
 
       :open ->
+        fields = Map.to_list(fields)
         {:%{}, [], [{:..., [], nil} | map_fields_to_quoted(tag, Enum.sort(fields), opts)]}
     end
   end

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

@@ -535,7 +535,7 @@ defmodule Module.Types.Expr do
     if stack.mode == :traversal do
       {dynamic(), context}
     else
-      Of.refine_existing_var(var, expected, expr, stack, context)
+      Of.refine_body_var(var, expected, expr, stack, context)
     end
   end
 
@@ -567,7 +567,10 @@ defmodule Module.Types.Expr do
         _ ->
           expected = if structs == [], do: @exception, else: Enum.reduce(structs, &union/2)
           formatter = fn expr -> {"rescue #{expr_to_string(expr)} ->", hints} end
-          {_ok?, _type, context} = Of.refine_var(var, expected, expr, formatter, stack, context)
+
+          {_ok?, _type, context} =
+            Of.refine_head_var(var, expected, expr, formatter, stack, context)
+
           context
       end
 

lib/elixir/lib/module/types/of.ex

@@ -42,13 +42,13 @@ defmodule Module.Types.Of do
   end
 
   @doc """
-  Refines a variable that already exists.
+  Refines a variable that already exists (in a body).
 
   This only happens if the var contains a gradual type,
   or if we are doing a guard analysis or occurrence typing.
   Returns `true` if there was a refinement, `false` otherwise.
   """
-  def refine_existing_var({_, meta, _}, type, expr, stack, context) do
+  def refine_body_var({_, meta, _}, type, expr, stack, context) do
     version = Keyword.fetch!(meta, :version)
     %{vars: %{^version => %{type: old_type, off_traces: off_traces} = data} = vars} = context
 
@@ -76,8 +76,12 @@ defmodule Module.Types.Of do
 
   @doc """
   Refines the type of a variable.
+
+  Since this happens in a head, we use intersection
+  because we want to refine types. Otherwise we should
+  use compatibility.
   """
-  def refine_var(var, type, expr, formatter \\ :default, stack, context) do
+  def refine_head_var(var, type, expr, formatter \\ :default, stack, context) do
     {var_name, meta, var_context} = var
     version = Keyword.fetch!(meta, :version)
 
@@ -96,7 +100,7 @@ defmodule Module.Types.Of do
         # We need to return error otherwise it leads to cascading errors
         if empty?(new_type) do
           {:error, error_type(),
-           error({:refine_var, old_type, type, var, context}, meta, stack, context)}
+           error({:refine_head_var, old_type, type, var, context}, meta, stack, context)}
         else
           {:ok, new_type, context}
         end
@@ -357,12 +361,13 @@ defmodule Module.Types.Of do
           Module.Types.Pattern.of_match_var(left, type, expr, stack, context)
 
         :guard ->
-          Module.Types.Pattern.of_guard(left, type, expr, stack, context)
+          {actual, context} = Module.Types.Pattern.of_guard(left, type, expr, stack, context)
+          compatible(actual, type, expr, stack, context)
 
         :expr ->
           left = annotate_interpolation(left, right)
           {actual, context} = Module.Types.Expr.of_expr(left, {type, expr}, stack, context)
-          intersect(actual, type, expr, stack, context)
+          compatible(actual, type, expr, stack, context)
       end
 
     specifier_size(kind, right, stack, context)
@@ -402,13 +407,14 @@ defmodule Module.Types.Of do
   defp specifier_size(:expr, {:size, _, [arg]} = expr, stack, context)
        when not is_integer(arg) do
     {actual, context} = Module.Types.Expr.of_expr(arg, {integer(), expr}, stack, context)
-    {_, context} = intersect(actual, integer(), expr, stack, context)
+    {_, context} = compatible(actual, integer(), expr, stack, context)
     context
   end
 
   defp specifier_size(_pattern_or_guard, {:size, _, [arg]} = expr, stack, context)
        when not is_integer(arg) do
-    {_type, context} = Module.Types.Pattern.of_guard(arg, integer(), expr, stack, context)
+    {actual, context} = Module.Types.Pattern.of_guard(arg, integer(), expr, stack, context)
+    {_, context} = compatible(actual, integer(), expr, stack, context)
     context
   end
 
@@ -437,15 +443,14 @@ defmodule Module.Types.Of do
   ## Warning helpers
 
   @doc """
-  Intersects two types and emit an incompatible error if empty.
+  Checks if two types are compatible and emit an incompatible error if not.
   """
-  def intersect(actual, expected, expr, stack, context) do
-    type = intersection(actual, expected)
-
-    if empty?(type) do
-      {error_type(), incompatible_error(expr, expected, actual, stack, context)}
+  # TODO: Consider getting rid of this and emitting precise errors instead.
+  def compatible(actual, expected, expr, stack, context) do
+    if compatible?(actual, expected) do
+      {actual, context}
     else
-      {type, context}
+      {error_type(), incompatible_error(expr, expected, actual, stack, context)}
     end
   end
 
@@ -468,7 +473,7 @@ defmodule Module.Types.Of do
 
   ## Warning formatting
 
-  def format_diagnostic({:refine_var, old_type, new_type, var, context}) do
+  def format_diagnostic({:refine_head_var, old_type, new_type, var, context}) do
     traces = collect_traces(var, context)
 
     %{

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

@@ -183,7 +183,7 @@ defmodule Module.Types.Pattern do
                     {var_changed?, context}
                   else
                     _ ->
-                      case Of.refine_var(var, type, expr, stack, context) do
+                      case Of.refine_head_var(var, type, expr, stack, context) do
                         {:ok, _type, context} -> {var_changed? or reachable_var?, context}
                         {:error, _type, context} -> throw(context)
                       end
@@ -337,23 +337,27 @@ defmodule Module.Types.Pattern do
   @doc """
   Function used to assign a type to a variable. Used by %struct{}
   and binary patterns.
+
+  Given those values are actually checked at compile-time,
+  except for the variables, that's the only scenario we need to handle.
   """
+  # TODO: Perhaps merge this with guards
   def of_match_var({:^, _, [var]}, expected, expr, stack, context) do
-    {type, context} = Of.refine_existing_var(var, expected, expr, stack, context)
-    Of.intersect(type, expected, expr, stack, context)
+    {type, context} = Of.refine_body_var(var, expected, expr, stack, context)
+    Of.compatible(type, expected, expr, stack, context)
   end
 
   def of_match_var({:_, _, _}, expected, _expr, _stack, context) do
     {expected, context}
   end
 
   def of_match_var(var, expected, expr, stack, context) when is_var(var) do
-    {_ok?, type, context} = Of.refine_var(var, expected, expr, stack, context)
+    {_ok?, type, context} = Of.refine_head_var(var, expected, expr, stack, context)
     {type, context}
   end
 
-  def of_match_var(ast, expected, expr, stack, context) do
-    of_match(ast, expected, expr, :default, stack, context)
+  def of_match_var(_ast, expected, _expr, _stack, context) do
+    {expected, context}
   end
 
   ## Patterns
@@ -678,9 +682,9 @@ defmodule Module.Types.Pattern do
 
   # ^var
   def of_guard({:^, _meta, [var]}, expected, expr, stack, context) do
-    # This is by definition a variable defined outside of this pattern, so we don't track it.
-    {type, context} = Of.refine_existing_var(var, expected, expr, stack, context)
-    Of.intersect(type, expected, expr, stack, context)
+    # This is used by binary size, which behaves as a mixture of match and guard
+    {type, context} = Of.refine_body_var(var, expected, expr, stack, context)
+    Of.compatible(type, expected, expr, stack, context)
   end
 
   # {...}
@@ -716,7 +720,7 @@ defmodule Module.Types.Pattern do
 
   # var
   def of_guard(var, expected, expr, stack, context) when is_var(var) do
-    Of.intersect(Of.var(var, context), expected, expr, stack, context)
+    Of.compatible(Of.var(var, context), expected, expr, stack, context)
   end
 
   ## Helpers

lib/elixir/src/elixir_bitstring.erl

@@ -1,6 +1,6 @@
 -module(elixir_bitstring).
 -export([expand/5, format_error/1, validate_spec/2]).
--import(elixir_errors, [function_error/4, file_error/4]).
+-import(elixir_errors, [function_error/4]).
 -include("elixir.hrl").
 
 expand_match(Expr, {S, OriginalS}, E) ->
@@ -13,11 +13,6 @@ expand(Meta, Args, S, E, RequireSize) ->
       {EArgs, Alignment, {SA, _}, EA} =
         expand(Meta, fun expand_match/3, Args, [], {S, S}, E, 0, RequireSize),
 
-      case find_match(EArgs) of
-        false -> ok;
-        Match -> file_error(Meta, EA, ?MODULE, {nested_match, Match})
-      end,
-
       {{'<<>>', [{alignment, Alignment} | Meta], EArgs}, SA, EA};
     _ ->
       PairS = {elixir_env:prepare_write(S), S},
@@ -32,6 +27,7 @@ expand(_BitstrMeta, _Fun, [], Acc, S, E, Alignment, _RequireSize) ->
   {lists:reverse(Acc), Alignment, S, E};
 expand(BitstrMeta, Fun, [{'::', Meta, [Left, Right]} | T], Acc, S, E, Alignment, RequireSize) ->
   {ELeft, {SL, OriginalS}, EL} = expand_expr(Left, Fun, S, E),
+  validate_expr(ELeft, Meta, E),
 
   MatchOrRequireSize = RequireSize or is_match_size(T, EL),
   EType = expr_type(ELeft),
@@ -47,6 +43,7 @@ expand(BitstrMeta, Fun, [{'::', Meta, [Left, Right]} | T], Acc, S, E, Alignment,
 expand(BitstrMeta, Fun, [H | T], Acc, S, E, Alignment, RequireSize) ->
   Meta = extract_meta(H, BitstrMeta),
   {ELeft, {SS, OriginalS}, ES} = expand_expr(H, Fun, S, E),
+  validate_expr(ELeft, Meta, E),
 
   MatchOrRequireSize = RequireSize or is_match_size(T, ES),
   EType = expr_type(ELeft),
@@ -145,6 +142,17 @@ expand_expr({{'.', _, [Mod, to_string]}, _, [Arg]} = AST, Fun, S, #{context := C
 expand_expr(Component, Fun, S, E) ->
   Fun(Component, S, E).
 
+validate_expr(Expr, Meta, #{context := match} = E) ->
+  case Expr of
+    {Var, _Meta, Ctx} when is_atom(Var), is_atom(Ctx) -> ok;
+    {'<<>>', _, _} -> ok;
+    {'^', _, _} -> ok;
+    _ when is_number(Expr); is_binary(Expr) -> ok;
+    _ -> function_error(extract_meta(Expr, Meta), E, ?MODULE, {unknown_match, Expr})
+  end;
+validate_expr(_Expr, _Meta, _E) ->
+  ok.
+
 %% Expands and normalizes types of a bitstring.
 
 expand_specs(ExprType, Meta, Info, S, OriginalS, E, ExpectSize) ->
@@ -353,18 +361,6 @@ valid_float_size(_) -> false.
 add_spec(default, Spec) -> Spec;
 add_spec(Key, Spec) -> [{Key, [], nil} | Spec].
 
-find_match([{'=', _, [_Left, _Right]} = Expr | _Rest]) ->
-  Expr;
-find_match([{_, _, Args} | Rest]) when is_list(Args) ->
-  case find_match(Args) of
-    false -> find_match(Rest);
-    Match -> Match
-  end;
-find_match([_Arg | Rest]) ->
-  find_match(Rest);
-find_match([]) ->
-  false.
-
 format_error({unaligned_binary, Expr}) ->
   Message = "expected ~ts to be a binary but its number of bits is not divisible by 8",
   io_lib:format(Message, ['Elixir.Macro':to_string(Expr)]);
@@ -409,10 +405,9 @@ format_error({bittype_mismatch, Val1, Val2, Where}) ->
 format_error({bad_unit_argument, Unit}) ->
   io_lib:format("unit in bitstring expects an integer as argument, got: ~ts",
                 ['Elixir.Macro':to_string(Unit)]);
-format_error({nested_match, Expr}) ->
+format_error({unknown_match, Expr}) ->
   Message =
-    "cannot pattern match inside a bitstring "
-      "that is already in match, got: ~ts",
+    "a bitstring only accepts binaries, numbers, and variables inside a match, got: ~ts",
   io_lib:format(Message, ['Elixir.Macro':to_string(Expr)]);
 format_error({undefined_var_in_spec, Var}) ->
   Message =

lib/elixir/test/elixir/kernel/errors_test.exs

@@ -941,7 +941,7 @@ defmodule Kernel.ErrorsTest do
 
   test "failed remote call stacktrace includes file/line info" do
     try do
-      bad_remote_call(1)
+      bad_remote_call(Process.get(:unused, 1))
     rescue
       ArgumentError ->
         assert [

lib/elixir/test/elixir/kernel/expansion_test.exs

@@ -2387,6 +2387,15 @@ defmodule Kernel.ExpansionTest do
                |> clean_bit_modifiers()
     end
 
+    test "invalid match" do
+      assert_compile_error(
+        "a bitstring only accepts binaries, numbers, and variables inside a match",
+        fn ->
+          expand(quote(do: <<%{}>> = foo()))
+        end
+      )
+    end
+
     test "nested match" do
       assert expand(quote(do: <<foo = bar>>)) |> clean_meta([:alignment]) ==
                quote(do: <<foo = bar()::integer>>) |> clean_bit_modifiers()
@@ -2395,16 +2404,6 @@ defmodule Kernel.ExpansionTest do
              |> clean_meta([:alignment]) ==
                quote(do: <<45::integer, <<_::integer, _::binary>> = rest()::binary>>)
                |> clean_bit_modifiers()
-
-      message = ~r"cannot pattern match inside a bitstring that is already in match"
-
-      assert_compile_error(message, fn ->
-        expand(quote(do: <<bar = baz>> = foo()))
-      end)
-
-      assert_compile_error(message, fn ->
-        expand(quote(do: <<?-, <<_, _::binary>> = rest::binary>> = foo()))
-      end)
     end
 
     test "inlines binaries inside interpolation" do