[compiler] Implement PartialOrd via Ord for Span and newtype_indexes#158996
Conversation
…dexes For something unconditionally `Ord`, this is better than using `partial_cmp` on fields. I couldn't find any other cases in the compiler codebase of this, but it's entirely possible I missed some. Let me know if you can think of any.
|
r? @folkertdev rustbot has assigned @folkertdev. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
@bors r+ rollup |
…kertdev [compiler] Implement `PartialOrd` via `Ord` for `Span` and newtype_indexes For something unconditionally `Ord`, this is better than using `partial_cmp` on fields. I couldn't find any other cases in the compiler codebase of this, but it's entirely possible I missed some. Let me know if you can think of any.
…kertdev [compiler] Implement `PartialOrd` via `Ord` for `Span` and newtype_indexes For something unconditionally `Ord`, this is better than using `partial_cmp` on fields. I couldn't find any other cases in the compiler codebase of this, but it's entirely possible I missed some. Let me know if you can think of any.
…kertdev [compiler] Implement `PartialOrd` via `Ord` for `Span` and newtype_indexes For something unconditionally `Ord`, this is better than using `partial_cmp` on fields. I couldn't find any other cases in the compiler codebase of this, but it's entirely possible I missed some. Let me know if you can think of any.
|
@bors try @rust-timer queue Just to make sure, spans are quite hot. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
[compiler] Implement `PartialOrd` via `Ord` for `Span` and newtype_indexes
…uwer Rollup of 21 pull requests Successful merges: - #150946 (intrinsics: Add a fallback for non-const libm float functions) - #158510 (Enable `static_position_independent_executables` on all gnu and musl targets) - #158541 (Move `std::io::Write` to `core::io`) - #158899 (Fix `unaligned_volatile_store` by removing `MemFlags::UNALIGNED`) - #155429 (Support `u128`/`i128` c-variadic arguments) - #156370 (Reject linked dylib EII default overrides) - #156508 (Infer all anonymous lifetimes in assoc consts as `'static`) - #158617 (allow mGCA const arguments to fall back to anon consts) - #158645 (Fix splat ICEs and ban it in closures) - #158859 (Improve `-Zls` diagnostic message on `.rs` files) - #158988 (Redo `TokenStreamIter`) - #158347 (Improve generic parameters handling for #[diagnostic::on_const]) - #158722 (delegation: do not always inherit `ConstArgHasType` predicates) - #158739 (view-types: HIR lowering) - #158883 (tests: fix enum-match.rs to handle LLVM 23) - #158886 (Add documentation for the `no_std` attribute) - #158940 (Implement feature `char_to_u32`) - #158951 (Merge three `MaxUniverse`s into one) - #158961 (Reapply "LLVM 23: Run AssignGUIDPass in some places") - #158996 ([compiler] Implement `PartialOrd` via `Ord` for `Span` and newtype_indexes) - #159005 (Use `as_lang_item` instead of repeatedly matching)
This comment has been minimized.
This comment has been minimized.
…kertdev [compiler] Implement `PartialOrd` via `Ord` for `Span` and newtype_indexes For something unconditionally `Ord`, this is better than using `partial_cmp` on fields. I couldn't find any other cases in the compiler codebase of this, but it's entirely possible I missed some. Let me know if you can think of any.
|
Finished benchmarking commit (5e75c18): comparison URL. Overall result: no relevant changes - no action neededBenchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up. @rustbot label: -S-waiting-on-perf -perf-regression Instruction countThis perf run didn't have relevant results for this metric. Max RSS (memory usage)Results (secondary 6.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 0.3%, secondary 0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 489.147s -> 489.205s (0.01%) |
…uwer Rollup of 24 pull requests Successful merges: - #150946 (intrinsics: Add a fallback for non-const libm float functions) - #158510 (Enable `static_position_independent_executables` on all gnu and musl targets) - #158541 (Move `std::io::Write` to `core::io`) - #158899 (Fix `unaligned_volatile_store` by removing `MemFlags::UNALIGNED`) - #156027 (Consider captured regions for opaque type region liveness.) - #156370 (Reject linked dylib EII default overrides) - #156508 (Infer all anonymous lifetimes in assoc consts as `'static`) - #157561 (rustdoc: do not include extra stuff in span) - #158617 (allow mGCA const arguments to fall back to anon consts) - #158645 (Fix splat ICEs and ban it in closures) - #158859 (Improve `-Zls` diagnostic message on `.rs` files) - #158988 (Redo `TokenStreamIter`) - #158347 (Improve generic parameters handling for #[diagnostic::on_const]) - #158384 (Allow BackwardIncompatibleDropHint in polonius legacy) - #158722 (delegation: do not always inherit `ConstArgHasType` predicates) - #158739 (view-types: HIR lowering) - #158877 (borrowck: Keep returned `path` from `best_blame_constraint()` consistent) - #158883 (tests: fix enum-match.rs to handle LLVM 23) - #158886 (Add documentation for the `no_std` attribute) - #158940 (Implement feature `char_to_u32`) - #158951 (Merge three `MaxUniverse`s into one) - #158961 (Reapply "LLVM 23: Run AssignGUIDPass in some places") - #158995 (Use REST API in linkchecker script) - #158996 ([compiler] Implement `PartialOrd` via `Ord` for `Span` and newtype_indexes)
…kertdev [compiler] Implement `PartialOrd` via `Ord` for `Span` and newtype_indexes For something unconditionally `Ord`, this is better than using `partial_cmp` on fields. I couldn't find any other cases in the compiler codebase of this, but it's entirely possible I missed some. Let me know if you can think of any.
Rollup of 24 pull requests Successful merges: - #150946 (intrinsics: Add a fallback for non-const libm float functions) - #158541 (Move `std::io::Write` to `core::io`) - #156027 (Consider captured regions for opaque type region liveness.) - #156370 (Reject linked dylib EII default overrides) - #156508 (Infer all anonymous lifetimes in assoc consts as `'static`) - #157561 (rustdoc: do not include extra stuff in span) - #158617 (allow mGCA const arguments to fall back to anon consts) - #158645 (Fix splat ICEs and ban it in closures) - #158859 (Improve `-Zls` diagnostic message on `.rs` files) - #158988 (Redo `TokenStreamIter`) - #158347 (Improve generic parameters handling for #[diagnostic::on_const]) - #158384 (Allow BackwardIncompatibleDropHint in polonius legacy) - #158722 (delegation: do not always inherit `ConstArgHasType` predicates) - #158739 (view-types: HIR lowering) - #158877 (borrowck: Keep returned `path` from `best_blame_constraint()` consistent) - #158883 (tests: fix enum-match.rs to handle LLVM 23) - #158886 (Add documentation for the `no_std` attribute) - #158940 (Implement feature `char_to_u32`) - #158951 (Merge three `MaxUniverse`s into one) - #158960 (Fix bootstrap submodule path prefix matching) - #158961 (Reapply "LLVM 23: Run AssignGUIDPass in some places") - #158995 (Use REST API in linkchecker script) - #158996 ([compiler] Implement `PartialOrd` via `Ord` for `Span` and newtype_indexes) - #159036 (bootstrap: expand '@argfile' arguments to rustc shim)
Rollup merge of #158996 - scottmcm:better-partial-cmp, r=folkertdev [compiler] Implement `PartialOrd` via `Ord` for `Span` and newtype_indexes For something unconditionally `Ord`, this is better than using `partial_cmp` on fields. I couldn't find any other cases in the compiler codebase of this, but it's entirely possible I missed some. Let me know if you can think of any.
|
@rust-timer build 4db5f96 |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (4db5f96): comparison URL. Overall result: ❌ regressions - please read:Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. Next, please: If you can, justify the regressions found in this try perf run in writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary -3.6%, secondary -3.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -0.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis perf run didn't have relevant results for this metric. Bootstrap: 490.768s -> 489.353s (-0.29%) |
|
@rustbot label: +perf-regression-triaged |
For something unconditionally
Ord, this is better than usingpartial_cmpon fields.I couldn't find any other cases in the compiler codebase of this, but it's entirely possible I missed some. Let me know if you can think of any.