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

make writes_through_immutable_pointer a hard error #129199

Merged
merged 1 commit into from
Aug 25, 2024

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Aug 17, 2024

This turns the lint added in #118324 into a hard error. This has been reported in cargo's future-compat reports since Rust 1.76 (released in February). Given that const_mut_refs is still unstable, it should be impossible to even hit this error on stable: we did accidentally stabilize some functions that can cause this error, but that got reverted in #117905. Still, let's do a crater run just to be sure.

Given that this should only affect unstable code, I don't think it needs an FCP, but let's Cc @rust-lang/lang anyway -- any objection to making this unambiguous UB into a hard error during const-eval? This can be viewed as part of #129195 which is already nominated for discussion.

@rustbot
Copy link
Collaborator

rustbot commented Aug 17, 2024

r? @fmease

rustbot has assigned @fmease.
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 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 Aug 17, 2024
@RalfJung
Copy link
Member Author

@bors try

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 17, 2024
…inter, r=<try>

make writes_through_immutable_pointer a hard error

This turns the lint added in rust-lang#118324 into a hard error. This has been reported in cargo's future-compat reports since Rust 1.76 (released in February). Given that const_mut_refs is still unstable, it should be impossible to even hit this error on stable: we did accidentally stabilize some functions that can cause this error, but that got reverted in rust-lang#117905. Still, let's do a crater run just to be sure.

Given that this should only affect unstable code, I don't think it needs an FCP, but let's Cc `@rust-lang/lang` anyway -- any objection to making this unambiguous UB into a hard error during const-eval? This can be viewed as part of rust-lang#129195 which is already nominated for discussion.
@bors
Copy link
Contributor

bors commented Aug 17, 2024

⌛ Trying commit 8b642a1 with merge 2c70eb4...

@jieyouxu jieyouxu added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Aug 17, 2024
@bors
Copy link
Contributor

bors commented Aug 17, 2024

☀️ Try build successful - checks-actions
Build commit: 2c70eb4 (2c70eb47850052381670935f807d41647a7dc992)

@RalfJung
Copy link
Member Author

@craterbot check

@craterbot
Copy link
Collaborator

👌 Experiment pr-129199 created and queued.
🤖 Automatically detected try build 2c70eb4
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 17, 2024
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

i'm happy to approve this if crater results are unremarkable. I agree that if the lint is truly unreachable on stable, then it doesn't need to be FCP'd.

@craterbot
Copy link
Collaborator

🚧 Experiment pr-129199 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-129199 is completed!
📊 24 regressed and 4 fixed (501799 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-crater Status: Waiting on a crater run to be completed. labels Aug 24, 2024
@RalfJung
Copy link
Member Author

We have one

  • error: lint writes_through_immutable_pointer has been removed: converted into hard error

That's on them for adding `deny(renamed_and_removed_lints), they are basically asking to have breaking changes. 🤷

The rest seems spurious. :)

@bors r=compiler-errors

@bors
Copy link
Contributor

bors commented Aug 24, 2024

📌 Commit 8b642a1 has been approved by compiler-errors

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 Aug 24, 2024
@RalfJung
Copy link
Member Author

Let's try rerunning crater to see if the strange spurious errors reproduce.

@craterbot run mode=check-only crates=https://crater-reports.s3.amazonaws.com/pr-129199/retry-regressed-list.txt p=1

@craterbot
Copy link
Collaborator

👌 Experiment pr-129199-1 created and queued.
🤖 Automatically detected try build 2c70eb4
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 24, 2024
@RalfJung RalfJung 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-crater Status: Waiting on a crater run to be completed. labels Aug 24, 2024
@craterbot
Copy link
Collaborator

🚧 Experiment pr-129199-1 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot
Copy link
Collaborator

🎉 Experiment pr-129199-1 is completed!
📊 1 regressed and 0 fixed (24 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 24, 2024
@RalfJung RalfJung 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 Aug 24, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 24, 2024
…pointer, r=compiler-errors

make writes_through_immutable_pointer a hard error

This turns the lint added in rust-lang#118324 into a hard error. This has been reported in cargo's future-compat reports since Rust 1.76 (released in February). Given that const_mut_refs is still unstable, it should be impossible to even hit this error on stable: we did accidentally stabilize some functions that can cause this error, but that got reverted in rust-lang#117905. Still, let's do a crater run just to be sure.

Given that this should only affect unstable code, I don't think it needs an FCP, but let's Cc `@rust-lang/lang` anyway -- any objection to making this unambiguous UB into a hard error during const-eval? This can be viewed as part of rust-lang#129195 which is already nominated for discussion.
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 24, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#128596 (stabilize const_fn_floating_point_arithmetic)
 - rust-lang#129199 (make writes_through_immutable_pointer a hard error)
 - rust-lang#129246 (Retroactively feature gate `ConstArgKind::Path`)
 - rust-lang#129290 (Pin `cc` to 1.0.105)
 - rust-lang#129323 (Implement `ptr::fn_addr_eq`)
 - rust-lang#129500 (remove invalid `TyCompat` relation for effects)
 - rust-lang#129501 (panicking: improve hint for Miri's RUST_BACKTRACE behavior)
 - rust-lang#129505 (interpret: ImmTy: tighten sanity checks in offset logic)
 - rust-lang#129509 (Add a hack to workaround MSVC CI issues)
 - rust-lang#129510 (Fix `elided_named_lifetimes` in code)

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

Rollup of 9 pull requests

Successful merges:

 - rust-lang#128596 (stabilize const_fn_floating_point_arithmetic)
 - rust-lang#129199 (make writes_through_immutable_pointer a hard error)
 - rust-lang#129246 (Retroactively feature gate `ConstArgKind::Path`)
 - rust-lang#129290 (Pin `cc` to 1.0.105)
 - rust-lang#129323 (Implement `ptr::fn_addr_eq`)
 - rust-lang#129500 (remove invalid `TyCompat` relation for effects)
 - rust-lang#129501 (panicking: improve hint for Miri's RUST_BACKTRACE behavior)
 - rust-lang#129505 (interpret: ImmTy: tighten sanity checks in offset logic)
 - rust-lang#129510 (Fix `elided_named_lifetimes` in code)

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

Rollup of 9 pull requests

Successful merges:

 - rust-lang#128596 (stabilize const_fn_floating_point_arithmetic)
 - rust-lang#129199 (make writes_through_immutable_pointer a hard error)
 - rust-lang#129246 (Retroactively feature gate `ConstArgKind::Path`)
 - rust-lang#129290 (Pin `cc` to 1.0.105)
 - rust-lang#129323 (Implement `ptr::fn_addr_eq`)
 - rust-lang#129500 (remove invalid `TyCompat` relation for effects)
 - rust-lang#129501 (panicking: improve hint for Miri's RUST_BACKTRACE behavior)
 - rust-lang#129505 (interpret: ImmTy: tighten sanity checks in offset logic)
 - rust-lang#129510 (Fix `elided_named_lifetimes` in code)

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

Rollup of 9 pull requests

Successful merges:

 - rust-lang#128596 (stabilize const_fn_floating_point_arithmetic)
 - rust-lang#129199 (make writes_through_immutable_pointer a hard error)
 - rust-lang#129246 (Retroactively feature gate `ConstArgKind::Path`)
 - rust-lang#129290 (Pin `cc` to 1.0.105)
 - rust-lang#129323 (Implement `ptr::fn_addr_eq`)
 - rust-lang#129500 (remove invalid `TyCompat` relation for effects)
 - rust-lang#129501 (panicking: improve hint for Miri's RUST_BACKTRACE behavior)
 - rust-lang#129505 (interpret: ImmTy: tighten sanity checks in offset logic)
 - rust-lang#129510 (Fix `elided_named_lifetimes` in code)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 05b8bcc into rust-lang:master Aug 25, 2024
7 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 25, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 25, 2024
Rollup merge of rust-lang#129199 - RalfJung:writes_through_immutable_pointer, r=compiler-errors

make writes_through_immutable_pointer a hard error

This turns the lint added in rust-lang#118324 into a hard error. This has been reported in cargo's future-compat reports since Rust 1.76 (released in February). Given that const_mut_refs is still unstable, it should be impossible to even hit this error on stable: we did accidentally stabilize some functions that can cause this error, but that got reverted in rust-lang#117905. Still, let's do a crater run just to be sure.

Given that this should only affect unstable code, I don't think it needs an FCP, but let's Cc ``@rust-lang/lang`` anyway -- any objection to making this unambiguous UB into a hard error during const-eval? This can be viewed as part of rust-lang#129195 which is already nominated for discussion.
@matthiaskrgr
Copy link
Member

completed jobs 0/0
1 regressed 24 total

fascinating ... 🤔

image

@RalfJung RalfJung deleted the writes_through_immutable_pointer branch August 25, 2024 11:13
@RalfJung
Copy link
Member Author

https://crater-reports.s3.amazonaws.com/pr-129199-1/full.html shows there were 24 crates tested... so probably just a bug in the queue display?

Cc @Mark-Simulacrum

@Mark-Simulacrum
Copy link
Member

After completion we clean up the database entries, so 0/0 is expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants