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)

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.

rust-bors · 2026-05-24T03:16:00Z

☀️ Test successful - CI
Approved by: nnethercote
Duration: 3h 9m 29s
Pushing dd8b2d6 to main...

github-actions · 2026-05-24T03:19:24Z

What is this?

This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 23a3312 (parent) -> dd8b2d6 (this PR)

Test differences

Show 2 test diffs

2 doctest diffs were found. These are ignored, as they are noisy.

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard dd8b2d6162c5616d7030238c29f892a431093cdd --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

x86_64-gnu-llvm-22-2: 1h -> 1h 36m (+57.6%)
x86_64-gnu-llvm-21-2: 1h 36m -> 1h 2m (-34.7%)
dist-ohos-armv7: 1h 12m -> 48m 20s (-33.2%)
x86_64-gnu-llvm-22-3: 1h 30m -> 1h 56m (+28.9%)
x86_64-msvc-ext2: 1h 30m -> 1h 52m (+25.2%)
x86_64-mingw-2: 2h -> 2h 29m (+24.9%)
dist-x86_64-freebsd: 1h 8m -> 1h 25m (+24.3%)
dist-i686-mingw: 2h 41m -> 2h 7m (-20.9%)
dist-aarch64-llvm-mingw: 1h 36m -> 1h 49m (+13.7%)
x86_64-gnu-llvm-22-1: 1h 8m -> 1h 15m (+9.9%)

How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

rust-timer · 2026-05-24T03:58:58Z

Finished benchmarking commit (dd8b2d6): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

If the regression was expected or you think it can be justified,
please write a comment with sufficient written justification, and add
@rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
If you think that you know of a way to resolve the regression, try to create
a new PR with a fix for the regression.
If you do not understand the regression or you think that it is just noise,
you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
were already notified of this PR).

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

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.7%	[0.5%, 0.8%]	3
Regressions ❌ (secondary)	0.7%	[0.2%, 1.0%]	9
Improvements ✅ (primary)	-0.3%	[-0.5%, -0.2%]	20
Improvements ✅ (secondary)	-0.3%	[-0.5%, -0.2%]	5
All ❌✅ (primary)	-0.2%	[-0.5%, 0.8%]	23

Max RSS (memory usage)

Results (primary -2.3%, secondary 7.0%)

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

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	7.0%	[5.4%, 8.6%]	2
Improvements ✅ (primary)	-2.3%	[-4.0%, -0.6%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.3%	[-4.0%, -0.6%]	2

Cycles

Results (primary -2.2%, secondary 3.7%)

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

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	4.7%	[3.7%, 5.7%]	7
Improvements ✅ (primary)	-2.2%	[-2.2%, -2.2%]	1
Improvements ✅ (secondary)	-3.4%	[-3.4%, -3.4%]	1
All ❌✅ (primary)	-2.2%	[-2.2%, -2.2%]	1

Binary size

Results (primary -0.2%, 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.5%	[0.1%, 0.9%]	2
Regressions ❌ (secondary)	0.1%	[0.1%, 0.1%]	1
Improvements ✅ (primary)	-0.3%	[-1.0%, -0.0%]	9
Improvements ✅ (secondary)	-0.1%	[-0.4%, -0.0%]	16
All ❌✅ (primary)	-0.2%	[-1.0%, 0.9%]	11

Bootstrap: 511.109s -> 511.311s (0.04%)
Artifact size: 400.56 MiB -> 400.72 MiB (0.04%)

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