Skip to content

Optimize subtype? by aborting early #14561

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
merged 1 commit into from
Jun 7, 2025
+24 −2
Merged

Optimize subtype? by aborting early #14561

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 24 additions & 2 deletions lib/elixir/lib/module/types/descr.ex
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,26 @@ defmodule Module.Types.Descr do

defp iterator_difference_static(:none, map), do: map

defp empty_difference_static?(left, :term), do: not Map.has_key?(left, :optional)
Copy link
Contributor

Choose a reason for hiding this comment

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

@josevalim sorry asking for my understanding, couldn't figure this one out.

What would be an example case that justifies checking for optional?

Currently the following seems to pass all the tests, so it's probably not covered:

-  defp empty_difference_static?(left, :term), do: not Map.has_key?(left, :optional)
+  defp empty_difference_static?(left, :term), do: true

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think there is a practical example, but i am keeping semantic equivalence with the empty? function. empty? check if all fields empty and, if optional is set, it is not considered empty. So I am doing the same here. I have asked @gldubc if empty?(optional(none()) is true or false.


defp empty_difference_static?(left, right) do
iterator_empty_difference_static?(:maps.next(:maps.iterator(unfold(left))), unfold(right))
end

defp iterator_empty_difference_static?({key, v1, iterator}, map) do
case map do
%{^key => v2} ->
value = difference(key, v1, v2)
value in @empty_difference or empty_key?(key, value)

%{} ->
empty_key?(key, v1)
end and
iterator_empty_difference_static?(:maps.next(iterator), map)
end

defp iterator_empty_difference_static?(:none, _map), do: true

# Returning 0 from the callback is taken as none() for that subtype.
defp difference(:atom, v1, v2), do: atom_difference(v1, v2)
defp difference(:bitmap, v1, v2), do: v1 - (v1 &&& v2)
Expand Down Expand Up @@ -491,7 +511,7 @@ defmodule Module.Types.Descr do
def empty?(:term), do: false

def empty?(%{} = descr) do
case :maps.get(:dynamic, descr, _default = descr) do
case :maps.get(:dynamic, descr, descr) do
:term ->
false

Expand All @@ -511,9 +531,11 @@ defmodule Module.Types.Descr do

# For atom, bitmap, and optional, if the key is present,
# then they are not empty,
defp empty_key?(:fun, value), do: fun_empty?(value)
defp empty_key?(:map, value), do: map_empty?(value)
defp empty_key?(:list, value), do: list_empty?(value)
defp empty_key?(:tuple, value), do: tuple_empty?(value)
defp empty_key?(:dynamic, value), do: empty?(value)
defp empty_key?(_, _value), do: false

@doc """
Expand Down Expand Up @@ -640,7 +662,7 @@ defmodule Module.Types.Descr do
end

defp subtype_static?(same, same), do: true
defp subtype_static?(left, right), do: empty?(difference_static(left, right))
defp subtype_static?(left, right), do: empty_difference_static?(left, right)

@doc """
Check if a type is equal to another.
Expand Down