Implement fast path for `derive(PartialOrd)` when deriving `Ord` by makai410 · Pull Request #155598 · rust-lang/rust

makai410 · 2026-04-21T14:48:26Z

Unfortunately, this PR shares the same issue with #124794, which doesn't work when derive(PartialOrd) is before derive(Ord).

rustbot · 2026-04-21T14:48:29Z

Changes to the code generated for builtin derived traits.

cc @nnethercote

rustbot · 2026-04-21T14:48:32Z

r? @ShoyuVanilla

rustbot has assigned @ShoyuVanilla.
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: compiler
compiler expanded to 72 candidates
Random selection from 18 candidates

makai410 · 2026-04-21T14:49:24Z

-                    cx.dcx().span_bug(field.span, "not exactly 2 arguments in `derive(Ord)`");
+                    cx.dcx()
+                        .span_bug(field.span, "not exactly 2 arguments in `derive(PartialOrd)`");
                };


I think this is probably a copy-paste curse, so I changed that, not 100% sure though.

View changes since the review

nnethercote · 2026-04-21T22:43:00Z

I can review this: r? @nnethercote

cc @theemathas, who has made good comments on similar PRs in the past.

makai410 · 2026-04-22T08:37:37Z

@bors try @rust-timer queue

Implement fast path for `derive(PartialOrd)` when deriving `Ord`

petrochenkov · 2026-04-22T11:34:26Z

    resolutions: Vec<DeriveResolution>,
    helper_attrs: Vec<(usize, IdentKey, Span)>,
+    // if this list keeps getting extended, we could use `bitflags`,
+    // something like what [`rustc_type_ir::flags::TypeFlags`] is doing.


We had multiple flags here in the past, and they indeed used flags, and one map in the resolver instead of two containers_deriving_(copy,ord) maps.

View changes since the review

Remove PartialEq and Eq from the SpecialDerives. #65892

Thanks for providing the context. I was thinking about doing one more round of scanning derives to collect all flags such that it might solve #124794. I don't know if that had been discussed before, and if it did I'd really want to know the reason of why not doing that.

one more round of scanning derives to collect all flags

If I correctly understand what you are talking about, it's a much worse hack and it cannot work correctly.
We already doing something like that to support deprecation lint legacy_derive_helpers, and that logic is going to be removed soon.

petrochenkov · 2026-04-22T11:37:14Z

Similarly to the existing derive(Clone, Copy) I'd classify this as adding hacks to the expansion infra, and would recommend against this by default, unless there are really strong performance reasons.

Moreover, the Copy hack can be in theory removed in favor of some MIR transform, but for this one it would be harder to do.

rust-bors · 2026-04-22T14:46:45Z

💔 Test for f5daf9f failed: CI

makai410 · 2026-04-23T05:24:55Z

@bors try

Implement fast path for `derive(PartialOrd)` when deriving `Ord`

rust-bors · 2026-04-23T07:33:41Z

☀️ Try build successful (CI)
Build commit: 803fd19 (803fd197cdeec49615d500f71f89872b51cfc6a9, parent: 30837cb66de032fbc8072c8ecbf76c39c5b14478)

makai410 · 2026-05-13T11:51:42Z

But anyway, I don't think the current way is that bad, we just want to know if there is any other #[derive(Copy/Ord)] here during resolution for better generated code...

rustbot · 2026-05-21T09:59:05Z

Changes to the code generated for builtin derived traits.

cc @nnethercote

rustbot · 2026-05-21T10:01:07Z

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.

nnethercote · 2026-05-21T14:31:43Z

@bors r+

rust-bors · 2026-05-21T14:31:47Z

📌 Commit 911c27d has been approved by nnethercote

It is now in the queue for this repository.

Implement fast path for `derive(PartialOrd)` when deriving `Ord` Closes: #155537 Unfortunately, this PR shares the same issue with #124794, which doesn't work when `derive(PartialOrd)` is before `derive(Ord)`.

rust-bors · 2026-05-21T17:40:31Z

💔 Test for 3f43613 failed: CI. Failed job:

auto - test-various (web logs, enhanced plaintext logs)

… enums

nnethercote · 2026-05-23T20:27:51Z

@bors r+

rust-bors · 2026-05-23T20:27:53Z

📌 Commit 766428b has been approved by nnethercote

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

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

rustbot assigned ShoyuVanilla Apr 21, 2026

makai410 commented Apr 21, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot assigned nnethercote and unassigned ShoyuVanilla Apr 21, 2026

makai410 force-pushed the partial-ord branch from 159a353 to c15a04c Compare April 22, 2026 01:58

theemathas reviewed Apr 22, 2026

View reviewed changes

Comment thread compiler/rustc_builtin_macros/src/deriving/cmp/partial_ord.rs

This comment has been minimized.

Sign in to view

theemathas mentioned this pull request Apr 22, 2026

decl_macro can call pub fn methods of private types. #155631

Open

theemathas reviewed Apr 22, 2026

View reviewed changes

Comment thread tests/ui/deriving/deriving-all-codegen.stdout Outdated

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 Apr 22, 2026

rust-bors Bot pushed a commit that referenced this pull request Apr 22, 2026

Auto merge of #155598 - makai410:partial-ord, r=<try>

f5daf9f

Implement fast path for `derive(PartialOrd)` when deriving `Ord`

petrochenkov reviewed Apr 22, 2026

View reviewed changes

rust-bors Bot 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 Apr 22, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot pushed a commit that referenced this pull request Apr 23, 2026

Auto merge of #155598 - makai410:partial-ord, r=<try>

803fd19

Implement fast path for `derive(PartialOrd)` when deriving `Ord`

This comment has been minimized.

Sign in to view

makai410 force-pushed the partial-ord branch from 2fd68fb to 3d9d69d Compare May 21, 2026 09:59

This comment has been minimized.

Sign in to view

makai410 added 4 commits May 21, 2026 17:59

Implement fast path for derive(PartialOrd)

c9a560b

Use UFCS for Ord::cmp

802938c

Avoid regression in derive(PartialOrd) for unit structs

d436194

Avoid regression in derive(PartialOrd) for zero-variant enums

caabc4b

makai410 force-pushed the partial-ord branch from 3d9d69d to 752ee41 Compare May 21, 2026 10:01

This comment has been minimized.

Sign in to view

makai410 force-pushed the partial-ord branch from 752ee41 to 911c27d Compare May 21, 2026 10:38

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 May 21, 2026

This comment has been minimized.

Sign in to view

rust-bors Bot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 21, 2026

Avoid regression in derive(PartialOrd) for single fieldless variant…

766428b

… enums

makai410 force-pushed the partial-ord branch from 911c27d to 766428b Compare May 23, 2026 12:33

rust-bors Bot 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 May 23, 2026

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 May 23, 2026

Uh oh!

Conversation

makai410 commented Apr 21, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Apr 21, 2026

Uh oh!

rustbot commented Apr 21, 2026

Uh oh!

makai410 Apr 21, 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!

This comment has been minimized.

nnethercote commented Apr 21, 2026

Uh oh!

Uh oh!

This comment has been minimized.

Uh oh!

makai410 commented Apr 22, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

petrochenkov Apr 22, 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!

petrochenkov Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

makai410 Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

petrochenkov Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

petrochenkov commented Apr 22, 2026

Uh oh!

rust-bors Bot commented Apr 22, 2026

Uh oh!

makai410 commented Apr 23, 2026

Uh oh!

This comment has been minimized.

rust-bors Bot commented Apr 23, 2026

Uh oh!

This comment has been minimized.

makai410 commented May 13, 2026

Uh oh!

rustbot commented May 21, 2026

Uh oh!

This comment has been minimized.

rustbot commented May 21, 2026

Uh oh!

This comment has been minimized.

nnethercote commented May 21, 2026

Uh oh!

rust-bors Bot commented May 21, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

rust-bors Bot commented May 21, 2026

Uh oh!

nnethercote commented May 23, 2026

Uh oh!

rust-bors Bot commented May 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

makai410 commented Apr 21, 2026 •

edited by rustbot

Loading

makai410 Apr 21, 2026 •

edited by rustbot

Loading

petrochenkov Apr 22, 2026 •

edited by rustbot

Loading