Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ensure we always print all --print options in help #137862

Merged
merged 1 commit into from
Mar 4, 2025

Conversation

mtoner23
Copy link
Contributor

@mtoner23 mtoner23 commented Mar 1, 2025

Closes #137853
Refactors the PRINT_KINDS map into a public const so we always print every option for print. the list is quite long now, and idk if long term we want to keep printing all these options from --help.

@rustbot
Copy link
Collaborator

rustbot commented Mar 1, 2025

r? @nnethercote

rustbot has assigned @nnethercote.
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 A-run-make Area: port run-make Makefiles to rmake.rs S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 1, 2025
@rustbot
Copy link
Collaborator

rustbot commented Mar 1, 2025

This PR modifies tests/run-make/. If this PR is trying to port a Makefile
run-make test to use rmake.rs, please update the
run-make port tracking issue
so we can track our progress. You can either modify the tracking issue
directly, or you can comment on the tracking issue and link this PR.

cc @jieyouxu

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Mar 1, 2025
@@ -1508,6 +1534,13 @@ The default is {DEFAULT_EDITION} and the latest stable edition is {LATEST_STABLE
)
});

Copy link
Member

Choose a reason for hiding this comment

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

can you make this a local variable instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, just remove pub or put it in some function?

Copy link
Member

@Noratrieb Noratrieb Mar 1, 2025

Choose a reason for hiding this comment

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

Yes, I meant making it a let variable in the function that uses it, then you don't need a lazylock

Copy link
Member

Choose a reason for hiding this comment

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

Actually, when reading the code a bit, I see that this same thing is needed in the collect_print_requests function as well. In that case it makes sense to share it at the global scope, but I'd prefer a fn print_kinds_string() -> String function over a lazy lock.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sounds good i'll make come up with a func somehwere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

extracting to a function makes this all worse than a lazy lock imo. the reason theres a lazy lock is because make_opt expects your format string to be static lifetime. and its possible theres some way to craft a const fn print_kinds_str() that uses some const things and for loops to iterate over this list but it all seems overly complicated for this simple print code.

so can we stick with lazy lock? it follows the pattern of EDITION_STRING already there.

Copy link
Member

Choose a reason for hiding this comment

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

ah, okay. it's fine then, I wasn't aware of the static lifetime requirement.

@Noratrieb Noratrieb assigned Noratrieb and unassigned nnethercote Mar 2, 2025
("target-libdir", PrintKind::TargetLibdir),
("target-list", PrintKind::TargetList),
("target-spec-json", PrintKind::TargetSpec),
("tls-models", PrintKind::TlsModels),
Copy link
Member

Choose a reason for hiding this comment

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

If you want to make another PR improving --print, I'd suggest adding a third tuple field that indicates whether the print request is stable or not instead of the custom checks in collect_print_requests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea, I'll consider it. adding some more helper functions for the PrintKinds enum seem appropriate.

@nnethercote
Copy link
Contributor

LGTM

@bors r+

@bors
Copy link
Contributor

bors commented Mar 3, 2025

📌 Commit 6f505ba has been approved by nnethercote

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-review Status: Awaiting review from the assignee but also interested parties. labels Mar 3, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 3, 2025
ensure we always print all --print options in help

Closes rust-lang#137853
Refactors the PRINT_KINDS map into a public const so we always print every option for print. the list is quite long now, and idk if long term we want to keep printing all these options from --help.
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 3, 2025
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#132388 (Implement `#[cfg]` in `where` clauses)
 - rust-lang#134900 (Fix parsing of ranges after unary operators)
 - rust-lang#136938 (Remove `:` from `stack-protector-heuristics-effect.rs` Filecheck Pattern)
 - rust-lang#137054 (Make phantom variance markers transparent)
 - rust-lang#137525 (Simplify parallelization in test-float-parse)
 - rust-lang#137618 (Skip `tidy` in pre-push hook if the user is deleting a remote branch)
 - rust-lang#137685 (self-contained linker: conservatively default to `-znostart-stop-gc`)
 - rust-lang#137741 (Stop using `hash_raw_entry` in `CodegenCx::const_str`)
 - rust-lang#137849 (Revert "Remove Win SDK 10.0.26100.0 from CI")
 - rust-lang#137862 (ensure we always print all --print options in help)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 3, 2025
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#132388 (Implement `#[cfg]` in `where` clauses)
 - rust-lang#134900 (Fix parsing of ranges after unary operators)
 - rust-lang#136938 (Remove `:` from `stack-protector-heuristics-effect.rs` Filecheck Pattern)
 - rust-lang#137054 (Make phantom variance markers transparent)
 - rust-lang#137525 (Simplify parallelization in test-float-parse)
 - rust-lang#137618 (Skip `tidy` in pre-push hook if the user is deleting a remote branch)
 - rust-lang#137685 (self-contained linker: conservatively default to `-znostart-stop-gc`)
 - rust-lang#137741 (Stop using `hash_raw_entry` in `CodegenCx::const_str`)
 - rust-lang#137849 (Revert "Remove Win SDK 10.0.26100.0 from CI")
 - rust-lang#137862 (ensure we always print all --print options in help)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 3, 2025
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#132388 (Implement `#[cfg]` in `where` clauses)
 - rust-lang#134900 (Fix parsing of ranges after unary operators)
 - rust-lang#136938 (Remove `:` from `stack-protector-heuristics-effect.rs` Filecheck Pattern)
 - rust-lang#137054 (Make phantom variance markers transparent)
 - rust-lang#137525 (Simplify parallelization in test-float-parse)
 - rust-lang#137618 (Skip `tidy` in pre-push hook if the user is deleting a remote branch)
 - rust-lang#137741 (Stop using `hash_raw_entry` in `CodegenCx::const_str`)
 - rust-lang#137849 (Revert "Remove Win SDK 10.0.26100.0 from CI")
 - rust-lang#137862 (ensure we always print all --print options in help)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 3, 2025
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#132388 (Implement `#[cfg]` in `where` clauses)
 - rust-lang#134900 (Fix parsing of ranges after unary operators)
 - rust-lang#136938 (Remove `:` from `stack-protector-heuristics-effect.rs` Filecheck Pattern)
 - rust-lang#137054 (Make phantom variance markers transparent)
 - rust-lang#137525 (Simplify parallelization in test-float-parse)
 - rust-lang#137618 (Skip `tidy` in pre-push hook if the user is deleting a remote branch)
 - rust-lang#137741 (Stop using `hash_raw_entry` in `CodegenCx::const_str`)
 - rust-lang#137849 (Revert "Remove Win SDK 10.0.26100.0 from CI")
 - rust-lang#137862 (ensure we always print all --print options in help)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1998cf7 into rust-lang:master Mar 4, 2025
6 checks passed
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 4, 2025
Rollup merge of rust-lang#137862 - mtoner23:print-help, r=nnethercote

ensure we always print all --print options in help

Closes rust-lang#137853
Refactors the PRINT_KINDS map into a public const so we always print every option for print. the list is quite long now, and idk if long term we want to keep printing all these options from --help.
@rustbot rustbot added this to the 1.87.0 milestone Mar 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs 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.

rustc --help does not show all available flags of rustc --print
5 participants