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 NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE a deny-by-default lint in edition 2024 #126881

Conversation

WaffleLapkin
Copy link
Member

@WaffleLapkin WaffleLapkin commented Jun 23, 2024

I don't actually really care about this, but @traviscross asked me to do this, because lang team briefly discussed this before.

(TC here:)

Specifically, our original FCPed plan included this step:

  • Add a lint against fallback affecting a generic that is passed to an unsafe function.
    • Perhaps make this lint deny-by-default or a hard error in Rust 2024.

That is, we had left as an open question strengthening this in Rust 2024, and had marked it as an open question on the tracking issue. We're nominating here to address the open question. (Closing the remaining open question helps us to fully mark this off for Rust 2024.)

r? @compiler-errors

Tracking:

@WaffleLapkin WaffleLapkin added the I-lang-nominated Nominated for discussion during a lang team meeting. label Jun 23, 2024
@rustbot
Copy link
Collaborator

rustbot commented Jun 23, 2024

r? @jieyouxu

rustbot has assigned @jieyouxu.
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 Jun 23, 2024
@rustbot rustbot assigned compiler-errors and unassigned jieyouxu Jun 23, 2024
@traviscross
Copy link
Contributor

@rustbot labels -I-lang-nominated

We discussed this in the lang call today. We decided that we do want to strengthen this lint in Rust 2024, but to deny-by-default rather than to a hard error.

The criteria we used here is that, while we don't want people to lean on the behavior against which we're linting, there are a number of aspects of this lint that are a bit heuristic (i.e., we can't say definitively that UB is happening), and we could want some room to maneuver or adjust here, and keeping it as a lint (at deny-by-default) does that.

We also discussed how, in deciding between deny-by-default and a hard error, we might look at whether making something a hard error makes the definition of the language simpler in a new edition, and this probably doesn't rise to that level.

We had everyone there, and received unanimous consent, so if we can adjust this PR to make this existing lint deny-by-default in Rust 2024, this will be good to go as far as lang is concerned.

@rustbot rustbot removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Jun 26, 2024
@WaffleLapkin WaffleLapkin force-pushed the unsafe-code-affected-by-fallback-hard-in-2024 branch from 639d406 to 18c248b Compare June 28, 2024 20:31
@WaffleLapkin WaffleLapkin changed the title Make NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE a hard error in edition 2024 Make NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE a deny-by-default lint in edition 2024 Jun 28, 2024
@WaffleLapkin
Copy link
Member Author

Changed the PR to be a deny-by-default lint instead of a hard error, as T-lang decided 👍🏻

LL | unsafe { mem::zeroed() }
| ^^^^^^^^^^^^^
|
= warning: this will change its meaning in a future release!
Copy link
Member Author

Choose a reason for hiding this comment

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

this... is a bit weird.

for e2024, the behavior won't change, so this is misleading...

@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jul 7, 2024

📌 Commit 18c248b 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 Jul 7, 2024
@compiler-errors
Copy link
Member

@bors rollup

jieyouxu added a commit to jieyouxu/rust that referenced this pull request Jul 8, 2024
…by-fallback-hard-in-2024, r=compiler-errors

Make `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` a deny-by-default lint in edition 2024

I don't actually really care about this, but `@traviscross` asked me to do this, because lang team briefly discussed this before.

(TC here:)

Specifically, our original FCPed plan included this step:

- Add a lint against fallback affecting a generic that is passed to an `unsafe` function.
   - Perhaps make this lint `deny-by-default` or a hard error in Rust 2024.

That is, we had left as an open question strengthening this in Rust 2024, and had marked it as an open question on the tracking issue.  We're nominating here to address the open question.  (Closing the remaining open question helps us to fully mark this off for Rust 2024.)

r? `@compiler-errors`

Tracking:

- rust-lang#123748
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 8, 2024
Rollup of 12 pull requests

Successful merges:

 - rust-lang#113128 (Support tail calls in mir via `TerminatorKind::TailCall`)
 - rust-lang#126841 ([`macro_metavar_expr_concat`] Add support for literals)
 - rust-lang#126881 (Make `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` a deny-by-default lint in edition 2024)
 - rust-lang#126921 (Give VaList its own home)
 - rust-lang#127276 (rustdoc: Remove OpaqueTy)
 - rust-lang#127367 (Run alloc sync tests)
 - rust-lang#127431 (Use field ident spans directly instead of the full field span in diagnostics on local fields)
 - rust-lang#127437 (Uplift trait ref is knowable into `rustc_next_trait_solver`)
 - rust-lang#127439 (Uplift elaboration into `rustc_type_ir`)
 - rust-lang#127451 (Improve `run-make/output-type-permutations` code and improve `filename_not_in_denylist` API)
 - rust-lang#127452 (Fix intrinsic const parameter counting with `effects`)
 - rust-lang#127459 (rustdoc-json: add type/trait alias tests)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 8, 2024
Rollup of 10 pull requests

Successful merges:

 - rust-lang#126841 ([`macro_metavar_expr_concat`] Add support for literals)
 - rust-lang#126881 (Make `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` a deny-by-default lint in edition 2024)
 - rust-lang#126921 (Give VaList its own home)
 - rust-lang#127367 (Run alloc sync tests)
 - rust-lang#127431 (Use field ident spans directly instead of the full field span in diagnostics on local fields)
 - rust-lang#127437 (Uplift trait ref is knowable into `rustc_next_trait_solver`)
 - rust-lang#127439 (Uplift elaboration into `rustc_type_ir`)
 - rust-lang#127451 (Improve `run-make/output-type-permutations` code and improve `filename_not_in_denylist` API)
 - rust-lang#127452 (Fix intrinsic const parameter counting with `effects`)
 - rust-lang#127459 (rustdoc-json: add type/trait alias tests)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 29c1a43 into rust-lang:master Jul 8, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jul 8, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jul 8, 2024
Rollup merge of rust-lang#126881 - WaffleLapkin:unsafe-code-affected-by-fallback-hard-in-2024, r=compiler-errors

Make `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` a deny-by-default lint in edition 2024

I don't actually really care about this, but ``@traviscross`` asked me to do this, because lang team briefly discussed this before.

(TC here:)

Specifically, our original FCPed plan included this step:

- Add a lint against fallback affecting a generic that is passed to an `unsafe` function.
   - Perhaps make this lint `deny-by-default` or a hard error in Rust 2024.

That is, we had left as an open question strengthening this in Rust 2024, and had marked it as an open question on the tracking issue.  We're nominating here to address the open question.  (Closing the remaining open question helps us to fully mark this off for Rust 2024.)

r? ``@compiler-errors``

Tracking:

- rust-lang#123748
@WaffleLapkin WaffleLapkin deleted the unsafe-code-affected-by-fallback-hard-in-2024 branch July 8, 2024 10:44
flip1995 pushed a commit to flip1995/rust that referenced this pull request Jul 11, 2024
Rollup of 10 pull requests

Successful merges:

 - rust-lang#126841 ([`macro_metavar_expr_concat`] Add support for literals)
 - rust-lang#126881 (Make `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` a deny-by-default lint in edition 2024)
 - rust-lang#126921 (Give VaList its own home)
 - rust-lang#127367 (Run alloc sync tests)
 - rust-lang#127431 (Use field ident spans directly instead of the full field span in diagnostics on local fields)
 - rust-lang#127437 (Uplift trait ref is knowable into `rustc_next_trait_solver`)
 - rust-lang#127439 (Uplift elaboration into `rustc_type_ir`)
 - rust-lang#127451 (Improve `run-make/output-type-permutations` code and improve `filename_not_in_denylist` API)
 - rust-lang#127452 (Fix intrinsic const parameter counting with `effects`)
 - rust-lang#127459 (rustdoc-json: add type/trait alias tests)

r? `@ghost`
`@rustbot` modify labels: rollup
facebook-github-bot pushed a commit to facebook/buck2 that referenced this pull request Oct 7, 2024
Summary:
Upgrading Rust from `1.80.1` to `1.81.0` ([release notes](https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html)) with the following changes affecting `fbsource`:
* Feature stabilizations:
  * `io_slice_advance` ([#62726](rust-lang/rust#62726))
  * `panic_info_message` ([#66745](rust-lang/rust#66745))
  * `fs_try_exists` ([#83186](rust-lang/rust#83186))
  * `lint_reasons` ([#120924](rust-lang/rust#120924))
  * `duration_abs_diff` ([#117618](rust-lang/rust#117618))
  * `effects` ([#102090](rust-lang/rust#102090))
* New `clippy` lints:
  * `manual_inspect` ([#12287](rust-lang/rust-clippy#12287))
  * `manual_pattern_char_comparison` ([#12849](rust-lang/rust-clippy#12849))
* Other:
  * `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` became a deny-by-default lint ([#126881](rust-lang/rust#126881) & [#123748](rust-lang/rust#123748))
  * Changes to `stringify!` introducing whitespaces between some tokens ([#125174](rust-lang/rust#125174))
  * `format!` is now marked with a `must_use` hint ([#127355](rust-lang/rust#127355))

ignore-conflict-markers

Reviewed By: zertosh, dtolnay

Differential Revision: D63864870

fbshipit-source-id: c8d61f3e9483ec709c8116514cfb030c883ec262
facebook-github-bot pushed a commit to facebookexperimental/rust-shed that referenced this pull request Oct 7, 2024
Summary:
Upgrading Rust from `1.80.1` to `1.81.0` ([release notes](https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html)) with the following changes affecting `fbsource`:
* Feature stabilizations:
  * `io_slice_advance` ([#62726](rust-lang/rust#62726))
  * `panic_info_message` ([#66745](rust-lang/rust#66745))
  * `fs_try_exists` ([#83186](rust-lang/rust#83186))
  * `lint_reasons` ([#120924](rust-lang/rust#120924))
  * `duration_abs_diff` ([#117618](rust-lang/rust#117618))
  * `effects` ([#102090](rust-lang/rust#102090))
* New `clippy` lints:
  * `manual_inspect` ([#12287](rust-lang/rust-clippy#12287))
  * `manual_pattern_char_comparison` ([#12849](rust-lang/rust-clippy#12849))
* Other:
  * `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` became a deny-by-default lint ([#126881](rust-lang/rust#126881) & [#123748](rust-lang/rust#123748))
  * Changes to `stringify!` introducing whitespaces between some tokens ([#125174](rust-lang/rust#125174))
  * `format!` is now marked with a `must_use` hint ([#127355](rust-lang/rust#127355))

ignore-conflict-markers

Reviewed By: zertosh, dtolnay

Differential Revision: D63864870

fbshipit-source-id: c8d61f3e9483ec709c8116514cfb030c883ec262
facebook-github-bot pushed a commit to facebookincubator/buck2-change-detector that referenced this pull request Oct 7, 2024
Summary:
Upgrading Rust from `1.80.1` to `1.81.0` ([release notes](https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html)) with the following changes affecting `fbsource`:
* Feature stabilizations:
  * `io_slice_advance` ([#62726](rust-lang/rust#62726))
  * `panic_info_message` ([#66745](rust-lang/rust#66745))
  * `fs_try_exists` ([#83186](rust-lang/rust#83186))
  * `lint_reasons` ([#120924](rust-lang/rust#120924))
  * `duration_abs_diff` ([#117618](rust-lang/rust#117618))
  * `effects` ([#102090](rust-lang/rust#102090))
* New `clippy` lints:
  * `manual_inspect` ([#12287](rust-lang/rust-clippy#12287))
  * `manual_pattern_char_comparison` ([#12849](rust-lang/rust-clippy#12849))
* Other:
  * `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` became a deny-by-default lint ([#126881](rust-lang/rust#126881) & [#123748](rust-lang/rust#123748))
  * Changes to `stringify!` introducing whitespaces between some tokens ([#125174](rust-lang/rust#125174))
  * `format!` is now marked with a `must_use` hint ([#127355](rust-lang/rust#127355))

ignore-conflict-markers

Reviewed By: zertosh, dtolnay

Differential Revision: D63864870

fbshipit-source-id: c8d61f3e9483ec709c8116514cfb030c883ec262
facebook-github-bot pushed a commit to facebook/starlark-rust that referenced this pull request Oct 7, 2024
Summary:
Upgrading Rust from `1.80.1` to `1.81.0` ([release notes](https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html)) with the following changes affecting `fbsource`:
* Feature stabilizations:
  * `io_slice_advance` ([#62726](rust-lang/rust#62726))
  * `panic_info_message` ([#66745](rust-lang/rust#66745))
  * `fs_try_exists` ([#83186](rust-lang/rust#83186))
  * `lint_reasons` ([#120924](rust-lang/rust#120924))
  * `duration_abs_diff` ([#117618](rust-lang/rust#117618))
  * `effects` ([#102090](rust-lang/rust#102090))
* New `clippy` lints:
  * `manual_inspect` ([#12287](rust-lang/rust-clippy#12287))
  * `manual_pattern_char_comparison` ([#12849](rust-lang/rust-clippy#12849))
* Other:
  * `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` became a deny-by-default lint ([#126881](rust-lang/rust#126881) & [#123748](rust-lang/rust#123748))
  * Changes to `stringify!` introducing whitespaces between some tokens ([#125174](rust-lang/rust#125174))
  * `format!` is now marked with a `must_use` hint ([#127355](rust-lang/rust#127355))

ignore-conflict-markers

Reviewed By: zertosh, dtolnay

Differential Revision: D63864870

fbshipit-source-id: c8d61f3e9483ec709c8116514cfb030c883ec262
facebook-github-bot pushed a commit to facebookincubator/reindeer that referenced this pull request Oct 7, 2024
Summary:
Upgrading Rust from `1.80.1` to `1.81.0` ([release notes](https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html)) with the following changes affecting `fbsource`:
* Feature stabilizations:
  * `io_slice_advance` ([#62726](rust-lang/rust#62726))
  * `panic_info_message` ([#66745](rust-lang/rust#66745))
  * `fs_try_exists` ([#83186](rust-lang/rust#83186))
  * `lint_reasons` ([#120924](rust-lang/rust#120924))
  * `duration_abs_diff` ([#117618](rust-lang/rust#117618))
  * `effects` ([#102090](rust-lang/rust#102090))
* New `clippy` lints:
  * `manual_inspect` ([#12287](rust-lang/rust-clippy#12287))
  * `manual_pattern_char_comparison` ([#12849](rust-lang/rust-clippy#12849))
* Other:
  * `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` became a deny-by-default lint ([#126881](rust-lang/rust#126881) & [#123748](rust-lang/rust#123748))
  * Changes to `stringify!` introducing whitespaces between some tokens ([#125174](rust-lang/rust#125174))
  * `format!` is now marked with a `must_use` hint ([#127355](rust-lang/rust#127355))

ignore-conflict-markers

Reviewed By: zertosh, dtolnay

Differential Revision: D63864870

fbshipit-source-id: c8d61f3e9483ec709c8116514cfb030c883ec262
facebook-github-bot pushed a commit to facebookexperimental/allocative that referenced this pull request Oct 7, 2024
Summary:
Upgrading Rust from `1.80.1` to `1.81.0` ([release notes](https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html)) with the following changes affecting `fbsource`:
* Feature stabilizations:
  * `io_slice_advance` ([#62726](rust-lang/rust#62726))
  * `panic_info_message` ([#66745](rust-lang/rust#66745))
  * `fs_try_exists` ([#83186](rust-lang/rust#83186))
  * `lint_reasons` ([#120924](rust-lang/rust#120924))
  * `duration_abs_diff` ([#117618](rust-lang/rust#117618))
  * `effects` ([#102090](rust-lang/rust#102090))
* New `clippy` lints:
  * `manual_inspect` ([#12287](rust-lang/rust-clippy#12287))
  * `manual_pattern_char_comparison` ([#12849](rust-lang/rust-clippy#12849))
* Other:
  * `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` became a deny-by-default lint ([#126881](rust-lang/rust#126881) & [#123748](rust-lang/rust#123748))
  * Changes to `stringify!` introducing whitespaces between some tokens ([#125174](rust-lang/rust#125174))
  * `format!` is now marked with a `must_use` hint ([#127355](rust-lang/rust#127355))

ignore-conflict-markers

Reviewed By: zertosh, dtolnay

Differential Revision: D63864870

fbshipit-source-id: c8d61f3e9483ec709c8116514cfb030c883ec262
facebook-github-bot pushed a commit to facebook/hhvm that referenced this pull request Oct 7, 2024
Summary:
Upgrading Rust from `1.80.1` to `1.81.0` ([release notes](https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html)) with the following changes affecting `fbsource`:
* Feature stabilizations:
  * `io_slice_advance` ([#62726](rust-lang/rust#62726))
  * `panic_info_message` ([#66745](rust-lang/rust#66745))
  * `fs_try_exists` ([#83186](rust-lang/rust#83186))
  * `lint_reasons` ([#120924](rust-lang/rust#120924))
  * `duration_abs_diff` ([#117618](rust-lang/rust#117618))
  * `effects` ([#102090](rust-lang/rust#102090))
* New `clippy` lints:
  * `manual_inspect` ([#12287](rust-lang/rust-clippy#12287))
  * `manual_pattern_char_comparison` ([#12849](rust-lang/rust-clippy#12849))
* Other:
  * `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` became a deny-by-default lint ([#126881](rust-lang/rust#126881) & [#123748](rust-lang/rust#123748))
  * Changes to `stringify!` introducing whitespaces between some tokens ([#125174](rust-lang/rust#125174))
  * `format!` is now marked with a `must_use` hint ([#127355](rust-lang/rust#127355))

ignore-conflict-markers

Reviewed By: zertosh, dtolnay

Differential Revision: D63864870

fbshipit-source-id: c8d61f3e9483ec709c8116514cfb030c883ec262
facebook-github-bot pushed a commit to facebookincubator/gazebo that referenced this pull request Oct 7, 2024
Summary:
Upgrading Rust from `1.80.1` to `1.81.0` ([release notes](https://blog.rust-lang.org/2024/09/05/Rust-1.81.0.html)) with the following changes affecting `fbsource`:
* Feature stabilizations:
  * `io_slice_advance` ([#62726](rust-lang/rust#62726))
  * `panic_info_message` ([#66745](rust-lang/rust#66745))
  * `fs_try_exists` ([#83186](rust-lang/rust#83186))
  * `lint_reasons` ([#120924](rust-lang/rust#120924))
  * `duration_abs_diff` ([#117618](rust-lang/rust#117618))
  * `effects` ([#102090](rust-lang/rust#102090))
* New `clippy` lints:
  * `manual_inspect` ([#12287](rust-lang/rust-clippy#12287))
  * `manual_pattern_char_comparison` ([#12849](rust-lang/rust-clippy#12849))
* Other:
  * `NEVER_TYPE_FALLBACK_FLOWING_INTO_UNSAFE` became a deny-by-default lint ([#126881](rust-lang/rust#126881) & [#123748](rust-lang/rust#123748))
  * Changes to `stringify!` introducing whitespaces between some tokens ([#125174](rust-lang/rust#125174))
  * `format!` is now marked with a `must_use` hint ([#127355](rust-lang/rust#127355))

ignore-conflict-markers

Reviewed By: zertosh, dtolnay

Differential Revision: D63864870

fbshipit-source-id: c8d61f3e9483ec709c8116514cfb030c883ec262
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants