impl !PartialOrd for HirId #138610
impl !PartialOrd for HirId #138610
Conversation
rustbot
added
S-waiting-on-review
|
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. |
| @@ -903,7 +903,11 @@ Available lint options: | |||
|
|
|||
| 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| { | |||
There was a problem hiding this comment.
Why did this change? Isn't this doing the same exact thing as previosuly? (Sorting by a key of default_level + name?)
There was a problem hiding this comment.
Wait, why does it only have a partial ordering?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Yeah, actually the more I think about this, it seems to me to that LintLevel now has and invalid impl of partialord 🤔
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Yea that worked out nicely, just tripled the size of the PR ^^
compiler/rustc_passes/src/dead.rs
Outdated
| @@ -1131,7 +1131,7 @@ impl<'tcx> DeadVisitor<'tcx> { | |||
| if dead_codes.is_empty() { | |||
| return; | |||
| } | |||
| dead_codes.sort_by_key(|v| v.level); | |||
| dead_codes.sort_by(|a, b| a.level.partial_cmp(&b.level).unwrap()); | |||
There was a problem hiding this comment.
sort_by_key(|a| a.level)?
There was a problem hiding this comment.
Level isn't Ord anymore
| @@ -866,6 +867,9 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { | |||
| HirPlaceBase::Upvar(upvar_id) => upvar_id.var_path.hir_id, | |||
| _ => bug!("Expected an upvar"), | |||
| }; | |||
| 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; | |||
There was a problem hiding this comment.
can you inline this into its usage?
There was a problem hiding this comment.
there are no early returns, so I thought it good to do the check and the conversion as early as possible
This comment has been minimized.
This comment has been minimized.
oli-obk
force-pushed
the
no-sort-hir-ids
branch
from
3da287a to
84d7916
Compare
|
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
oli-obk
force-pushed
the
no-sort-hir-ids
branch
from
84d7916 to
0dde879
Compare
|
Some changes occurred to the CTFE machinery Some changes occurred in compiler/rustc_codegen_ssa Some changes occurred in exhaustiveness checking cc @Nadrieril Some changes occurred in match checking cc @Nadrieril |
revive of #92233
Another checkbox of #90317, another small step in making incremental less likely to die in horrible ways