Skip to content

Fix Enum.min/2,3 and Enum.max/2,3 with a custom sorter on ranges - #15603

Merged
josevalim merged 1 commit into
elixir-lang:mainfrom
pnezis:fix-enum-min-max-range-sorter
Jul 14, 2026
Merged

Fix Enum.min/2,3 and Enum.max/2,3 with a custom sorter on ranges#15603
josevalim merged 1 commit into
elixir-lang:mainfrom
pnezis:fix-enum-min-max-range-sorter

Conversation

@pnezis

@pnezis pnezis commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

The range clause of aggregate/3 only compared the two endpoints of the range, which is correct solely for sorters consistent with the natural integer order. Any other comparator returned a different result than the identical call on the equivalent list or stream:

sorter = fn a, b -> rem(a, 5) >= rem(b, 5) end
Enum.max(1..10, sorter)                         #=> 1, expected 4
Enum.max(Enum.to_list(1..10), sorter) #=> 4

Enum.min(-3..3, fn a, b -> abs(a) <= abs(b) end) #=> -3, expected 0

Only take the endpoint shortcut when the sorter is the default one (&>=/2 for max, &<=/2 for min); any custom sorter now reduces over the range elements like lists and streams do.

I am evaluating claude fable on code review tasks and this was one of the findings

Comment thread lib/elixir/lib/enum.ex Outdated
The range clause of `aggregate/3` only compared the two endpoints of the
range, which is correct solely for sorters consistent with the natural
integer order. Any other comparator returned a different result than
the identical call on the equivalent list or stream:

    sorter = fn a, b -> rem(a, 5) >= rem(b, 5) end
    Enum.max(1..10, sorter)               #=> 1, expected 4
    Enum.max(Enum.to_list(1..10), sorter) #=> 4

Once the range is known to be non-empty, take the endpoint shortcut
only when the sorter is one of the defaults (`&>=/2` or `&<=/2`, both
order-consistent); any other sorter traverses the elements through
`reduce_range/5`, seeded with the first element.
@pnezis
pnezis force-pushed the fix-enum-min-max-range-sorter branch from 93e10a8 to b70b46f Compare July 14, 2026 09:03
@josevalim
josevalim merged commit 92c3b9f into elixir-lang:main Jul 14, 2026
15 checks passed
@josevalim

Copy link
Copy Markdown
Member

💚 💙 💜 💛 ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants