Numberize fixes

lib/elixir/lib/module/types/apply.ex

@@ -861,30 +861,33 @@ defmodule Module.Types.Apply do
       skip_check? or not is_warning(stack) ->
         {result, context}
 
-      name in [:==, :"/="] and number_type?(left_type) and number_type?(right_type) ->
+      not disjoint?(left_type, right_type) ->
         {result, context}
 
-      # This check is incomplete. After all, we could have the number type nested
-      # inside a tuple or a list and the comparison would still be valid.
-      # However, nested comparison between distinct numbers is very uncommon,
-      # so we only check the direct value here.
-      disjoint?(left_type, right_type) ->
-        error = {:mismatched_comparison, left_type, right_type}
-        remote_error(error, :erlang, name, 2, expr, stack, context)
+      # `==` coerces integers and floats, at the top-level and nested inside
+      # containers alike, so `{1} == {1.0}` holds even though the types are
+      # disjoint as sets. `numberize/1` closes both sides over that coercion.
+      # `=:=` has no such coercion, so disjointedness settles it right away.
+      name in [:==, :"/="] and not disjoint?(numberize(left_type), numberize(right_type)) ->
+        {result, context}
 
       true ->
-        {result, context}
+        error = {:mismatched_comparison, left_type, right_type}
+        remote_error(error, :erlang, name, 2, expr, stack, context)
     end
   end
 
   defp mismatched_ordered_comparison(left_type, right_type, stack) do
     if is_warning(stack) do
       cond do
-        # These checks are incomplete. After all, we could have numbers and
-        # structs nested inside tuples or lists, but we only check the direct
-        # value here.
-        not (number_type?(left_type) and number_type?(right_type)) and
-            disjoint?(left_type, right_type) ->
+        # Ordered comparisons compare numbers by value, at the top-level and
+        # nested inside containers alike, so `numberize/1` closes both sides
+        # over that coercion before we call them distinct.
+        #
+        # The struct check below is still incomplete: we could have structs
+        # nested inside tuples or lists, but we only check the direct value.
+        disjoint?(left_type, right_type) and
+            disjoint?(numberize(left_type), numberize(right_type)) ->
           {:mismatched_comparison, left_type, right_type}
 
         match?({false, _}, map_fetch_key(dynamic(left_type), :__struct__)) and

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

@@ -584,22 +584,33 @@ defmodule Module.Types.Descr do
   defp numberize(:bitmap, bitmap), do: bitmap
 
   defp numberize(:map, bdd) do
-    bdd_map(bdd, fn bdd_leaf(tag, fields) ->
+    bdd_map_positive(bdd, fn bdd_leaf(tag, fields) ->
       bdd_leaf_new(
-        tag,
+        numberize_map_tag(tag),
         fields_map(fn _key, {value, optional?} -> {numberize(value), optional?} end, fields)
       )
     end)
   end
 
   defp numberize(:tuple, bdd) do
-    bdd_map(bdd, fn bdd_leaf(tag, fields) -> bdd_leaf_new(tag, Enum.map(fields, &numberize/1)) end)
+    bdd_map_positive(bdd, fn bdd_leaf(tag, fields) ->
+      bdd_leaf_new(tag, Enum.map(fields, &numberize/1))
+    end)
   end
 
   defp numberize(:list, bdd) do
-    bdd_map(bdd, fn bdd_leaf(head, tail) -> bdd_leaf_new(numberize(head), numberize(tail)) end)
+    bdd_map_positive(bdd, fn bdd_leaf(head, tail) ->
+      bdd_leaf_new(numberize(head), numberize(tail))
+    end)
   end
 
+  # Map keys are compared exactly by `==`, only their values coerce, so the
+  # domain keys are kept as is and only the types they point to are widened.
+  defp numberize_map_tag(domains) when is_list(domains),
+    do: fields_map(fn _key, value -> numberize(value) end, domains)
+
+  defp numberize_map_tag(tag), do: tag
+
   @doc """
   Returns if the type is a singleton.
   """
@@ -1036,15 +1047,6 @@ defmodule Module.Types.Descr do
 
   def bitstring_no_binary_type?(_), do: false
 
-  @doc """
-  Optimized version of `not empty?(bare_intersection(integer() or float(), type))`.
-  """
-  def number_type?(:term), do: true
-  def number_type?(%{dynamic: :term}), do: true
-  def number_type?(%{dynamic: %{bitmap: bitmap}}) when (bitmap &&& @bit_number) != 0, do: true
-  def number_type?(%{bitmap: bitmap}) when (bitmap &&& @bit_number) != 0, do: true
-  def number_type?(_), do: false
-
   ## Bitmaps
 
   defp bitmap_to_quoted(val) do
@@ -6167,6 +6169,39 @@ defmodule Module.Types.Descr do
     end
   end
 
+  # Like `bdd_map/2`, but only rewrites leaves in *positive* position.
+  #
+  # `bdd_map/2` is polarity-blind: on `A and not B` it rewrites `B` too, so a
+  # widening `fun` shrinks the result instead of enlarging it (`numberize/1`
+  # replacing `not {float()}` by `not {number()}` would drop `{1}` from the
+  # result even though `{1} == {1.0}`). Whenever `fun` changes a negated leaf,
+  # we drop the negation altogether: over-approximating is the safe direction
+  # for widening callers. Negations left untouched by `fun` are preserved,
+  # so the common negation-free case keeps its exact shape.
+  defp bdd_map_positive(bdd, fun) do
+    case bdd do
+      :bdd_bot ->
+        :bdd_bot
+
+      :bdd_top ->
+        :bdd_top
+
+      bdd_leaf(_, _) = leaf ->
+        fun.(leaf)
+
+      {_, leaf, left, union, right} ->
+        left = bdd_map_positive(left, fun)
+        union = bdd_map_positive(union, fun)
+        right = bdd_map_positive(right, fun)
+
+        case fun.(leaf) do
+          ^leaf -> bdd_node_new(leaf, left, union, right)
+          new_leaf when right == :bdd_bot -> bdd_node_new(new_leaf, left, union, :bdd_bot)
+          new_leaf -> bdd_union(bdd_intersection(new_leaf, left), bdd_union(union, right))
+        end
+    end
+  end
+
   defp bdd_reduce(bdd, acc, fun) do
     case bdd do
       :bdd_bot ->

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

@@ -2737,6 +2737,40 @@ defmodule Module.Types.DescrTest do
       assert dynamic(list(binary(), float())) |> numberize() ==
                dynamic(list(binary(), number()))
     end
+
+    test "with domain keys" do
+      # `==` compares map keys exactly but coerces values, so only the types
+      # the domain keys point to are widened.
+      assert closed_map([{domain_key(:integer), integer()}]) |> numberize() ==
+               closed_map([{domain_key(:integer), number()}])
+
+      assert open_map([{domain_key(:tuple), tuple([float()])}, {:a, {integer(), false}}])
+             |> numberize() ==
+               open_map([{domain_key(:tuple), tuple([number()])}, {:a, {number(), false}}])
+    end
+
+    test "with negations" do
+      # Negations must not be widened: `{1}` belongs to `term() and not {float()}`
+      # and `{1} == {1.0}`, so both must survive numberize.
+      negated = opt_difference(term(), tuple([float()]))
+      assert subtype?(tuple([integer()]), numberize(negated))
+      assert subtype?(tuple([float()]), numberize(negated))
+
+      negated = opt_difference(non_empty_list(integer(), atom()), non_empty_list(float(), atom()))
+      assert subtype?(non_empty_list(integer(), atom()), numberize(negated))
+      assert subtype?(non_empty_list(float(), atom()), numberize(negated))
+
+      negated = opt_difference(open_map(a: {integer(), false}), open_map(a: {float(), false}))
+      assert subtype?(open_map(a: {integer(), false}), numberize(negated))
+      assert subtype?(open_map(a: {float(), false}), numberize(negated))
+
+      # Negations untouched by numberize are kept as is
+      type = opt_difference(tuple(), tuple([binary()]))
+      assert numberize(type) == type
+
+      type = opt_difference(list(integer(), atom()), list(binary(), atom()))
+      assert numberize(type) == opt_difference(list(number(), atom()), list(binary(), atom()))
+    end
   end
 
   describe "map_get" do

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

@@ -1605,6 +1605,22 @@ defmodule Module.Types.ExprTest do
       assert typecheck!([x = 123, y = 456.0], min(x, y)) == dynamic(opt_union(integer(), float()))
     end
 
+    test "does not warn when numbers nested in containers coerce" do
+      # `{1} == {1.0}` is true at runtime, and `[1] < [2.0]` is not constant,
+      # even though the container types are disjoint as sets.
+      assert typecheck!([x = {123}, y = {456.0}], x == y) == boolean()
+      assert typecheck!([x = [123], y = [456.0]], x < y) == boolean()
+      assert typecheck!([x = %{a: 123}, y = %{a: 456.0}], x != y) == boolean()
+
+      # `===` does not coerce, so the nested types stay distinct
+      assert typeerror!([x = {123}, y = {456.0}], x === y) =~
+               "comparison between distinct types found"
+
+      # Neither does a nested type that has no number to coerce with
+      assert typeerror!([x = {123}, y = {"foo"}], x == y) =~
+               "comparison between distinct types found"
+    end
+
     test "warns when comparison is constant" do
       assert typeerror!([x = :foo, y = 321], min(x, y)) ==
                ~l"""
@@ -2094,6 +2110,49 @@ defmodule Module.Types.ExprTest do
                atom([:non_empty_map, :maybe_empty_map])
     end
 
+    test "does not discard equal values when narrowing against negated types" do
+      # `other` carries a negation (`... and not {float()}`) from the clause
+      # subtraction. Widening integers/floats inside that negation would wrongly
+      # remove `{1}`, even though `{1} == {1.0}` at runtime.
+      assert typecheck!(
+               [v, q],
+               (
+                 other =
+                   case v do
+                     {a} when is_float(a) -> {:float, a}
+                     other -> other
+                   end
+
+                 w = if q, do: {1}, else: {1.5}
+
+                 case w do
+                   x when x == other -> {:matched, x}
+                   _ -> :nomatch
+                 end
+               )
+             )
+             |> to_quoted_string() ==
+               "dynamic({:matched, {float() or integer()}}) or :nomatch"
+    end
+
+    test "does not discard equal values when narrowing against domain keys" do
+      # `==` coerces map values, so narrowing `x` from `x == y` must widen the
+      # `integer() => integer()` domain value to `integer() => number()`.
+      assert typecheck!(
+               (
+                 y = %{1 => 1}
+                 w = %{1 => 1.0}
+
+                 case w do
+                   x when x == y and map_size(x) == 1 -> {:eq, x}
+                   _ -> :ne
+                 end
+               )
+             )
+             |> to_quoted_string() ==
+               ":ne or {:eq, %{integer() => float()} and not empty_map()}"
+    end
+
     test "consider external variables as not precise" do
       assert typecheck!(
                [x],