`Vec::dedup_by` docs explicit function argument order by m4dh0rs3 · Pull Request #157995 · rust-lang/rust

m4dh0rs3 · 2026-06-16T20:56:48Z

Clarify and expand Vec::dedup_by and slice::partition_dedup_by docs.

rustbot · 2026-06-16T20:56:53Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

@rustbot author: the review is finished, PR author should check the comments and take action accordingly
@rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: libs
libs expanded to 11 candidates
Random selection from 6 candidates

Mark-Simulacrum

Mostly extra extensions - generally looks OK. Thanks!

View changes since this review

Mark-Simulacrum · 2026-06-21T19:07:35Z

+    /// If it returns `true`, the `current` element is removed from the vector.
    ///
-    /// If the vector is sorted, this removes all duplicates.
+    /// The element `prev` occurs *before* `current` in the vector (`[.., prev, current, ..]`),


Suggested change

/// The element `prev` occurs *before* `current` in the vector (`[.., prev, current, ..]`),

/// The element `prev` occurs *before* `current` in the vector (`[.., prev, .., current, ..]`),

It's not obvious to me whether / why we make the ordering guarantee, but I think it's probably clearer if we don't imply prev is directly before current? It's definitely not the case if you view this as the 'original' slice -- we batch move elements back into place, so there's dropped elements between prev and current.

Yeah, I misread the source-code. The first part checks whether a duplicate element exists, and elements are in consecutive order there. But that's not true for the actual de-duplication part later on.

For me its not about the ordering guarantee, but mentioning that the order (of the current implementation) is unexpected. And since that ordering does exist, I think we now should also guarantee it. Any future re-implementation of dedup_by changing the order would break a lot of code.

I think the ordering is guaranteed already:

The elements are passed in opposite order from their order in the slice

even if it's a bit more vaguely worded, I think this is the same guarantee you are making, right?

(happy to take a wording improvement but I'm not seeing a new guarantee here)

Yes, its only a wording improvement.

Mark-Simulacrum · 2026-06-21T19:09:00Z

+    /// however, the order (ascending vs. descending) can matter.
+    ///
+    /// Both references passed to `same_bucket` are mutable.
+    /// This allows merging elements by mutating `prev` and returning `true`.


Are there simple examples for both the 'complicated predicates' and the 'merging' discussed here that we could add? It seems plausibly useful to show how that might look. Definitely the current example given seems like it would be better to stay as a, b arguments -- current, prev is just adding noise there.

The reason I changed a, b in general was that (for me) they imply the order [.., a, b, ..], which is not the case. If you mean just change the naming in the example, then at least use maybe x, p or something.

Maybe I can think of a simple example of complicated predicates and merging yet. The example I came across in my personal code was sorting Vec<Range<usize>> by .start and then merging intervals that were subsets of another.

rustbot · 2026-06-21T19:09:39Z

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

m4dh0rs3 · 2026-06-21T21:56:11Z

I added a non-trivial example which depends on the sorting order and demonstrates how merging could look like. I also shortened the variable names.

@rustbot ready

m4dh0rs3 · 2026-06-22T11:58:17Z

Because they are essentially the same docs, I wanted to link from slice::partition_dedup_by to Vec::dedup_by. Looking at core/src/time.rs, the doc syntax /// [Instant]: ../../std/time/struct.Instant.html to link to an item in std should work, but did not for me. The linkchecker complains, but the link works for me in my browser for local docs. So now I just left the docs somewhat duplicated instead.

@rustbot ready

Mark-Simulacrum · 2026-06-28T21:30:51Z

-    /// must determine if the elements compare equal. The elements are passed in opposite order
-    /// from their order in the slice, so if `same_bucket(a, b)` returns `true`, `a` is moved
-    /// at the end of the slice.
+    /// The predicate `same_bucket(p, x)` is passed references to two elements from


Hm, so this flips the order of p and x compared to the other function and we're not documenting (pre-existing, probably good to keep for this PR at least) which of the two is moved if the comparison returns they're equal.

Is it intentional to flip the order? It looks like we actually have different ordering of arguments between this and the (stable) dedup_by, I'm wondering if that's intentional... it might be that we should change it to be consistent. Added this as an unresolved question to the tracking issue.

View changes since the review

Its not intentional and i fixed the doc in d253640. I do believe the slice::partition_dedup_by implementation ordering is consistent with Vec::dedup_by.

Mark-Simulacrum · 2026-06-28T21:32:32Z

@bors squash msg="improve Vec::dedup_by documentation"

rust-bors · 2026-06-28T21:32:52Z

🔨 8 commits were squashed into 281a2ba.

Mark-Simulacrum · 2026-07-04T15:34:14Z

r=me with commits squashed

@rustbot author

m4dh0rs3 · 2026-07-06T08:45:03Z

@bors squash msg="Vec::dedup_by docs explicit function argument order"

@rustbot ready

Co-authored-by: Mark Rousskov <mark.simulacrum@gmail.com>

rust-bors · 2026-07-06T08:45:26Z

🔨 2 commits were squashed into a29353b.

Mark-Simulacrum · 2026-07-09T00:22:17Z

@bors r+ rollup

rust-bors · 2026-07-09T00:22:20Z

📌 Commit a29353b has been approved by Mark-Simulacrum

It is now in the queue for this repository.

`Vec::dedup_by` docs explicit function argument order Clarify and expand `Vec::dedup_by` and `slice::partition_dedup_by` docs. Fixes rust-lang#157959

Rollup of 23 pull requests Successful merges: - #158968 (stdarch subtree update) - #154445 (rustdoc: Represent `--output-format=json` coverage and ir differently) - #158495 (Rename HAS_CT_PROJECTION to HAS_CONST_ALIAS) - #158666 (Carry the `b_offset` inside `BackendRepr::ScalarPair`) - #158870 (std: merge the unix-like io::error modules into one file) - #158920 (Update wasm-component-ld to 0.5.26) - #158926 (wrapping_sh* methods: clarify underspecified reference) - #158927 (add core test run with `-Zforce-intrinsic-fallback`) - #158932 (Do not build the compiler when invoking `x perf compare`) - #158937 (Emit the emscripten entry point as `__main_argc_argv`) - #151379 (Stabilize `VecDeque::retain_back` from `truncate_front`) - #156144 (Better docs for PartialEq (includes macro rename)) - #156548 ( Library support for aarch64-unknown-linux-pauthtest target) - #157995 (`Vec::dedup_by` docs explicit function argument order) - #158307 (CI job for parallel frontend ui tests) - #158741 (Simplify `Option::into_flat_iter` signature) - #158807 (Add regression test for CString::clone_into unwind safety) - #158862 (Fix the span for parameter suggestion ) - #158894 (Make the ordering of non-terminal binds in ambiguity error messages deterministic) - #158902 (add codegen test for range length bound propagation) - #158913 (Update `browser-ui-test` version to `0.24.1`) - #158935 (std: support real fd methods on Emscripten) - #158978 (Add regression test for too-big by-value ABI args)

Rollup merge of #157995 - m4dh0rs3:main, r=Mark-Simulacrum `Vec::dedup_by` docs explicit function argument order Clarify and expand `Vec::dedup_by` and `slice::partition_dedup_by` docs. Fixes #157959

Rollup of 23 pull requests Successful merges: - rust-lang/rust#158968 (stdarch subtree update) - rust-lang/rust#154445 (rustdoc: Represent `--output-format=json` coverage and ir differently) - rust-lang/rust#158495 (Rename HAS_CT_PROJECTION to HAS_CONST_ALIAS) - rust-lang/rust#158666 (Carry the `b_offset` inside `BackendRepr::ScalarPair`) - rust-lang/rust#158870 (std: merge the unix-like io::error modules into one file) - rust-lang/rust#158920 (Update wasm-component-ld to 0.5.26) - rust-lang/rust#158926 (wrapping_sh* methods: clarify underspecified reference) - rust-lang/rust#158927 (add core test run with `-Zforce-intrinsic-fallback`) - rust-lang/rust#158932 (Do not build the compiler when invoking `x perf compare`) - rust-lang/rust#158937 (Emit the emscripten entry point as `__main_argc_argv`) - rust-lang/rust#151379 (Stabilize `VecDeque::retain_back` from `truncate_front`) - rust-lang/rust#156144 (Better docs for PartialEq (includes macro rename)) - rust-lang/rust#156548 ( Library support for aarch64-unknown-linux-pauthtest target) - rust-lang/rust#157995 (`Vec::dedup_by` docs explicit function argument order) - rust-lang/rust#158307 (CI job for parallel frontend ui tests) - rust-lang/rust#158741 (Simplify `Option::into_flat_iter` signature) - rust-lang/rust#158807 (Add regression test for CString::clone_into unwind safety) - rust-lang/rust#158862 (Fix the span for parameter suggestion ) - rust-lang/rust#158894 (Make the ordering of non-terminal binds in ambiguity error messages deterministic) - rust-lang/rust#158902 (add codegen test for range length bound propagation) - rust-lang/rust#158913 (Update `browser-ui-test` version to `0.24.1`) - rust-lang/rust#158935 (std: support real fd methods on Emscripten) - rust-lang/rust#158978 (Add regression test for too-big by-value ABI args)

rustbot assigned Mark-Simulacrum Jun 16, 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 Jun 16, 2026

Mark-Simulacrum requested changes Jun 21, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 21, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 21, 2026

This comment has been minimized.

Sign in to view

Mark-Simulacrum approved these changes Jun 28, 2026

View reviewed changes

Mark-Simulacrum mentioned this pull request Jun 28, 2026

Tracking issue for slice::partition_dedup/by/by_key #54279

Open

5 tasks

This comment has been minimized.

Sign in to view

rust-bors Bot force-pushed the main branch from c7aa3a7 to 281a2ba Compare June 28, 2026 21:32

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 4, 2026

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

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 6, 2026

Vec::dedup_by docs explicit function argument order

a29353b

Co-authored-by: Mark Rousskov <mark.simulacrum@gmail.com>

rust-bors Bot force-pushed the main branch from d253640 to a29353b Compare July 6, 2026 08:45

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-review Status: Awaiting review from the assignee but also interested parties. labels Jul 9, 2026

This was referenced Jul 9, 2026

Rollup of 25 pull requests #158989

Closed

Rollup of 23 pull requests #158991

Merged

rust-bors Bot merged commit bba5dff into rust-lang:main Jul 9, 2026
13 checks passed

rustbot added this to the 1.99.0 milestone Jul 9, 2026

	/// The element `prev` occurs before `current` in the vector (`[.., prev, current, ..]`),
	/// The element `prev` occurs before `current` in the vector (`[.., prev, .., current, ..]`),

Uh oh!

Uh oh!

Conversation

m4dh0rs3 commented Jun 16, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Jun 16, 2026

Uh oh!

Mark-Simulacrum 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!

Mark-Simulacrum Jun 21, 2026

Choose a reason for hiding this comment

Uh oh!

m4dh0rs3 Jun 21, 2026

Choose a reason for hiding this comment

Uh oh!

Mark-Simulacrum Jun 21, 2026

Choose a reason for hiding this comment

Uh oh!

m4dh0rs3 Jun 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mark-Simulacrum Jun 21, 2026

Choose a reason for hiding this comment

Uh oh!

m4dh0rs3 Jun 21, 2026

Choose a reason for hiding this comment

Uh oh!

m4dh0rs3 Jun 21, 2026

Choose a reason for hiding this comment

Uh oh!

rustbot commented Jun 21, 2026

Uh oh!

m4dh0rs3 commented Jun 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

m4dh0rs3 commented Jun 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mark-Simulacrum Jun 28, 2026 • 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!

m4dh0rs3 Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

Mark-Simulacrum commented Jun 28, 2026

Uh oh!

This comment has been minimized.

rust-bors Bot commented Jun 28, 2026

Uh oh!

Mark-Simulacrum commented Jul 4, 2026

Uh oh!

m4dh0rs3 commented Jul 6, 2026

Uh oh!

This comment has been minimized.

rust-bors Bot commented Jul 6, 2026

Uh oh!

Mark-Simulacrum commented Jul 9, 2026

Uh oh!

rust-bors Bot commented Jul 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

m4dh0rs3 commented Jun 16, 2026 •

edited by rustbot

Loading

Mark-Simulacrum left a comment •

edited by rustbot

Loading

m4dh0rs3 Jun 21, 2026 •

edited

Loading

m4dh0rs3 commented Jun 21, 2026 •

edited

Loading

m4dh0rs3 commented Jun 22, 2026 •

edited

Loading

Mark-Simulacrum Jun 28, 2026 •

edited by rustbot

Loading