Experiment: Add `core::cmp::smallest` and `core::cmp::largest` by bushrat011899 · Pull Request #160687 · rust-lang/rust

bushrat011899 · 2026-08-07T11:13:18Z

I did not use an LLM to create a change in this PR.
I used an LLM to create a change in this PR, and I have explained below how it was used.

Tracking Issue: #160728
Zulip: #t-libs-api/api-changes > variadic min/max

Description

Adds variadic alternatives to cmp::min and cmp::max which use #[rustc_splat] to allow anywhere from 1 to 12 arguments to be compared. Twelve chosen as an arbitrary limit. This doesn't have a pre-existing ACP or tracking issue (linking the splat issue since it's the most relevant), so I've added these two functions, smallest and largest under a new feature, cmp_splat.

This mostly exists to demonstrate a possible solution to the problem posited in the linked Zulip thread. Happy to close if undesirable, as my current focus is on no_std I/O.

rustbot · 2026-08-07T11:13:22Z

r? @nia-e

rustbot has assigned @nia-e.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: libs
libs expanded to 12 candidates
Random selection from JohnTitor, Mark-Simulacrum, clarfonthey, nia-e

nia-e

implementation itself looks great, some tiny nits only. ty oomfie

View changes since this review

rustbot · 2026-08-07T12:26:34Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

nia-e · 2026-08-08T01:12:12Z

looks good now ^^ I'll wait a couple days to give folks time to see this given there's no ACP, and if nothing comes up r=me

teor2345

Looks good, but one doc comment needs a tweak (or maybe the doctests are wrong and haven't been run?)

There aren't any tests for const calls or non-Copy types, but we already have splat UI tests for those. So that's not a blocker for this PR.

The place we are lacking testing for splat is incorrect argument suggestions. So if you see any weird behaviour there, please open a ticket/PR and cc me.

@rustbot label +F-splat

View changes since this review

bushrat011899 · 2026-08-10T04:10:03Z

Thanks for the feedback so far everyone! I've updated this PR as per discussion on the ACP, namely making TupleReduce a private implementation detail (and removing the type parameter in favour of an associated type).

nia-e · 2026-08-12T17:11:35Z

Main thing missing here would be some tests (especially for const behaviour), otherwise LGTM ^^

nia-e · 2026-08-12T23:17:00Z

r=me once you have tests for const behaviour - smoke tests should be fine, i don't see any particular nasty edge case here.
@bors delegate+

rust-bors · 2026-08-12T23:17:03Z

✌️ @bushrat011899, you can now approve this pull request!

If @nia-e told you to "r=me" after making some further change, then please make that change and post @bors r=nia-e.

View changes since this delegation.

teor2345 · 2026-08-12T23:49:05Z

i don't see any particular nasty edge case here.

There aren't any edge cases in splat, either, except for the empty tuple case (which produces a negative argument difference, has splat tests, and is excluded here by design).

The ~~smallest and largest~~ shortest and longest argument counts should be enough, and maybe a check it matches the behaviour of min/max.

Variadic alternatives to `min` and `max` using `splat`.

rustbot · 2026-08-13T01:40:29Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

bushrat011899 · 2026-08-13T01:50:46Z

r=me once you have tests for const behaviour - smoke tests should be fine, i don't see any particular nasty edge case here.

While adding tests, I ran into an interesting bug with the specific "hack" that core and alloc use to run tests. It appears that #[rustc_splat] creates a symbol that clashes when core gets linked twice, similar to how incoherent implementations work. I've worked around the bug by marking both splat functions as #[cfg(not(test))], which is very interesting, since the symbol is still available for use. To confirm that, I swapped min and max's internal implementation to just call smallest and largest respectively. Those functions still link and compile in #[cfg(test)], despite the splat functions nominally being cfg'ed out.

@teor2345 I'm not sure if you're aware of this quirk already, but you can confirm it yourself by removing the #[cfg(not(test))] attributes from smallest and/or largest and running ./x test library/core.

The ~~smallest and largest~~ shortest and longest argument counts should be enough, and maybe a check it matches the behaviour of min/max.

In happier news, smallest and largest are now API-compatible with min and max, and I can confirm they produce the same results, since replacing the inner implementation still passes all tests.

Anyway, this is probably quirky enough and last minute enough that I think a re-review might be warranted? I'm happy to merge, but better safe than sorry.

teor2345

Replacing the internals of stable min and max requires a perf run (and a check we have benchmarks for them). I'll ask for one.

Otherwise we're good here, thanks!

Looking forward to working out the weirdness around core tests, too, but in a separate ticket/PR.

View changes since this review

teor2345 · 2026-08-13T02:32:21Z

+#[unstable(feature = "cmp_splat", issue = "160728")]
+#[rustc_const_unstable(feature = "cmp_splat", issue = "160728")]
+#[expect(private_bounds, reason = "`SmallestArgs` is an internal implementation detail")]
+#[cfg(not(test))] // FIXME: splat interacts poorly with the double linking of `core` in tests


Non-blocking:

While adding tests, I ran into an interesting bug with the specific "hack" that core and alloc use to run tests. It appears that #[rustc_splat] creates a symbol that clashes when core gets linked twice, similar to how incoherent implementations work.

Huh, symbol clashes between splatted and un-splatted function types were fixed in #158890, and we have regression tests for incremental and non-incremental compilation:

splat-mangling-issue-158644.rs

splat-mangling.rs

This is a vaguely similar bug, but here, the function symbol should just be the path. Makes me wonder if type mangling is involved, or it could be something to do with const-time checks.

It would be good to open a ticket, we'll work it out from there.

This is not a blocker for merging.

Yeah my assumption is it has something to do with symbols since the "fix" is the same as what's used for incoherent implementations. The exact error was something along the lines of two crates (core and core) both being linked with identical hashes.

nia-e · 2026-08-13T02:59:33Z

@bors try @rust-timer queue

Experiment: Add `core::cmp::smallest` and `core::cmp::largest`

nia-e · 2026-08-13T03:03:42Z

Unless the perf run pulls up notable improvements, I'd be tempted to still say not to replace min/max yet, in large part since codegen here may be quite target-specific. Otherwise seems fine to me still; the symbol conflict is unfortunate but I think tolerable for an experiment

rust-bors · 2026-08-13T03:57:55Z

☀️ Try build successful (CI)
Build commit: 06cbba6 (06cbba631d5f10039ae2f189f8a56d4c958b1c2d)
Base parent: 52d0866 (52d08664805c8deb47b7e86999d7f8647b424a15)

rust-timer · 2026-08-13T04:40:49Z

Finished benchmarking commit (06cbba6): comparison URL.

Overall result: ✅ improvements - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.2%	[-0.3%, -0.1%]	2
All ❌✅ (primary)	-	-	0

Max RSS (memory usage)

Results (primary 1.0%, secondary 1.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	1.7%	[0.8%, 2.8%]	9
Regressions ❌ (secondary)	1.8%	[0.5%, 3.5%]	8
Improvements ✅ (primary)	-2.2%	[-3.1%, -1.4%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	1.0%	[-3.1%, 2.8%]	11

Cycles

Results (primary 2.3%, secondary 6.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.3%	[2.3%, 2.3%]	1
Regressions ❌ (secondary)	6.8%	[6.8%, 6.8%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.3%	[2.3%, 2.3%]	1

Binary size

Results (primary 0.1%, secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	0.1%	[0.0%, 0.3%]	18
Regressions ❌ (secondary)	0.1%	[0.0%, 0.3%]	14
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	0.1%	[0.0%, 0.3%]	18

Bootstrap: 459.695s -> 460.71s (0.22%)
Artifact size: 396.45 MiB -> 396.33 MiB (-0.03%)

bushrat011899 · 2026-08-13T04:46:26Z

Unless the perf run pulls up notable improvements...

How does 0.1% in 2 benchmarks sound?

...I'd be tempted to still say not to replace min/max yet, in large part since codegen here may be quite target-specific. Otherwise seems fine to me still; the symbol conflict is unfortunate but I think tolerable for an experiment

Agreed, I'll split off that commit for now. I've got a handful of specific tests included regardless, and I'm satisfied that my implementation matches the original.

nia-e · 2026-08-13T04:51:23Z

this is very much within what we expect for noise ^^ i'd keep them separate. but i'm happy to see this, it means we could consider doing this in the near future

teor2345 · 2026-08-13T05:06:21Z

I asked if we have runtime benchmarks for min/max on Zulip.

Notes for a later implementation replacement PR:

It's possible that we don't have any runtime benchmarks, or in the benchmarks that use them, it's a tiny amount of the overall run time. A quick local benchmark check would be good.

Alternatively, we could check the new assembly on Godbolt is similar to the current assembly.

nia-e · 2026-08-13T05:25:43Z

agreed, but not a blocker for this PR ^^ based on blocking concerns being resolved, i think this is best followed up in subsequent PRs.

@bors r+

rust-bors · 2026-08-13T05:25:45Z

📌 Commit f7eb24d has been approved by nia-e

It is now in the queue for this repository.

Rollup of 8 pull requests Successful merges: - #159593 (merge ambiguity errors that blame the same inference variable) - #160687 (Experiment: Add `core::cmp::smallest` and `core::cmp::largest`) - #160856 (Replace infers and non-rigid aliases with `Ty/Const::Error` if param env normalization fails) - #160961 (bootstrap: Overhaul matching of command-line selectors to steps) - #160975 (Remove target argument from get_proc_macros) - #161023 (bootstrap: Replace the `exit!` macro with a function `helpers::exit_process`) - #160932 (Make tidy::Version public) - #161029 (mailmap: Update my default email)

Rollup merge of #160687 - bushrat011899:experiment_splat_min_max, r=nia-e Experiment: Add `core::cmp::smallest` and `core::cmp::largest` Tracking Issue: #160728 Zulip: [#t-libs-api/api-changes > variadic min/max](https://rust-lang.zulipchat.com/#narrow/channel/327149-t-libs-api.2Fapi-changes/topic/variadic.20min.2Fmax/with/570428142) # Description Adds variadic alternatives to `cmp::min` and `cmp::max` which use `#[rustc_splat]` to allow anywhere from 1 to 12 arguments to be compared. Twelve chosen as an arbitrary limit. This doesn't have a pre-existing ACP or tracking issue (linking the `splat` issue since it's the most relevant), so I've added these two functions, `smallest` and `largest` under a new feature, `cmp_splat`. This mostly exists to demonstrate a possible solution to the problem posited in the linked Zulip thread. Happy to close if undesirable, as my current focus is on `no_std` I/O.

scottmcm · 2026-08-13T18:39:42Z

Thank you for not moving cmp::min and friends to call this yet.

One thing I would suggest as part of whatever PR does that later: write a mir-opt test to show that at the rust level it folds down to the same thing. Even if LLVM can remove it, we don't want to end up with giant MIR from using simple things like cmp::min::<usize>.

(Note how the version in that perf run made a bunch of debug binaries bigger, for example. Of course it's debug so that might be fine, but it'd be worth thinking about.)

Add const-compatible `cmp_splat` test

rustbot assigned nia-e Aug 7, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 7, 2026

nia-e requested changes Aug 7, 2026

View reviewed changes

Comment thread library/core/src/cmp.rs Outdated

Comment thread library/core/src/cmp.rs Outdated

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 7, 2026

rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 7, 2026

nia-e added the needs-acp This change is blocked on the author creating an ACP. label Aug 7, 2026

Urgau reviewed Aug 7, 2026

View reviewed changes

Comment thread library/core/src/cmp.rs Outdated

scottmcm reviewed Aug 7, 2026

View reviewed changes

Comment thread library/core/src/cmp.rs

bushrat011899 mentioned this pull request Aug 7, 2026

Tracking Issue for cmp_splat (Variadic min and max) #160728

Open

5 tasks

bushrat011899 force-pushed the experiment_splat_min_max branch from 0ca7197 to 9a53733 Compare August 7, 2026 23:33

bushrat011899 mentioned this pull request Aug 8, 2026

ACP: Variadic core::cmp::{min, max} rust-lang/libs-team#848

Closed

teor2345 suggested changes Aug 10, 2026

View reviewed changes

Comment thread library/core/src/cmp.rs Outdated

rustbot added the F-splat `#![feature(splat)]` https://github.com/rust-lang/rust/issues/153629 label Aug 10, 2026

nia-e removed the needs-acp This change is blocked on the author creating an ACP. label Aug 12, 2026

bushrat011899 added 2 commits August 13, 2026 11:13

Implement core::cmp::{smallest, largest}

a693668

Variadic alternatives to `min` and `max` using `splat`.

Add const-compatible cmp_splat test

f7eb24d

bushrat011899 force-pushed the experiment_splat_min_max branch from 6ed1781 to 7f255a8 Compare August 13, 2026 01:40

bushrat011899 requested a review from teor2345 August 13, 2026 01:52

teor2345 suggested changes Aug 13, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 13, 2026

rust-bors Bot pushed a commit that referenced this pull request Aug 13, 2026

Auto merge of #160687 - bushrat011899:experiment_splat_min_max, r=<try>

06cbba6

Experiment: Add `core::cmp::smallest` and `core::cmp::largest`

This comment has been minimized.

Sign in to view

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Aug 13, 2026

bushrat011899 force-pushed the experiment_splat_min_max branch from 7f255a8 to f7eb24d Compare August 13, 2026 04:48

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 13, 2026

jhpratt mentioned this pull request Aug 13, 2026

Rollup of 8 pull requests #161031

Merged

rust-bors Bot merged commit 41c3350 into rust-lang:main Aug 13, 2026
13 checks passed

rustbot added this to the 1.99.0 milestone Aug 13, 2026

rust-bors Bot pushed a commit that referenced this pull request Aug 14, 2026

Unrolled build for #160687

52027e0

Add const-compatible `cmp_splat` test

Uh oh!

Uh oh!

Conversation

bushrat011899 commented Aug 7, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

rustbot commented Aug 7, 2026

Uh oh!

nia-e left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rustbot commented Aug 7, 2026

Uh oh!

Uh oh!

Uh oh!

nia-e commented Aug 8, 2026

Uh oh!

teor2345 left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bushrat011899 commented Aug 10, 2026

Uh oh!

nia-e commented Aug 12, 2026

Uh oh!

nia-e commented Aug 12, 2026

Uh oh!

rust-bors Bot commented Aug 12, 2026

Uh oh!

teor2345 commented Aug 12, 2026

Uh oh!

rustbot commented Aug 13, 2026

Uh oh!

bushrat011899 commented Aug 13, 2026

Uh oh!

teor2345 left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

teor2345 Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

bushrat011899 Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

nia-e commented Aug 13, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

nia-e commented Aug 13, 2026

Uh oh!

rust-bors Bot commented Aug 13, 2026

Uh oh!

This comment has been minimized.

rust-timer commented Aug 13, 2026

Overall result: ✅ improvements - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Uh oh!

bushrat011899 commented Aug 13, 2026

Uh oh!

nia-e commented Aug 13, 2026

Uh oh!

teor2345 commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nia-e commented Aug 13, 2026

Uh oh!

rust-bors Bot commented Aug 13, 2026

Uh oh!

Uh oh!

scottmcm commented Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

bushrat011899 commented Aug 7, 2026 •

edited by rustbot

Loading

nia-e left a comment •

edited by rustbot

Loading

teor2345 left a comment •

edited by rustbot

Loading

teor2345 left a comment •

edited by rustbot

Loading

teor2345 commented Aug 13, 2026 •

edited

Loading

scottmcm commented Aug 13, 2026 •

edited

Loading