Skip to content

Conversation

jieyouxu
Copy link
Member

@jieyouxu jieyouxu commented Mar 16, 2025

I was working on implementing --print=supported-crate-types, then I noticed some things that were mildly annoying me, so I pulled out these changes. In this PR:

  • First commit adds a centralized test tests/ui/print/stability.rs that is responsible for exercising stability gating of the print requests.
    • AFAICT we didn't have any test that systematically checks this.
    • I coalesced tests/ui/feature-gates/feature-gate-print-check-cfg.rs (for --print=check-cfg) into this test too, since --print=check-cfg is only -Z unstable-options-gated like other unstable print requests, and is not additionally feature-gated. cc @Urgau in case you have any concerns.
  • Second commit alphabetically sorts the PrintKind enum for consistency because the PRINT_KINDS list (using the enum) is already alphabetically sorted.
  • Third commit pulls out two helpers:
    1. A helper check_print_request_stability for checking stability of print requests and the diagnostics for using unstable print requests without -Z unstable-options, to avoid repeating the same logic over and over.
    2. A helper emit_unknown_print_request_help for the unknown print request diagnostics to make print request collection control flow more obvious.
  • Fourth commit renames PrintKind::{TargetSpec,AllTargetSpecs} to PrintKind::{TargetSpecJson,AllTargetSpecsJson} to better reflect their actual print names, --print={target-spec-json,all-target-specs-json}.

r? @nnethercote (or compiler/reroll)

@rustbot 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 16, 2025
@jieyouxu jieyouxu force-pushed the print-request-cleanups branch from 4fb7ac0 to a8619bd Compare March 16, 2025 05:41
@jieyouxu jieyouxu added the A-print-requests Area: print requests (`--print=...`) label Mar 16, 2025
Copy link
Member

@Urgau Urgau left a comment

Choose a reason for hiding this comment

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

Wasn't going for reviewing the all PR but ended-up doing it.

LGTM, modulo the readability of tests/ui/print-request/stability.rs.

@Urgau
Copy link
Member

Urgau commented Mar 16, 2025

r=me after addressing the readability issue

r? Urgau
@rustbot author

@rustbot rustbot assigned Urgau and unassigned nnethercote Mar 16, 2025
@rustbot 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 Mar 16, 2025
I can't find any dedicated tests that actually exercises the stability
gating (via `-Z unstable-options`) of print requests, so here's a
dedicated one.

I coalesced `tests/ui/feature-gates/feature-gate-print-check-cfg.rs`
into this test, because AFAICT that print request is not feature gated,
but only `-Z unstable-options`-gated just like other unstable print
requests.
… into helpers

To avoid duplicating stability check logic and make the print request
collection logic more straightforward.
…on,TargetSpecJson}`

To correspond to their actual print request names, `target-spec-json`
and `all-target-specs-json`, and for consistency with other print name
<-> print kind mappings.
@jieyouxu jieyouxu force-pushed the print-request-cleanups branch from a8619bd to 24edbfb Compare March 16, 2025 13:56
@jieyouxu
Copy link
Member Author

Not sure why I squished all the options together lmao. Anyway, added spacing (which I find is much more readable too) to the test, no other changes.

@bors r=@Urgau rollup

@bors
Copy link
Collaborator

bors commented Mar 16, 2025

📌 Commit 24edbfb has been approved by Urgau

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 16, 2025
jhpratt added a commit to jhpratt/rust that referenced this pull request Mar 16, 2025
…=Urgau

Misc print request handling cleanups + a centralized test for print request stability gating

I was working on implementing `--print=supported-crate-types`, then I noticed some things that were mildly annoying me, so I pulled out these changes. In this PR:

- First commit adds a centralized test `tests/ui/print/stability.rs` that is responsible for exercising stability gating of the print requests.
    - AFAICT we didn't have any test that systematically checks this.
    - I coalesced `tests/ui/feature-gates/feature-gate-print-check-cfg.rs` (for `--print=check-cfg`) into this test too, since `--print=check-cfg` is only `-Z unstable-options`-gated like other unstable print requests, and is not additionally feature-gated. cc `@Urgau` in case you have any concerns.
- Second commit alphabetically sorts the `PrintKind` enum for consistency because the `PRINT_KINDS` list (using the enum) is *already* alphabetically sorted.
- Third commit pulls out two helpers:
    1. A helper `check_print_request_stability` for checking stability of print requests and the diagnostics for using unstable print requests without `-Z unstable-options`, to avoid repeating the same logic over and over.
    2. A helper `emit_unknown_print_request_help` for the unknown print request diagnostics to make print request collection control flow more obvious.
- Fourth commit renames `PrintKind::{TargetSpec,AllTargetSpecs}` to `PrintKind::{TargetSpecJson,AllTargetSpecsJson}` to better reflect their actual print names, `--print={target-spec-json,all-target-specs-json}`.

r? `@nnethercote` (or compiler/reroll)
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 16, 2025
Rollup of 7 pull requests

Successful merges:

 - rust-lang#136293 (document capacity for ZST as example)
 - rust-lang#136355 (Add `*_value` methods to proc_macro lib)
 - rust-lang#136359 (doc all differences of ptr:copy(_nonoverlapping) with memcpy and memmove)
 - rust-lang#136816 (refactor `notable_traits_button` to use iterator combinators  instead of for loop)
 - rust-lang#138363 (Add `From<{integer}>` for `f16`/`f128` impls)
 - rust-lang#138552 (Misc print request handling cleanups + a centralized test for print request stability gating)
 - rust-lang#138573 (Make `_Unwind_Action` a type alias, not enum)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 17, 2025
Rollup of 5 pull requests

Successful merges:

 - rust-lang#136293 (document capacity for ZST as example)
 - rust-lang#136359 (doc all differences of ptr:copy(_nonoverlapping) with memcpy and memmove)
 - rust-lang#136816 (refactor `notable_traits_button` to use iterator combinators  instead of for loop)
 - rust-lang#138552 (Misc print request handling cleanups + a centralized test for print request stability gating)
 - rust-lang#138573 (Make `_Unwind_Action` a type alias, not enum)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6da26f7 into rust-lang:master Mar 17, 2025
6 checks passed
@rustbot rustbot added this to the 1.87.0 milestone Mar 17, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 17, 2025
Rollup merge of rust-lang#138552 - jieyouxu:print-request-cleanups, r=Urgau

Misc print request handling cleanups + a centralized test for print request stability gating

I was working on implementing `--print=supported-crate-types`, then I noticed some things that were mildly annoying me, so I pulled out these changes. In this PR:

- First commit adds a centralized test `tests/ui/print/stability.rs` that is responsible for exercising stability gating of the print requests.
    - AFAICT we didn't have any test that systematically checks this.
    - I coalesced `tests/ui/feature-gates/feature-gate-print-check-cfg.rs` (for `--print=check-cfg`) into this test too, since `--print=check-cfg` is only `-Z unstable-options`-gated like other unstable print requests, and is not additionally feature-gated. cc ``@Urgau`` in case you have any concerns.
- Second commit alphabetically sorts the `PrintKind` enum for consistency because the `PRINT_KINDS` list (using the enum) is *already* alphabetically sorted.
- Third commit pulls out two helpers:
    1. A helper `check_print_request_stability` for checking stability of print requests and the diagnostics for using unstable print requests without `-Z unstable-options`, to avoid repeating the same logic over and over.
    2. A helper `emit_unknown_print_request_help` for the unknown print request diagnostics to make print request collection control flow more obvious.
- Fourth commit renames `PrintKind::{TargetSpec,AllTargetSpecs}` to `PrintKind::{TargetSpecJson,AllTargetSpecsJson}` to better reflect their actual print names, `--print={target-spec-json,all-target-specs-json}`.

r? ``@nnethercote`` (or compiler/reroll)
@jieyouxu jieyouxu deleted the print-request-cleanups branch March 17, 2025 09:28
github-actions bot pushed a commit to tautschnig/verify-rust-std that referenced this pull request Mar 26, 2025
Rollup of 5 pull requests

Successful merges:

 - rust-lang#136293 (document capacity for ZST as example)
 - rust-lang#136359 (doc all differences of ptr:copy(_nonoverlapping) with memcpy and memmove)
 - rust-lang#136816 (refactor `notable_traits_button` to use iterator combinators  instead of for loop)
 - rust-lang#138552 (Misc print request handling cleanups + a centralized test for print request stability gating)
 - rust-lang#138573 (Make `_Unwind_Action` a type alias, not enum)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-print-requests Area: print requests (`--print=...`) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants