Skip to content

Fixes to support 0TP27 #13351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sabiwara merged 6 commits into from
Feb 16, 2024
Merged

Fixes to support 0TP27 #13351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
06cbefe
Fix non-deterministic key-value tests
sabiwara Feb 15, 2024
fff947b
Fix non-deterministic Enum tests
sabiwara Feb 15, 2024
0b7ff47
Fix :only option when deriving Inspect
sabiwara Feb 15, 2024
5edfdd6
Float.ceil and Float.floor return -0.0 for negative numbers
sabiwara Feb 15, 2024
ab31e6c
Fix non-deterministic Registry doctest
sabiwara Feb 15, 2024
d59afeb
Simplify check
sabiwara Feb 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions lib/elixir/lib/enum.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3809,9 +3809,6 @@ defmodule Enum do
iex> Enum.unzip([{:a, 1}, {:b, 2}, {:c, 3}])
{[:a, :b, :c], [1, 2, 3]}

iex> Enum.unzip(%{a: 1, b: 2})
{[:a, :b], [1, 2]}
Comment on lines -3812 to -3813
Copy link
Contributor Author

@sabiwara sabiwara Feb 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see a relevant test to be added, a one-size map would hide the fact that the keys are misordered, we already have a keyword above...


"""
@spec unzip(t) :: {[element], [element]}

Expand Down Expand Up @@ -4067,7 +4064,7 @@ defmodule Enum do
...> end)
[{1, 2, 3}, {1, 2, 3}]

iex> enums = [[1, 2], %{a: 3, b: 4}, [5, 6]]
iex> enums = [[1, 2], [a: 3, b: 4], [5, 6]]
...> Enum.zip_reduce(enums, [], fn elements, acc ->
...> [List.to_tuple(elements) | acc]
...> end)
Expand Down
16 changes: 15 additions & 1 deletion lib/elixir/lib/float.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,8 @@ defmodule Float do
-56.0
iex> Float.ceil(34.251, 2)
34.26
iex> Float.ceil(-0.01)
-0.0

"""
@spec ceil(float, precision_range) :: float
Expand Down Expand Up @@ -332,6 +334,8 @@ defmodule Float do
-6.0
iex> Float.round(12.341444444444441, 15)
12.341444444444441
iex> Float.round(-0.01)
-0.0

"""
@spec round(float, precision_range) :: float
Expand All @@ -340,8 +344,13 @@ defmodule Float do
# and could be implemented in the future.
def round(float, precision \\ 0)

def round(float, 0) when float == 0.0, do: float

def round(float, 0) when is_float(float) do
float |> :erlang.round() |> :erlang.float()
case float |> :erlang.round() |> :erlang.float() do
zero when zero == 0.0 and float < 0.0 -> -0.0
rounded -> rounded
end
end

def round(float, precision) when is_float(float) and precision in @precision_range do
Expand All @@ -365,6 +374,8 @@ defmodule Float do
case rounding do
:ceil when sign === 0 -> 1 / power_of_10(precision)
:floor when sign === 1 -> -1 / power_of_10(precision)
:ceil when sign === 1 -> -0.0
Copy link
Contributor Author

@sabiwara sabiwara Feb 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixing Float.ceil/2 to match Float.ceil/1 and :math.ceil/1

Copy link
Contributor Author

@sabiwara sabiwara Feb 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I'm not sure, we might want to rethink this whole rounding spec.

Here is the current behavior:

Erlang/OTP 26 [erts-14.2.1] [source] [64-bit] [smp:8:8] [ds:8:8:10] [async-threads:1] [jit]

Interactive Elixir (1.16.1) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> for fun <- [:round, :ceil], num <- [-0.0, -0.01], prec <- 0..1, do: IO.puts "#{fun}(#{num}), #{prec}) = #{apply(Float, fun, [num, prec])}"
round(-0.0), 0) = 0.0
round(-0.0), 1) = -0.0
round(-0.01), 0) = 0.0
round(-0.01), 1) = 0.0
ceil(-0.0), 0) = -0.0
ceil(-0.0), 1) = -0.0
ceil(-0.01), 0) = -0.0
ceil(-0.01), 1) = 0.0

It feels like we'd want to have to following invariants:

  • ceil(x) >= round(x)
  • fun(-0.01) <= fun(-0.0)
  • fun(-0.01, 0) === fun(-0.01, 1)

but none of these are currently respected.

How about the following?

round(-0.0), 0) = -0.0
round(-0.0), 1) = -0.0
round(-0.01), 0) = -0.0
round(-0.01), 1) = -0.0
ceil(-0.0), 0) = -0.0
ceil(-0.0), 1) = -0.0
ceil(-0.01), 0) = -0.0
ceil(-0.01), 1) = -0.0

Seems consistent with what Javascript and Python are doing:

> Math.round(-0.0, 0)
-0
> Math.round(-0.0, 1)
-0
> Math.ceil(-0.0, 1)
-0
> Math.ceil(-0.01, 1)
-0
>>> round(-0.0, 0)
-0.0
>>> round(-0.0, 1)
-0.0
>>> round(-0.01, 1)
-0.0
>>> round(-0.01, 0)
-0.0

Copy link
Member

@josevalim josevalim Feb 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks excellent to me, awesome summary and proposal.

Copy link
Contributor Author

@sabiwara sabiwara Feb 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK should be good! 5edfdd6

:half_up when sign === 1 -> -0.0
_ -> 0.0
end

Expand Down Expand Up @@ -394,6 +405,9 @@ defmodule Float do
boundary = den <<< 52

cond do
num == 0 and sign == 1 ->
-0.0

num == 0 ->
0.0

Expand Down
5 changes: 3 additions & 2 deletions lib/elixir/lib/inspect.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,8 @@ end

defimpl Inspect, for: Any do
defmacro __deriving__(module, struct, options) do
fields = Map.keys(struct) -- [:__exception__, :__struct__]
fields = Enum.sort(Map.keys(struct) -- [:__exception__, :__struct__])

only = Keyword.get(options, :only, fields)
except = Keyword.get(options, :except, [])
optional = Keyword.get(options, :optional, [])
Expand All @@ -545,7 +546,7 @@ defimpl Inspect, for: Any do
:ok = validate_option(:optional, optional, fields, module)

inspect_module =
if fields == only and except == [] do
if fields == Enum.sort(only) and except == [] do
Inspect.Map
else
Inspect.Any
Expand Down
4 changes: 2 additions & 2 deletions lib/elixir/lib/map.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ defmodule Map do

## Examples

iex> Map.keys(%{a: 1, b: 2})
Map.keys(%{a: 1, b: 2})
[:a, :b]

"""
Expand All @@ -161,7 +161,7 @@ defmodule Map do

## Examples

iex> Map.values(%{a: 1, b: 2})
Map.values(%{a: 1, b: 2})
[1, 2]

"""
Expand Down
8 changes: 4 additions & 4 deletions lib/elixir/lib/registry.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1301,16 +1301,16 @@ defmodule Registry do
iex> Registry.start_link(keys: :unique, name: Registry.SelectAllTest)
iex> {:ok, _} = Registry.register(Registry.SelectAllTest, "hello", :value)
iex> {:ok, _} = Registry.register(Registry.SelectAllTest, "world", :value)
iex> Registry.select(Registry.SelectAllTest, [{{:"$1", :"$2", :"$3"}, [], [{{:"$1", :"$2", :"$3"}}]}])
[{"world", self(), :value}, {"hello", self(), :value}]
iex> Registry.select(Registry.SelectAllTest, [{{:"$1", :"$2", :"$3"}, [], [{{:"$1", :"$2", :"$3"}}]}]) |> Enum.sort()
Copy link
Contributor Author

@sabiwara sabiwara Feb 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's probably this change in :ets, adding the sort in the doctest makes it obvious that we should not rely on the order? (unit tests are already doing this)

[{"hello", self(), :value}, {"world", self(), :value}]

Get all keys in the registry:

iex> Registry.start_link(keys: :unique, name: Registry.SelectAllTest)
iex> {:ok, _} = Registry.register(Registry.SelectAllTest, "hello", :value)
iex> {:ok, _} = Registry.register(Registry.SelectAllTest, "world", :value)
iex> Registry.select(Registry.SelectAllTest, [{{:"$1", :_, :_}, [], [:"$1"]}])
["world", "hello"]
iex> Registry.select(Registry.SelectAllTest, [{{:"$1", :_, :_}, [], [:"$1"]}]) |> Enum.sort()
["hello", "world"]

"""
@doc since: "1.9.0"
Expand Down
8 changes: 4 additions & 4 deletions lib/elixir/test/elixir/enum_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ defmodule EnumTest do
end

test "mix and match" do
enums = [[1, 2], %{a: 3, b: 4}, [5, 6]]
enums = [[1, 2], 3..4, [5, 6]]
result = Enum.zip_reduce(enums, [], fn elements, acc -> [List.to_tuple(elements) | acc] end)
assert result == [{2, {:b, 4}, 6}, {1, {:a, 3}, 5}]
assert result == [{2, 4, 6}, {1, 3, 5}]
end
end

Expand Down Expand Up @@ -412,7 +412,7 @@ defmodule EnumTest do
assert Enum.into([a: 1, b: 2], %{c: 3}) == %{a: 1, b: 2, c: 3}
assert Enum.into(MapSet.new(a: 1, b: 2), %{}) == %{a: 1, b: 2}
assert Enum.into(MapSet.new(a: 1, b: 2), %{c: 3}) == %{a: 1, b: 2, c: 3}
assert Enum.into(%{a: 1, b: 2}, []) == [a: 1, b: 2]
assert Enum.into(%{a: 1, b: 2}, []) |> Enum.sort() == [a: 1, b: 2]
assert Enum.into(1..3, []) == [1, 2, 3]
assert Enum.into(["H", "i"], "") == "Hi"
end
Expand Down Expand Up @@ -1444,7 +1444,7 @@ defmodule EnumTest do
test "unzip/1" do
assert Enum.unzip([{:a, 1}, {:b, 2}, {:c, 3}]) == {[:a, :b, :c], [1, 2, 3]}
assert Enum.unzip([]) == {[], []}
assert Enum.unzip(%{a: 1, b: 2}) == {[:a, :b], [1, 2]}
assert Enum.unzip(%{a: 1}) == {[:a], [1]}
assert Enum.unzip(foo: "a", bar: "b") == {[:foo, :bar], ["a", "b"]}

assert_raise FunctionClauseError, fn -> Enum.unzip([{:a, 1}, {:b, 2, "foo"}]) end
Expand Down
22 changes: 19 additions & 3 deletions lib/elixir/test/elixir/float_test.exs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ defmodule FloatTest do
assert Float.ceil(7.5432e3) === 7544.0
assert Float.ceil(7.5e-3) === 1.0
assert Float.ceil(-12.32453e4) === -123_245.0
assert Float.ceil(-12.32453e-10) === 0.0
assert Float.ceil(-12.32453e-10) === -0.0
Copy link
Contributor Author

@sabiwara sabiwara Feb 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was already the case on OTP26, the test was misleading precisely because 0.0 === -0.0 was true

assert Float.ceil(0.32453e-10) === 1.0
assert Float.ceil(-0.32453e-10) === 0.0
assert Float.ceil(-0.32453e-10) === -0.0
assert Float.ceil(1.32453e-10) === 1.0
assert Float.ceil(0.0) === 0.0
end
Expand All @@ -130,7 +130,7 @@ defmodule FloatTest do
assert Float.ceil(-12.524235, 3) === -12.524

assert Float.ceil(12.32453e-20, 2) === 0.01
assert Float.ceil(-12.32453e-20, 2) === 0.0
assert Float.ceil(-12.32453e-20, 2) === -0.0

assert Float.ceil(0.0, 2) === 0.0

Expand All @@ -139,6 +139,11 @@ defmodule FloatTest do
end
end

test "with small floats rounded up to -0.0" do
assert Float.ceil(-0.1, 0) === -0.0
assert Float.ceil(-0.01, 1) === -0.0
end

test "with subnormal floats" do
assert Float.ceil(5.0e-324, 0) === 1.0
assert Float.ceil(5.0e-324, 1) === 0.1
Expand Down Expand Up @@ -172,6 +177,17 @@ defmodule FloatTest do
end
end

test "with small floats rounded to +0.0 / -0.0" do
assert Float.round(0.01, 0) === 0.0
assert Float.round(0.01, 1) === 0.0

assert Float.round(-0.01, 0) === -0.0
assert Float.round(-0.01, 1) === -0.0

assert Float.round(-0.49999, 0) === -0.0
assert Float.round(-0.049999, 1) === -0.0
end

test "with subnormal floats" do
for precision <- 0..15 do
assert Float.round(5.0e-324, precision) === 0.0
Expand Down