Skip to content

Conversation

hkBst
Copy link
Member

@hkBst hkBst commented Oct 10, 2025

No description provided.

@rustbot
Copy link
Collaborator

rustbot commented Oct 10, 2025

Some changes occurred in cfg and check-cfg configuration

cc @Urgau

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 10, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 10, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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

@rust-log-analyzer

This comment has been minimized.

@Urgau
Copy link
Member

Urgau commented Oct 10, 2025

r? Urgau

@rustbot rustbot assigned Urgau and unassigned Mark-Simulacrum Oct 10, 2025
@rust-log-analyzer

This comment has been minimized.

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.

Comment on lines +494 to 500
pub fn check_cfg_arg(name: &str, values: &[&str]) -> String {
if values.is_empty() {
format!("--check-cfg=cfg({name})")
} else {
format!("--check-cfg=cfg({name},values(\"{}\"))", values.join("\",\""))
}
}
Copy link
Member

Choose a reason for hiding this comment

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

This is not technically correct, having an empty values() is valid and has the semantic of defining a cfg that has no expected values (and thus will always warn).

But it doesn't really make sense for bootstrap to be defining such cfgs.

Copy link
Member Author

Choose a reason for hiding this comment

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

I read about this, but it seems really weird that values() makes the cfg name unusable(?) and what accepts an empty value is values(none()).

Comment on lines +84 to +90
const EXTRA_CHECK_CFGS: &[(Option<Mode>, &str, &[&str])] = &[
(Some(Mode::Rustc), "bootstrap", &[]),
(Some(Mode::Codegen), "bootstrap", &[]),
(Some(Mode::ToolRustcPrivate), "bootstrap", &[]),
(Some(Mode::ToolStd), "bootstrap", &[]),
(Some(Mode::ToolRustcPrivate), "rust_analyzer", &[]),
(Some(Mode::ToolStd), "rust_analyzer", &[]),
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about having &[Option<&str>] for representing the expected values? which would make the none() part explicit.

The generation part would then always be like that cfg(name, values(...)).

Copy link
Member Author

Choose a reason for hiding this comment

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

That would allow &[None, None, None] which seems silly...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) 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