impl !PartialOrd for HirId by oli-obk · Pull Request #138610 · rust-lang/rust

oli-obk · 2025-03-17T14:54:25Z

revive of #92233

Another checkbox of #90317, another small step in making incremental less likely to die in horrible ways

rustbot · 2025-03-17T14:54:29Z

rustbot has assigned @fmease.
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

rustbot · 2025-03-17T14:54:31Z

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

compiler-errors

just nits, maybe i'm missing something about the things i asked

compiler-errors · 2025-03-17T14:58:14Z

compiler/rustc_driver_impl/src/lib.rs

    fn sort_lints(sess: &Session, mut lints: Vec<&'static Lint>) -> Vec<&'static Lint> {
        // The sort doesn't case-fold but it's doubtful we care.
-        lints.sort_by_cached_key(|x: &&Lint| (x.default_level(sess.edition()), x.name));
+        lints.sort_by(|a, b| {


Why did this change? Isn't this doing the same exact thing as previosuly? (Sorting by a key of default_level + name?)

lack of Ord impl

Wait, why does it only have a partial ordering?

Like, if the reason is because of those skip'd arguments in the derive, then if I'm not mistaken then even the PartialOrd impl violates the guarantee that

a == b if and only if partial_cmp(a, b) == Some(Equal)

i.e. it is inconsistent w/ PartialEq.

Yeah, actually the more I think about this, it seems to me to that LintLevel now has and invalid impl of partialord 🤔

I had a manual impl before that first did an equality check. But I have been considering instead to remove the LintExpectationId fields and store them elsewhere. It's a bit involved, so I'm gonna try landing that on its own on master

Yea that worked out nicely, just tripled the size of the PR ^^

compiler/rustc_hir_typeck/src/upvar.rs

compiler-errors · 2025-03-17T15:01:44Z

compiler/rustc_passes/src/dead.rs

            return;
        }
-        dead_codes.sort_by_key(|v| v.level);
+        dead_codes.sort_by(|a, b| a.level.partial_cmp(&b.level).unwrap());


sort_by_key(|a| a.level)?

Level isn't Ord anymore

compiler-errors · 2025-03-17T15:05:26Z

compiler/rustc_mir_build/src/builder/mod.rs

                };
+                let upvar_base = upvar_owner.get_or_insert(var_id.owner);
+                assert_eq!(*upvar_base, var_id.owner);
+                let var_id = var_id.local_id;


can you inline this into its usage?

there are no early returns, so I thought it good to do the check and the conversion as early as possible

compiler/rustc_lint_defs/src/lib.rs

rustbot · 2025-03-17T15:31:02Z

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

rustbot · 2025-03-19T11:42:27Z

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

Some changes occurred in exhaustiveness checking

cc @Nadrieril

Some changes occurred in match checking

cc @Nadrieril

compiler-errors

r=me after question, since I think my previous concern still applies here w/ this sort_by_key -> sort_by change.

compiler/rustc_passes/src/dead.rs

oli-obk · 2025-04-02T15:54:09Z

@bors r=compiler-errors rollup

bors · 2025-04-02T15:54:11Z

📌 Commit 7103ce7 has been approved by compiler-errors

It is now in the queue for this repository.

…r-errors impl !PartialOrd for HirId revive of rust-lang#92233 Another checkbox of rust-lang#90317, another small step in making incremental less likely to die in horrible ways

matthiaskrgr · 2025-04-02T18:18:30Z

may need rebase after #139269 lands

…r-errors impl !PartialOrd for HirId revive of rust-lang#92233 Another checkbox of rust-lang#90317, another small step in making incremental less likely to die in horrible ways

matthiaskrgr · 2025-04-03T05:38:26Z

@bors r-

m-ou-se · 2025-04-03T05:42:44Z

Looks like this PR was the cause of a failure in this rollup: #139300 (comment)

oli-obk · 2025-04-03T09:23:00Z

@bors r=compiler-errors rollup

bors · 2025-04-03T09:23:03Z

📌 Commit 57c4ab7 has been approved by compiler-errors

It is now in the queue for this repository.

Kobzol · 2025-04-03T20:25:52Z

The impl changes don't look trivial, maybe this could use a perf. run or at least be marked as iffy/maybe? 🤔

Edit: nevermind, the rollup this was in was a perf. win.

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#138017 (Tighten up assignment operator representations.) - rust-lang#138462 (Dedup `&mut *` reborrow suggestion in loops) - rust-lang#138610 (impl !PartialOrd for HirId) - rust-lang#138767 (Allow boolean literals in `check-cfg`) - rust-lang#139068 (io: Avoid marking some bytes as uninit) - rust-lang#139255 (Remove unused variables generated in merged doctests) - rust-lang#139270 (Add a mailmap entry for myself) - rust-lang#139303 (Put Noratrieb on vacation) - rust-lang#139312 (add Marco Ieni to mailmap) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#138610 - oli-obk:no-sort-hir-ids, r=compiler-errors impl !PartialOrd for HirId revive of rust-lang#92233 Another checkbox of rust-lang#90317, another small step in making incremental less likely to die in horrible ways

…r-errors impl !PartialOrd for HirId revive of rust-lang#92233 Another checkbox of rust-lang#90317, another small step in making incremental less likely to die in horrible ways

rustbot assigned fmease Mar 17, 2025

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 Mar 17, 2025

compiler-errors suggested changes Mar 17, 2025

View reviewed changes

compiler-errors assigned compiler-errors and unassigned fmease Mar 17, 2025

This comment has been minimized.

Sign in to view

oli-obk force-pushed the no-sort-hir-ids branch from 3da287a to 84d7916 Compare March 17, 2025 15:31

oli-obk force-pushed the no-sort-hir-ids branch from 84d7916 to 0dde879 Compare March 19, 2025 11:42

compiler-errors suggested changes Apr 1, 2025

View reviewed changes

compiler/rustc_passes/src/dead.rs Outdated Show resolved Hide resolved

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 Apr 1, 2025

oli-obk force-pushed the no-sort-hir-ids branch from 0dde879 to 7103ce7 Compare April 2, 2025 15:53

bors 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 Apr 2, 2025

matthiaskrgr mentioned this pull request Apr 2, 2025

Rollup of 7 pull requests #139268

Closed

m-ou-se mentioned this pull request Apr 3, 2025

Rollup of 6 pull requests #139300

Closed

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 3, 2025

oli-obk added 6 commits April 3, 2025 09:17

Split ExpectationLintId off Level

f3eaf16

Make LevelAndSource a struct

c51816e

Remove LintExpectationId from Level variants

805f389

Upvars HirIds always have the same owner, thus just use an ItemLocalId

0795b3d

impl !PartialOrd for UpvarMigrationInfo

5a4e7eb

impl !PartialOrd for HirId

57c4ab7

oli-obk force-pushed the no-sort-hir-ids branch from 7103ce7 to 57c4ab7 Compare April 3, 2025 09:22

bors 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 Apr 3, 2025

matthiaskrgr mentioned this pull request Apr 3, 2025

Rollup of 9 pull requests #139336

Merged

bors merged commit 48a3919 into rust-lang:master Apr 4, 2025
6 checks passed

rustbot added this to the 1.88.0 milestone Apr 4, 2025

oli-obk deleted the no-sort-hir-ids branch April 4, 2025 05:06

Uh oh!

Conversation

oli-obk commented Mar 17, 2025

Uh oh!

rustbot commented Mar 17, 2025

Uh oh!

rustbot commented Mar 17, 2025

Uh oh!

compiler-errors left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment has been minimized.

rustbot commented Mar 17, 2025

Uh oh!

rustbot commented Mar 19, 2025

Uh oh!

compiler-errors left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

oli-obk commented Apr 2, 2025

Uh oh!

bors commented Apr 2, 2025

Uh oh!

matthiaskrgr commented Apr 2, 2025

Uh oh!

matthiaskrgr commented Apr 3, 2025

Uh oh!

m-ou-se commented Apr 3, 2025

Uh oh!

oli-obk commented Apr 3, 2025

Uh oh!

bors commented Apr 3, 2025

Uh oh!

Kobzol commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Kobzol commented Apr 3, 2025 •

edited

Loading