Skip to content

Reject leading unsafe in cfg!(...) and --check-cfg#131057

Merged
bors merged 1 commit intorust-lang:masterfrom
Urgau:cfg-erronous-unsafe
Sep 30, 2024
Merged

Reject leading unsafe in cfg!(...) and --check-cfg#131057
bors merged 1 commit intorust-lang:masterfrom
Urgau:cfg-erronous-unsafe

Conversation

@Urgau
Copy link
Member

@Urgau Urgau commented Sep 30, 2024 •
edited
Loading

This PR reject leading unsafe in cfg!(...) and --check-cfg.

cfg!(unsafe(foo)) and --check-cfg=unsafe(cfg(foo)) both are erroneously accepted on beta.

Fixes (after-backport) #131055
r? @jieyouxu

@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 Sep 30, 2024
@Urgau Urgau mentioned this pull request Sep 30, 2024
Merged
jieyouxu
jieyouxu approved these changes Sep 30, 2024
View reviewed changes
Copy link
Member

@jieyouxu jieyouxu left a comment

Choose a reason for hiding this comment

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

Thanks. You can r=me after PR CI is green.

@jieyouxu
Copy link
Member

jieyouxu commented Sep 30, 2024 •
edited
Loading

Though, does this need T-lang feedback regarding whether unsafe is accepted/rejected in that position? I'm okay with rejecting this conservatively first and back-port to beta, and possibly relaxing/accept unsafe in that position intentionally in the future (accepting previously-rejected code isn't a breaking change, but breaking previously-accepted code is, regardless of if the breakage is considered a bug fix, AFAIK).

@Urgau
Copy link
Member Author

Urgau commented Sep 30, 2024

It's rejected for #[cfg]&#[cfg_attr] so rejecting for cfg! makes everything consistent. If T-lang ever wants to allow it inside that can be changed latter without a breaking change.

As for --check-cfg it's T-compiler area and it really doesn't make any sense to allow unsafe here.

@jieyouxu
Copy link
Member

jieyouxu commented Sep 30, 2024

Alright, in that case we're fine. Thanks for clarifying.

@jieyouxu
Copy link
Member

jieyouxu commented Sep 30, 2024 •
edited
Loading

Nominating for beta backport to reject erroneously accepted cfg!(unsafe(foo)) and --check-cfg=unsafe(cfg(foo)) on beta. See issue #131055.

@rustbot label +beta-nominated

@rustbot rustbot added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Sep 30, 2024
@Urgau
Copy link
Member Author

Urgau commented Sep 30, 2024

@bors r=@jieyouxu rollup

@bors
Copy link
Collaborator

bors commented Sep 30, 2024

📌 Commit 9cb540a has been approved by jieyouxu

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 Sep 30, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request Sep 30, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 30, 2024
@bors
Auto merge of rust-lang#131063 - matthiaskrgr:rollup-hfs3fo1, r=matth…
c3ce4e6 
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#130895 (make type-check-4 asm tests about non-const expressions)
 - rust-lang#131057 (Reject leading unsafe in `cfg!(...)` and `--check-cfg`)
 - rust-lang#131060 (Drop conditionally applied cargo `-Zon-broken-pipe=kill` flags to fix stage 1 cargo rebuilds)
 - rust-lang#131061 (replace manual verbose checks with `Config::is_verbose`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 90fdb11 into rust-lang:master Sep 30, 2024
@rustbot rustbot added this to the 1.83.0 milestone Sep 30, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 30, 2024
@rust-timer
Unrolled build for rust-lang#131057
9b1a46c 
Rollup merge of rust-lang#131057 - Urgau:cfg-erronous-unsafe, r=jieyouxu

Reject leading unsafe in `cfg!(...)` and `--check-cfg`

This PR reject leading unsafe in `cfg!(...)` and `--check-cfg`.

Fixes (after-backport) rust-lang#131055
r? `@jieyouxu`
@apiraino
Copy link
Contributor

apiraino commented Oct 3, 2024

Beta backport accepted as per compiler team on Zulip. A backport PR will be authored by the release team at the end of the current development cycle.

@rustbot label +beta-accepted

@rustbot rustbot added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Oct 3, 2024
@cuviper cuviper mentioned this pull request Oct 9, 2024
Merged
@cuviper cuviper modified the milestones: 1.83.0, 1.82.0 Oct 9, 2024
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 9, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 11, 2024
@bors
Auto merge of rust-lang#131423 - cuviper:beta-next, r=cuviper
ba9dd66 
[beta] backports

- Only add an automatic SONAME for Rust dylibs rust-lang#130960
- Reject leading unsafe in `cfg!(...)` and `--check-cfg` rust-lang#131057, resolving rust-lang#131055
- Disable jump threading `UnOp::Not` for non-bool rust-lang#131201
- Update LLVM submodule rust-lang#131448

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 11, 2024
@bors
Auto merge of rust-lang#131423 - cuviper:beta-next, r=cuviper
c120d96 
[beta] backports

- Only add an automatic SONAME for Rust dylibs rust-lang#130960
- Reject leading unsafe in `cfg!(...)` and `--check-cfg` rust-lang#131057, resolving rust-lang#131055
- Disable jump threading `UnOp::Not` for non-bool rust-lang#131201
- Update LLVM submodule rust-lang#131448

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 12, 2024
@bors
Auto merge of rust-lang#131423 - cuviper:beta-next, r=cuviper
0986126 
[beta] backports

- Only add an automatic SONAME for Rust dylibs rust-lang#130960
- Reject leading unsafe in `cfg!(...)` and `--check-cfg` rust-lang#131057, resolving rust-lang#131055
- Disable jump threading `UnOp::Not` for non-bool rust-lang#131201
- Update LLVM submodule rust-lang#131448

r? ghost
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 12, 2024
@bors
Auto merge of rust-lang#131423 - cuviper:beta-next, r=cuviper
763ad52 
[beta] backports

- Only add an automatic SONAME for Rust dylibs rust-lang#130960
- Reject leading unsafe in `cfg!(...)` and `--check-cfg` rust-lang#131057, resolving rust-lang#131055
- Disable jump threading `UnOp::Not` for non-bool rust-lang#131201
- Update LLVM submodule rust-lang#131448
- Split x86_64-msvc-ext into two jobs rust-lang#130072
- Use a small runner for msvc-ext2 job rust-lang#130151

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

Reviewers

@jieyouxu jieyouxu jieyouxu approved these changes

Assignees

@jieyouxu jieyouxu

Labels

beta-accepted Accepted for backporting to the compiler in the beta channel. 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

Milestone

1.82.0

Development

Successfully merging this pull request may close these issues.

6 participants

@Urgau @jieyouxu @bors @apiraino @cuviper @rustbot