Disallow setting some built-in cfg via the command-line #126158

Urgau · 2024-06-08T12:59:21Z

This PR disallow users from setting some built-in cfg via set the command-line in order to prevent incoherent state, eg. windows cfg active but target is Linux based.

This implements MCP rust-lang/compiler-team#610, with the caveat that we disallow cfgs no matter if they make sense or not, since I don't think it's useful to allow users to set a cfg that will be set anyway. It also complicates the implementation.

The explicit_builtin_cfgs_in_flags lint detects builtin cfgs set via the --cfg flag.

(deny-by-default)

Example

rustc --cfg unix

fn main() {}

This will produce:

error: unexpected `--cfg unix` flag
  |
  = note: config `unix` is only supposed to be controlled by `--target`
  = note: manually setting a built-in cfg can and does create incoherent behaviours
  = note: `#[deny(explicit_builtin_cfgs_in_flags)]` on by default

Explanation

Setting builtin cfgs can and does produce incoherent behaviour, it's better to the use the appropriate rustc flag that controls the config. For example setting the windows cfg but on Linux based target.

r? @petrochenkov
cc @jyn514

try-job: aarch64-apple
try-job: test-various
try-job: armhf-gnu
try-job: x86_64-msvc
try-job: x86_64-mingw
try-job: i686-msvc
try-job: i686-mingw
try-job: x86_64-gnu-llvm-17
try-job: dist-various-1

compiler/rustc_session/src/config/cfg.rs

Urgau · 2024-06-22T15:23:18Z

I've changed the error from a hard-error to a deny-by-default lint as asked and included all the possible cfgs that wouldn't break the world (and added some links for the one that we can't lint on them).

@rustbot ready

tests/codegen/default-requires-uwtable.rs

tests/debuginfo/thread-names.rs

compiler/rustc_session/src/config/cfg.rs

Urgau · 2024-06-24T19:09:48Z

Addressed or replied to all the review comments.

@rustbot ready

petrochenkov · 2024-06-25T10:02:20Z

Ok, let's try landing this.
@bors r

bors · 2024-06-25T10:02:22Z

📌 Commit 971cb13 has been approved by petrochenkov

It is now in the queue for this repository.

Urgau · 2024-08-07T12:32:52Z

Another case of a revisions: containing windows/unix as revision name.
Fixed those and re-did a grep on tests/.

But just to be sure, and to not waste more CI-merge time. Let's do a big try build across many different targets.
@bors try

Disallow setting some built-in cfg via set the command-line This PR disallow users from setting some built-in cfg via set the command-line in order to prevent incoherent state, eg. `windows` cfg active but target is Linux based. This implements MCP rust-lang/compiler-team#610, with the caveat that we disallow cfgs no matter if they make sense or not, since I don't think it's useful to allow users to set a cfg that will be set anyway. It also complicates the implementation. ------ The `explicit_builtin_cfgs_in_flags` lint detects builtin cfgs set via the `--cfg` flag. *(deny-by-default)* ### Example ```text rustc --cfg unix ``` ```rust,ignore (needs command line option) fn main() {} ``` This will produce: ```text error: unexpected `--cfg unix` flag | = note: config `unix` is only supposed to be controlled by `--target` = note: manually setting a built-in cfg can and does create incoherent behaviours = note: `#[deny(explicit_builtin_cfgs_in_flags)]` on by default ``` ### Explanation Setting builtin cfgs can and does produce incoherent behaviour, it's better to the use the appropriate `rustc` flag that controls the config. For example setting the `windows` cfg but on Linux based target. ----- r? `@petrochenkov` cc `@jyn514` try-job: aarch64-apple try-job: test-various try-job: armhf-gnu try-job: x86_64-msvc try-job: x86_64-mingw try-job: i686-msvc try-job: i686-mingw try-job: x86_64-gnu-llvm-17 try-job: dist-various-1

bors · 2024-08-07T12:34:03Z

⌛ Trying commit c0c57b3 with merge ac7969d...

petrochenkov · 2024-08-07T13:23:20Z

r=me after CI is green.
@rustbot author

lcnr · 2024-08-07T13:57:43Z

@bors r=petrochenkov

bors · 2024-08-07T13:57:46Z

📌 Commit c0c57b3 has been approved by petrochenkov

It is now in the queue for this repository.

Urgau · 2024-08-07T13:58:57Z

Wait, try isn't finishing yet, and bors doesn't like r and try at the same time.
@bors r-

bors · 2024-08-07T14:56:51Z

☀️ Try build successful - checks-actions
Build commit: ac7969d (ac7969db6cc18caaabd206a34b8c5034a329765d)

Urgau · 2024-08-07T15:00:59Z

@bors r=petrochenkov

bors · 2024-08-07T15:01:01Z

📌 Commit c0c57b3 has been approved by petrochenkov

It is now in the queue for this repository.

bors · 2024-08-07T17:32:19Z

⌛ Testing commit c0c57b3 with merge ce20e15...

bors · 2024-08-07T19:59:22Z

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing ce20e15 to master...

rust-timer · 2024-08-07T22:19:38Z

Finished benchmarking commit (ce20e15): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

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

Max RSS (memory usage)

Results (primary -2.6%, secondary -1.9%)

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)	3.3%	[3.3%, 3.3%]	1
Improvements ✅ (primary)	-2.6%	[-2.6%, -2.6%]	1
Improvements ✅ (secondary)	-3.6%	[-3.7%, -3.5%]	3
All ❌✅ (primary)	-2.6%	[-2.6%, -2.6%]	1

Cycles

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

Binary size

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

Bootstrap: 760.558s -> 760.891s (0.04%)
Artifact size: 336.96 MiB -> 336.96 MiB (0.00%)

rustbot assigned petrochenkov Jun 8, 2024

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 Jun 8, 2024

Urgau force-pushed the disallow-cfgs branch from 4b1e14f to 37aa414 Compare June 8, 2024 14:36

petrochenkov reviewed Jun 11, 2024

View reviewed changes

compiler/rustc_session/src/config/cfg.rs Outdated Show resolved Hide resolved

petrochenkov reviewed Jun 11, 2024

View reviewed changes

compiler/rustc_session/src/config/cfg.rs Outdated Show resolved Hide resolved

petrochenkov 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 Jun 11, 2024

Urgau force-pushed the disallow-cfgs branch 3 times, most recently from 4b6c448 to 4ebdd5c Compare June 22, 2024 13:21

This comment has been minimized.

Sign in to view

Urgau force-pushed the disallow-cfgs branch from 4ebdd5c to 37d63c6 Compare June 22, 2024 13:50

This comment has been minimized.

Sign in to view

Urgau force-pushed the disallow-cfgs branch 2 times, most recently from e63e8f7 to 30480ad Compare June 22, 2024 14:19

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 22, 2024

petrochenkov reviewed Jun 24, 2024

View reviewed changes

tests/codegen/default-requires-uwtable.rs Show resolved Hide resolved

tests/debuginfo/thread-names.rs Outdated Show resolved Hide resolved

compiler/rustc_session/src/config/cfg.rs Show resolved Hide resolved

compiler/rustc_session/src/config/cfg.rs Outdated Show resolved Hide resolved

petrochenkov 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 Jun 24, 2024

Urgau force-pushed the disallow-cfgs branch from 30480ad to 971cb13 Compare June 24, 2024 18:05

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 24, 2024

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 Jun 25, 2024

Urgau force-pushed the disallow-cfgs branch from 20090e6 to c0c57b3 Compare August 7, 2024 12:20

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 Aug 7, 2024

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 Aug 7, 2024

bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 7, 2024

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 Aug 7, 2024

Urgau changed the title ~~Disallow setting some built-in cfg via set the command-line~~ Disallow setting some built-in cfg via the command-line Aug 7, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 7, 2024

bors merged commit ce20e15 into rust-lang:master Aug 7, 2024
7 checks passed

rustbot added this to the 1.82.0 milestone Aug 7, 2024

bors mentioned this pull request Aug 7, 2024

Split elided_lifetime_in_paths into tied and untied #120808

Draft

3 tasks

apiraino removed the to-announce Announce this issue on triage meeting label Aug 15, 2024

Urgau added the relnotes Marks issues that should be documented in the release notes of the next release. label Sep 6, 2024

rustbot mentioned this pull request Sep 6, 2024

Tracking issue for release notes of #126158: Disallow setting some built-in cfg via the command-line #130035

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Disallow setting some built-in cfg via the command-line #126158

Disallow setting some built-in cfg via the command-line #126158

Urgau commented Jun 8, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

Urgau commented Jun 22, 2024 •

edited

Loading

Urgau commented Jun 24, 2024 •

edited

Loading

petrochenkov commented Jun 25, 2024

bors commented Jun 25, 2024

Urgau commented Aug 7, 2024

bors commented Aug 7, 2024

petrochenkov commented Aug 7, 2024

lcnr commented Aug 7, 2024

bors commented Aug 7, 2024

Urgau commented Aug 7, 2024

bors commented Aug 7, 2024

Urgau commented Aug 7, 2024

bors commented Aug 7, 2024

bors commented Aug 7, 2024

bors commented Aug 7, 2024

rust-timer commented Aug 7, 2024

Disallow setting some built-in cfg via the command-line #126158

Disallow setting some built-in cfg via the command-line #126158

Conversation

Urgau commented Jun 8, 2024 • edited Loading

Example

Explanation

This comment has been minimized.

This comment has been minimized.

Urgau commented Jun 22, 2024 • edited Loading

Urgau commented Jun 24, 2024 • edited Loading

petrochenkov commented Jun 25, 2024

bors commented Jun 25, 2024

Urgau commented Aug 7, 2024

bors commented Aug 7, 2024

petrochenkov commented Aug 7, 2024

lcnr commented Aug 7, 2024

bors commented Aug 7, 2024

Urgau commented Aug 7, 2024

bors commented Aug 7, 2024

Urgau commented Aug 7, 2024

bors commented Aug 7, 2024

bors commented Aug 7, 2024

bors commented Aug 7, 2024

rust-timer commented Aug 7, 2024

Overall result: no relevant changes - no action needed

Instruction count

Max RSS (memory usage)

Cycles

Binary size

Urgau commented Jun 8, 2024 •

edited

Loading

Urgau commented Jun 22, 2024 •

edited

Loading

Urgau commented Jun 24, 2024 •

edited

Loading