diff --git a/lib/elixir/lib/module/types/apply.ex b/lib/elixir/lib/module/types/apply.ex index 958c9ea269..d49e1b9266 100644 --- a/lib/elixir/lib/module/types/apply.ex +++ b/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 diff --git a/lib/elixir/lib/module/types/descr.ex b/lib/elixir/lib/module/types/descr.ex index 0017204c4f..73769979bd 100644 --- a/lib/elixir/lib/module/types/descr.ex +++ b/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 -> diff --git a/lib/elixir/test/elixir/module/types/descr_test.exs b/lib/elixir/test/elixir/module/types/descr_test.exs index 3041600d83..d419e94fb8 100644 --- a/lib/elixir/test/elixir/module/types/descr_test.exs +++ b/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 diff --git a/lib/elixir/test/elixir/module/types/expr_test.exs b/lib/elixir/test/elixir/module/types/expr_test.exs index 3604340572..e374872f9e 100644 --- a/lib/elixir/test/elixir/module/types/expr_test.exs +++ b/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],