Add `internal_features` lint for internal unstable features #596

Noratrieb · 2023-02-28T19:20:54Z

Proposal

There are a lot of nightly features and it's difficult to set expectations for nightly users. We have some way of telling users about broken features (like generic_const_exprs) via the incomplete_features lint, but no way of warning people about using internal features. lang_items and let_chains are very different and it might not be obvious for a user that they are. let_chains are close to stabilization and it's reasonable for a nightly user to make use of them. lang_items are a perma unstable implementation detail that should never be used in code unless the user really knows that they want to.

Therefore I propose adding a new internal_features (the name is bikesheddable) lint, similar to incomplete_features. All features that are perma unstable (this includes lang_items, rustc_attrs, unsafe_pin_internals and more) fall under this lint. Just like with incomplete_features, Rusts soundness guarantees might not strictly hold when using internal_features (although we should of course still attempt to keep them as sound as possible for maintainability).

With this lint people can use nightly features but still get the assurance that they are only testing new features, not relying on implementation details.

The lint would be deny-by-default to require explicit opt-in.

Mentors or Reviewers

The implementation should be pretty simple as it can basically copy incomplete_features.

The most interesting work is determining which features fall under this lint. Some are obvious, some might be less obvious. It probably makes sense to ask the lang team about the feature on uncertainty.

Process

The main points of the Major Change Process are as follows:

File an issue describing the proposal.
A compiler team member or contributor who is knowledgeable in the area can second by writing @rustbot second.
- Finding a "second" suffices for internal changes. If however, you are proposing a new public-facing feature, such as a -C flag, then full team check-off is required.
- Compiler team members can initiate a check-off via @rfcbot fcp merge on either the MCP or the PR.
Once an MCP is seconded, the Final Comment Period begins. If no objections are raised after 10 days, the MCP is considered approved.

You can read more about Major Change Proposals on forge.

Comments

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

The text was updated successfully, but these errors were encountered:

rustbot · 2023-02-28T19:20:57Z

This issue is not meant to be used for technical discussion. There is a Zulip stream for that. Use this issue to leave procedural comments, such as volunteering to review, indicating that you second the proposal (or third, etc), or raising a concern that you would like to be addressed.

cc @rust-lang/compiler @rust-lang/compiler-contributors

jyn514 · 2023-02-28T19:30:20Z

@rustbot second

apiraino · 2023-03-15T10:36:58Z

@rustbot label -final-comment-period +major-change-accepted

Add `internal_features` lint Implements rust-lang/compiler-team#596 Also requires some more test blessing for codegen tests etc `@jyn514` had the idea of just `allow`ing the lint by default in the test suite. I'm not sure whether this is a good idea, but it's definitely one worth considering. Additional input encouraged.

This is the next upgrade to the Rust toolchain, from 1.72.1 to 1.73.0 (i.e. the latest) [1]. See the upgrade policy [2] and the comments on the first upgrade in commit 3ed03f4 ("rust: upgrade to Rust 1.68.2"). # Unstable features No unstable features (that we use) were stabilized. Therefore, the only unstable feature allowed to be used outside the `kernel` crate is still `new_uninit`, though other code to be upstreamed may increase the list. Please see [3] for details. # Required changes For the upgrade, the following changes are required: - Allow `internal_features` for `feature(compiler_builtins)` since now Rust warns about using internal compiler and standard library features (similar to how it also warns about incomplete ones) [4]. - A cleanup for a documentation link thanks to a new `rustdoc` lint. See previous commits for details. - A need to make an intra-doc link to a macro explicit, due to a change in behavior in `rustdoc`. See previous commits for details. # `alloc` upgrade and reviewing The vast majority of changes are due to our `alloc` fork being upgraded at once. There are two kinds of changes to be aware of: the ones coming from upstream, which we should follow as closely as possible, and the updates needed in our added fallible APIs to keep them matching the newer infallible APIs coming from upstream. Instead of taking a look at the diff of this patch, an alternative approach is reviewing a diff of the changes between upstream `alloc` and the kernel's. This allows to easily inspect the kernel additions only, especially to check if the fallible methods we already have still match the infallible ones in the new version coming from upstream. Another approach is reviewing the changes introduced in the additions in the kernel fork between the two versions. This is useful to spot potentially unintended changes to our additions. To apply these approaches, one may follow steps similar to the following to generate a pair of patches that show the differences between upstream Rust and the kernel (for the subset of `alloc` we use) before and after applying this patch: # Get the difference with respect to the old version. git -C rust checkout $(linux/scripts/min-tool-version.sh rustc) git -C linux ls-tree -r --name-only HEAD -- rust/alloc | cut -d/ -f3- | grep -Fv README.md | xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH git -C linux diff --patch-with-stat --summary -R > old.patch git -C linux restore rust/alloc # Apply this patch. git -C linux am rust-upgrade.patch # Get the difference with respect to the new version. git -C rust checkout $(linux/scripts/min-tool-version.sh rustc) git -C linux ls-tree -r --name-only HEAD -- rust/alloc | cut -d/ -f3- | grep -Fv README.md | xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH git -C linux diff --patch-with-stat --summary -R > new.patch git -C linux restore rust/alloc Now one may check the `new.patch` to take a look at the additions (first approach) or at the difference between those two patches (second approach). For the latter, a side-by-side tool is recommended. Link: https://github.com/rust-lang/rust/blob/stable/RELEASES.md#version-1730-2023-10-05 [1] Link: https://rust-for-linux.com/rust-version-policy [2] Link: Rust-for-Linux#2 [3] Link: rust-lang/compiler-team#596 [4] Signed-off-by: Miguel Ojeda <[email protected]>

This is the next upgrade to the Rust toolchain, from 1.72.1 to 1.73.0 (i.e. the latest) [1]. See the upgrade policy [2] and the comments on the first upgrade in commit 3ed03f4 ("rust: upgrade to Rust 1.68.2"). # Unstable features No unstable features (that we use) were stabilized. Therefore, the only unstable feature allowed to be used outside the `kernel` crate is still `new_uninit`, though other code to be upstreamed may increase the list. Please see [3] for details. # Required changes For the upgrade, the following changes are required: - Allow `internal_features` for `feature(compiler_builtins)` since now Rust warns about using internal compiler and standard library features (similar to how it also warns about incomplete ones) [4]. - A cleanup for a documentation link thanks to a new `rustdoc` lint. See previous commits for details. - A need to make an intra-doc link to a macro explicit, due to a change in behavior in `rustdoc`. See previous commits for details. # `alloc` upgrade and reviewing The vast majority of changes are due to our `alloc` fork being upgraded at once. There are two kinds of changes to be aware of: the ones coming from upstream, which we should follow as closely as possible, and the updates needed in our added fallible APIs to keep them matching the newer infallible APIs coming from upstream. Instead of taking a look at the diff of this patch, an alternative approach is reviewing a diff of the changes between upstream `alloc` and the kernel's. This allows to easily inspect the kernel additions only, especially to check if the fallible methods we already have still match the infallible ones in the new version coming from upstream. Another approach is reviewing the changes introduced in the additions in the kernel fork between the two versions. This is useful to spot potentially unintended changes to our additions. To apply these approaches, one may follow steps similar to the following to generate a pair of patches that show the differences between upstream Rust and the kernel (for the subset of `alloc` we use) before and after applying this patch: # Get the difference with respect to the old version. git -C rust checkout $(linux/scripts/min-tool-version.sh rustc) git -C linux ls-tree -r --name-only HEAD -- rust/alloc | cut -d/ -f3- | grep -Fv README.md | xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH git -C linux diff --patch-with-stat --summary -R > old.patch git -C linux restore rust/alloc # Apply this patch. git -C linux am rust-upgrade.patch # Get the difference with respect to the new version. git -C rust checkout $(linux/scripts/min-tool-version.sh rustc) git -C linux ls-tree -r --name-only HEAD -- rust/alloc | cut -d/ -f3- | grep -Fv README.md | xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH git -C linux diff --patch-with-stat --summary -R > new.patch git -C linux restore rust/alloc Now one may check the `new.patch` to take a look at the additions (first approach) or at the difference between those two patches (second approach). For the latter, a side-by-side tool is recommended. Link: https://github.com/rust-lang/rust/blob/stable/RELEASES.md#version-1730-2023-10-05 [1] Link: https://rust-for-linux.com/rust-version-policy [2] Link: #2 [3] Link: rust-lang/compiler-team#596 [4] Reviewed-by: Martin Rodriguez Reboredo <[email protected]> Reviewed-by: Vincenzo Palazzo <[email protected]> Reviewed-by: Alice Ryhl <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]>

Add `internal_features` lint Implements rust-lang/compiler-team#596 Also requires some more test blessing for codegen tests etc `@jyn514` had the idea of just `allow`ing the lint by default in the test suite. I'm not sure whether this is a good idea, but it's definitely one worth considering. Additional input encouraged.

…s, r=wesleywiser Stop telling people to submit bugs for internal feature ICEs This keeps track of usage of internal features, and changes the message to instead tell them that using internal features is not supported. I thought about several ways to do this but now used the explicit threading of an `Arc<AtomicBool>` through `Session`. This is not exactly incremental-safe, but this is fine, as this is set during macro expansion, which is pre-incremental, and also only affects the output of ICEs, at which point incremental correctness doesn't matter much anyways. See [MCP 620.](rust-lang/compiler-team#596) ![image](https://github.com/rust-lang/rust/assets/48135649/be661f05-b78a-40a9-b01d-81ad2dbdb690)

…eywiser Stop telling people to submit bugs for internal feature ICEs This keeps track of usage of internal features, and changes the message to instead tell them that using internal features is not supported. I thought about several ways to do this but now used the explicit threading of an `Arc<AtomicBool>` through `Session`. This is not exactly incremental-safe, but this is fine, as this is set during macro expansion, which is pre-incremental, and also only affects the output of ICEs, at which point incremental correctness doesn't matter much anyways. See [MCP 620.](rust-lang/compiler-team#596) ![image](https://github.com/rust-lang/rust/assets/48135649/be661f05-b78a-40a9-b01d-81ad2dbdb690)

…nfig. [Commit 9d42b1e2683644d81e37d55468257cd612d906f6](rust-lang/rust@9d42b1e) introduced this flag as part of [the effort](rust-lang/compiler-team#596) to introduce a lint for using internal rustc features (which would also include "perma unstable" rustc features). PiperOrigin-RevId: 578909357 Change-Id: I6ddd31e961a40ce8697b1b3010758e7bff269e6c

This is the next upgrade to the Rust toolchain, from 1.72.1 to 1.73.0 (i.e. the latest) [1]. See the upgrade policy [2] and the comments on the first upgrade in commit 3ed03f4 ("rust: upgrade to Rust 1.68.2"). No unstable features (that we use) were stabilized. Therefore, the only unstable feature allowed to be used outside the `kernel` crate is still `new_uninit`, though other code to be upstreamed may increase the list. Please see [3] for details. For the upgrade, the following changes are required: - Allow `internal_features` for `feature(compiler_builtins)` since now Rust warns about using internal compiler and standard library features (similar to how it also warns about incomplete ones) [4]. - A cleanup for a documentation link thanks to a new `rustdoc` lint. See previous commits for details. - A need to make an intra-doc link to a macro explicit, due to a change in behavior in `rustdoc`. See previous commits for details. The vast majority of changes are due to our `alloc` fork being upgraded at once. There are two kinds of changes to be aware of: the ones coming from upstream, which we should follow as closely as possible, and the updates needed in our added fallible APIs to keep them matching the newer infallible APIs coming from upstream. Instead of taking a look at the diff of this patch, an alternative approach is reviewing a diff of the changes between upstream `alloc` and the kernel's. This allows to easily inspect the kernel additions only, especially to check if the fallible methods we already have still match the infallible ones in the new version coming from upstream. Another approach is reviewing the changes introduced in the additions in the kernel fork between the two versions. This is useful to spot potentially unintended changes to our additions. To apply these approaches, one may follow steps similar to the following to generate a pair of patches that show the differences between upstream Rust and the kernel (for the subset of `alloc` we use) before and after applying this patch: # Get the difference with respect to the old version. git -C rust checkout $(linux/scripts/min-tool-version.sh rustc) git -C linux ls-tree -r --name-only HEAD -- rust/alloc | cut -d/ -f3- | grep -Fv README.md | xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH git -C linux diff --patch-with-stat --summary -R > old.patch git -C linux restore rust/alloc # Apply this patch. git -C linux am rust-upgrade.patch # Get the difference with respect to the new version. git -C rust checkout $(linux/scripts/min-tool-version.sh rustc) git -C linux ls-tree -r --name-only HEAD -- rust/alloc | cut -d/ -f3- | grep -Fv README.md | xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH git -C linux diff --patch-with-stat --summary -R > new.patch git -C linux restore rust/alloc Now one may check the `new.patch` to take a look at the additions (first approach) or at the difference between those two patches (second approach). For the latter, a side-by-side tool is recommended. Link: https://github.com/rust-lang/rust/blob/stable/RELEASES.md#version-1730-2023-10-05 [1] Link: https://rust-for-linux.com/rust-version-policy [2] Link: Rust-for-Linux/linux#2 [3] Link: rust-lang/compiler-team#596 [4] Reviewed-by: Martin Rodriguez Reboredo <[email protected]> Reviewed-by: Vincenzo Palazzo <[email protected]> Reviewed-by: Alice Ryhl <[email protected]> Link: https://lore.kernel.org/r/[email protected] Change-Id: Iad84aed3c1b36f527fad52ac6681f9bc5934a76e Signed-off-by: Miguel Ojeda <[email protected]> Signed-off-by: Matthew Maurer <[email protected]> (cherry picked from commit e08ff622c91af997cb89bc47e90a1a383e938bd0)

commit e08ff62 upstream. This is the next upgrade to the Rust toolchain, from 1.72.1 to 1.73.0 (i.e. the latest) [1]. See the upgrade policy [2] and the comments on the first upgrade in commit 3ed03f4 ("rust: upgrade to Rust 1.68.2"). # Unstable features No unstable features (that we use) were stabilized. Therefore, the only unstable feature allowed to be used outside the `kernel` crate is still `new_uninit`, though other code to be upstreamed may increase the list. Please see [3] for details. # Required changes For the upgrade, the following changes are required: - Allow `internal_features` for `feature(compiler_builtins)` since now Rust warns about using internal compiler and standard library features (similar to how it also warns about incomplete ones) [4]. - A cleanup for a documentation link thanks to a new `rustdoc` lint. See previous commits for details. - A need to make an intra-doc link to a macro explicit, due to a change in behavior in `rustdoc`. See previous commits for details. # `alloc` upgrade and reviewing The vast majority of changes are due to our `alloc` fork being upgraded at once. There are two kinds of changes to be aware of: the ones coming from upstream, which we should follow as closely as possible, and the updates needed in our added fallible APIs to keep them matching the newer infallible APIs coming from upstream. Instead of taking a look at the diff of this patch, an alternative approach is reviewing a diff of the changes between upstream `alloc` and the kernel's. This allows to easily inspect the kernel additions only, especially to check if the fallible methods we already have still match the infallible ones in the new version coming from upstream. Another approach is reviewing the changes introduced in the additions in the kernel fork between the two versions. This is useful to spot potentially unintended changes to our additions. To apply these approaches, one may follow steps similar to the following to generate a pair of patches that show the differences between upstream Rust and the kernel (for the subset of `alloc` we use) before and after applying this patch: # Get the difference with respect to the old version. git -C rust checkout $(linux/scripts/min-tool-version.sh rustc) git -C linux ls-tree -r --name-only HEAD -- rust/alloc | cut -d/ -f3- | grep -Fv README.md | xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH git -C linux diff --patch-with-stat --summary -R > old.patch git -C linux restore rust/alloc # Apply this patch. git -C linux am rust-upgrade.patch # Get the difference with respect to the new version. git -C rust checkout $(linux/scripts/min-tool-version.sh rustc) git -C linux ls-tree -r --name-only HEAD -- rust/alloc | cut -d/ -f3- | grep -Fv README.md | xargs -IPATH cp rust/library/alloc/src/PATH linux/rust/alloc/PATH git -C linux diff --patch-with-stat --summary -R > new.patch git -C linux restore rust/alloc Now one may check the `new.patch` to take a look at the additions (first approach) or at the difference between those two patches (second approach). For the latter, a side-by-side tool is recommended. Link: https://github.com/rust-lang/rust/blob/stable/RELEASES.md#version-1730-2023-10-05 [1] Link: https://rust-for-linux.com/rust-version-policy [2] Link: Rust-for-Linux/linux#2 [3] Link: rust-lang/compiler-team#596 [4] Reviewed-by: Martin Rodriguez Reboredo <[email protected]> Reviewed-by: Vincenzo Palazzo <[email protected]> Reviewed-by: Alice Ryhl <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Miguel Ojeda <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>

…eywiser Stop telling people to submit bugs for internal feature ICEs This keeps track of usage of internal features, and changes the message to instead tell them that using internal features is not supported. I thought about several ways to do this but now used the explicit threading of an `Arc<AtomicBool>` through `Session`. This is not exactly incremental-safe, but this is fine, as this is set during macro expansion, which is pre-incremental, and also only affects the output of ICEs, at which point incremental correctness doesn't matter much anyways. See [MCP 620.](rust-lang/compiler-team#596) ![image](https://github.com/rust-lang/rust/assets/48135649/be661f05-b78a-40a9-b01d-81ad2dbdb690)

Noratrieb added T-compiler Add this label so rfcbot knows to poll the compiler team major-change A proposal to make a major change to rustc labels Feb 28, 2023

rustbot added the to-announce Announce this issue on triage meeting label Feb 28, 2023

rustbot added the final-comment-period The FCP has started, most (if not all) team members are in agreement label Feb 28, 2023

This was referenced Mar 9, 2023

Allow internal_features rust-lang/stdarch#1394

Merged

Add internal_features lint rust-lang/rust#108955

Merged

apiraino closed this as completed Mar 15, 2023

rustbot added major-change-accepted A major change proposal that was accepted and removed final-comment-period The FCP has started, most (if not all) team members are in agreement labels Mar 15, 2023

apiraino removed the to-announce Announce this issue on triage meeting label Mar 23, 2023

Noratrieb mentioned this issue Apr 20, 2023

Policy for ICEs on incorrect usage of internal-only features #620

Closed

3 tasks

workingjubilee mentioned this issue Jul 11, 2023

ICE: rustdoc json when #[doc(primitive)] mod name is same as primitive name. rust-lang/rust#102363

Closed

lyonbeckers mentioned this issue Aug 6, 2023

get the sdk path to find the executable on linux pd-rs/crank#45

Merged

Noratrieb mentioned this issue Oct 16, 2023

Stop telling people to submit bugs for internal feature ICEs rust-lang/rust#116818

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `internal_features` lint for internal unstable features #596

Add `internal_features` lint for internal unstable features #596

Noratrieb commented Feb 28, 2023 •

edited by WaffleLapkin

Loading

rustbot commented Feb 28, 2023

jyn514 commented Feb 28, 2023

apiraino commented Mar 15, 2023

Add internal_features lint for internal unstable features #596

Add internal_features lint for internal unstable features #596

Comments

Noratrieb commented Feb 28, 2023 • edited by WaffleLapkin Loading

Proposal

Mentors or Reviewers

Process

Comments

rustbot commented Feb 28, 2023

jyn514 commented Feb 28, 2023

apiraino commented Mar 15, 2023

Add `internal_features` lint for internal unstable features #596

Add `internal_features` lint for internal unstable features #596

Noratrieb commented Feb 28, 2023 •

edited by WaffleLapkin

Loading