Skip to content

Conversation

CAD97
Copy link
Contributor

@CAD97 CAD97 commented Mar 26, 2025

All of these uses of cfg_if! do not utilize that cfg_if! works with auto-doc(cfg), so no functionality is lost from switching to use cfg_match! instead. We do lose what rustfmt special support exists for cfg_if!, though.

Tracking issue: #115585

@rustbot
Copy link
Collaborator

rustbot commented Mar 26, 2025

r? @scottmcm

rustbot has assigned @scottmcm.
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 rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 26, 2025
@CAD97 CAD97 requested a review from Copilot March 26, 2025 18:40
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR updates the core library to use cfg_match! instead of cfg_if! to take advantage of the auto‑doc(cfg) capability.

  • Replace occurrences of cfg_if! with cfg_match! in multiple sorting modules and FFI definitions.
  • Remove the redundant cfg_if! macro in internal_macros.rs and enable the new cfg_match feature in lib.rs.

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
library/core/src/slice/sort/stable/mod.rs Updated conditional compilation to use cfg_match! in stable sort logic.
library/core/src/slice/sort/select.rs Replaced cfg_if! with cfg_match! for improved pivot selection handling.
library/core/src/slice/sort/unstable/quicksort.rs Swapped cfg_if! for cfg_match! in quicksort partitioning routines.
library/core/src/ffi/primitives.rs Migrated FFI type definitions from cfg_if! to cfg_match! to align with new standards.
library/core/src/slice/sort/unstable/mod.rs Updated unstable sort algorithms to use cfg_match! for conditional branches.
library/core/src/num/f32.rs Changed f32 midpoint calculation to use cfg_match! for performance optimizations.
library/core/src/internal_macros.rs Removed the legacy cfg_if! macro implementation.
library/core/src/lib.rs Added the cfg_match feature flag to enable new conditional compilation support.

@CAD97
Copy link
Contributor Author

CAD97 commented Mar 26, 2025

Well, I wanted to see how well that worked, and no, that's wrong. Uses of cfg_match! aren't visible to feature(doc_autocfg), and none of the few exported definitions covered here document a required cfg. Certain versions of cfg_if! do propagate the cfg to individual items, in order to work somewhat better with doc autocfg, but core's version didn't anyway.

Probably won't be clicking that button ever again.

@joboet
Copy link
Member

joboet commented Mar 26, 2025

Great, thank you!
r? joboet
@bors r+ rollup=never p=1
(bumping this because it's quite likely to bitrot)

@bors
Copy link
Collaborator

bors commented Mar 26, 2025

📌 Commit 2e5a76c has been approved by joboet

It is now in the queue for this repository.

@rustbot rustbot assigned joboet and unassigned scottmcm Mar 26, 2025
@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 26, 2025
@bors
Copy link
Collaborator

bors commented Mar 26, 2025

⌛ Testing commit 2e5a76c with merge 1644cb0...

@bors
Copy link
Collaborator

bors commented Mar 27, 2025

☀️ Test successful - checks-actions
Approved by: joboet
Pushing 1644cb0 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 27, 2025
@bors bors merged commit 1644cb0 into rust-lang:master Mar 27, 2025
7 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 27, 2025
Copy link
Contributor

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 a2e6356 (parent) -> 1644cb0 (this PR)

Test differences

Show 58 test diffs

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

Job group index

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (1644cb0): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.3% [0.3%, 0.3%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary 3.0%, secondary 2.6%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
3.0% [0.9%, 3.9%] 5
Regressions ❌
(secondary)
2.7% [0.7%, 3.9%] 45
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-1.7% [-1.7%, -1.7%] 1
All ❌✅ (primary) 3.0% [0.9%, 3.9%] 5

Cycles

Results (primary -2.3%, secondary 2.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
2.2% [1.5%, 2.6%] 9
Improvements ✅
(primary)
-2.3% [-2.3%, -2.3%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.3% [-2.3%, -2.3%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 778.152s -> 777.476s (-0.09%)
Artifact size: 365.74 MiB -> 365.76 MiB (0.00%)

@CAD97 CAD97 deleted the use_cfg_match branch March 27, 2025 03:50
@jieyouxu jieyouxu added the F-cfg_select `#![feature(cfg_select)]` label Aug 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-cfg_select `#![feature(cfg_select)]` merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants