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

RFC: Checking conditional compilation at compile time #3013

Merged
merged 12 commits into from
Feb 23, 2021

Conversation

sivadeilra
Copy link
Contributor

@sivadeilra sivadeilra commented Nov 4, 2020

This proposes a minor new feature for rustc and cargo, which will allow rustc to detect
invalid usage of #[cfg] conditions, such as misspelling a "feature" name.

The feature is purely opt-in. If the user doesn't enable this feature, then there is no impact on anything.
If the user does enable this feature, then the only possible impact is that rustc fires new diagnostics.
Language semantics, codegen, etc. are all unchanged.

Rendered

@bjorn3
Copy link
Member

bjorn3 commented Nov 4, 2020

How will this handle build scripts? Those can set arbitrary cfgs without any statically known list on which they could set. These are for example used to detect features supported by the used rustc version.

@sivadeilra
Copy link
Contributor Author

sivadeilra commented Nov 4, 2020

I think there's two parts to it: 1) What rustc does, and 2) what build tools do.

This RFC covers what rustc does, and you get a lot of control over what forms of checking are turned on (and which are left turned off).

For Cargo, I think the best approach would be to enable checking of feature values, and initially nothing else. Since most crates use feature for conditional compilation, and not boolean --cfg values, this will cover most cases.

However, even crates that use build.rs and --cfg names other than feature could still use this feature. The build.rs script would emit --cfg valid(my_flag) in addition to (optionally) emitting --cfg my_flag. So rustc could still validate the source code, and the build.rs script could still control compilation.

edit: I could add a section clarifying the impact on build.rs scripts, if you think that would help

@jyn514
Copy link
Member

jyn514 commented Nov 4, 2020

However, even crates that use build.rs and --cfg names other than feature could still use this feature. The build.rs script would emit --cfg valid(my_flag) in addition to (optionally) emitting --cfg my_flag. So rustc could still validate the source code, and the build.rs script could still control compilation.

What happens if the build script forgets to do this (as will be the case for all proc-macros just after this is implemented)? Will cargo still warn about the missing cfg? That doesn't seem ideal.

@jyn514
Copy link
Member

jyn514 commented Nov 4, 2020

This also doesn't seem to consider that you can set --cfg in arbitrary ways, for example it's common to set --cfg docsrs from the command line when compiling for docs.rs: https://github.com/tokio-rs/tracing/blob/master/tracing-core/Cargo.toml#L39-L41

Copy link
Member

@jyn514 jyn514 left a comment

Choose a reason for hiding this comment

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

I think valid_values is the most useful part of this; tracking other arbitrary cfgs seems really hard.

text/0000-conditional-compilation-checking.md Outdated Show resolved Hide resolved
text/0000-conditional-compilation-checking.md Outdated Show resolved Hide resolved
text/0000-conditional-compilation-checking.md Outdated Show resolved Hide resolved
To catch this error, we give `rustc` the set of valid condition names:

```bash
rustc --cfg 'valid(name1, name2, ..., nameN)' ...
Copy link
Member

Choose a reason for hiding this comment

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

What happens if you pass a cfg that's not marked as valid? I think you shouldn't need to specify anything that's already being passed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Passing --cfg foo implicitly means that foo is valid. So --cfg foo --cfg 'valid(foo)' is legal, but is redundant.

Comment on lines 270 to 271
> 2. After stabilization, users can opt-in to conditional checking by using `cargo build --check-cfg`.
> 3. Eventually, if the community experience is positive, Cargo could enable this check by default.
Copy link
Member

Choose a reason for hiding this comment

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

I think cargo should unconditionally provide the information to rustc, and then you can configure the lint with normal lints flags (allow, deny, -D).

text/0000-conditional-compilation-checking.md Outdated Show resolved Hide resolved
@sivadeilra
Copy link
Contributor Author

@jyn512 writes:

What happens if the build script forgets to do this (as will be the case for all proc-macros just after this is implemented)? Will cargo still warn about the missing cfg? That doesn't seem ideal.

Nothing will happen, because Cargo will not enable checking the set of all --cfg names, only the set of values specified for feature. Cargo would specify --cfg valid_values(feature, "foo", "bar", ...), but by default would not specify --cfg valid(...).

Or, we could have Cargo change its behavior depending on whether there is a build.rs file or not. If there is no build.rs file, then Cargo could also enable checking cfg names (not just feature values, but cfg names). For crates where a build.rs file is used, we could disable this by default to prevent any breaks. Telemetry (like Crater runs) could then be used to find where this has any negative effect. If it's a reasonably small number of crates, I could work with the crate owners so that they could add the right --cfg valid(...) options to cover their cases. Then, this could be turned on by default, even for crates that have build.rs scripts.

This also doesn't seem to consider that you can set --cfg in arbitrary ways [...]

Good to know; I wasn't aware of docsrs.

I think this just emphasizes that it's good to split this into two parts: the mechanisms (which go in rustc) and the policy (when to use it, and when not to use it). Maybe the best policy to start with (post-stabilization) would be:

  1. The code is present in rustc, but doesn't do anything until you opt-in.
  2. Cargo always specifies valid_values(feature, ...) so that feature values are checked, but nothing else is checked.
  3. A new Cargo.toml property controls whether checking is turned on for a given package (or crate within a package).
  4. We iterate on these use cases (such as building docs, build.rs scripts, etc.) to find where this can be applied.

The goal is to make life better for devs, not worse, of course. :) I think getting the mechanisms into the compiler, and then experimenting with how to turn them on/off, would be a good first step.

For example, `rustc` can detect this bug, where the `test` condition is misspelled as `tset`:

```rust
if cfg!(tset) { // uh oh, should have been 'test'
Copy link

@Julian-Wollersberger Julian-Wollersberger Nov 4, 2020

Choose a reason for hiding this comment

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

A future possibility would be to check for simple misspellings for the well-known condition names, even when -cfg valid(...) isn't used.

I guess this would catch most of the bugs this RFC wants to detect, but avoids the problem of false warnings caused by unknown -cfgs from build scripts.
Implementation wise, maybe the logic from some diagnostics can be reused, like help: a config with a similar name exists: 'test'

Copy link

@Julian-Wollersberger Julian-Wollersberger left a comment

Choose a reason for hiding this comment

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

The feature proposed here feels very much desireable to me and actually I'm surprised that rustc doesn't already do this.
A warning for a wrong cfg falls in the caregory of "Thank you compiler, you just safed me from a weird bug." :)

But the CLI syntax should be simplified:

rustc --cfg 'valid(name1, name2, ..., nameN)' ...
rustc --cfg 'valid_values(c, "value1", "value2", ... "valueN")' ...
  • There will surely be an environment that messes up the quotes or parentheses.
  • The format is inconsistent with other rustc arguments, which are like --arg=val1,val2,val3.
  • These are not really --cfg arguments, so they should have their own --whatever flags.

Alternatives that spring to my mind:

rustc --valid_cfgs=name1,name2,nameN
# Not shure how the handle this "arg with a list" situation
rustc --valid_values_for=cond1=value1,value2,valueN
# Special-casing features might be worth it if it's the only thing that will be implemented
rustc --valid_features=feature1,feature2,featureN 

Co-authored-by: Joshua Nelson <joshua@yottadb.com>
@sivadeilra
Copy link
Contributor Author

But the CLI syntax should be simplified: [...]

I like that. I'll update the doc.

Added new --check-cfg flag. Rather than changing the behavior of
the existing --cfg flag, we enable validation by using --check-cfg.

Clarified some special cases, like empty valid name sets.
@sivadeilra
Copy link
Contributor Author

I've updated the RFC. This moves the command-line options to a new --check-cfg option; the original --cfg is now unchanged. I've also clarified a few corner cases in the spec, like how to specify empty sets of valid names but still turn on checking.

I have a prototype implementation: https://github.com/sivadeilra/rust/tree/user/ardavis/check_cfg

This prototype is NOT ready for a thorough review, but it does implement the RFC.

@Havvy
Copy link
Contributor

Havvy commented Nov 12, 2020

You can probably link to the reference on conditional compilation for well-known values.

As for drawbacks, it's more surface area. You're adding in command line arguments merely for a lint. And those command line arguments take up a lot of space that detracts from the arguments that actually do something.

@kennytm kennytm changed the title User/ardavis/checked cfg RFC: Checking conditional compilation at compile time Nov 12, 2020
@sivadeilra
Copy link
Contributor Author

You can probably link to the reference on conditional compilation for well-known values.

As for drawbacks, it's more surface area. You're adding in command line arguments merely for a lint. And those command line arguments take up a lot of space that detracts from the arguments that actually do something.

What would you suggest?

I originally wrote the spec in such a way that the existing --cfg option was used for this information, and just expanded the set of syntactic forms that it would accept.

detracts from the arguments that actually do something

I'm going to push back on this. This "does something" in the same sense that the -A, -D, -W, and -F args do something. They are providing control over diagnostics. And in the case of --check-cfg, they are providing a form of static checking that we did not have before. "Clutter" is subjective; there is no ambiguity about what --check-cfg does, nor does it interfere with parsing other arguments.

@Havvy
Copy link
Contributor

Havvy commented Nov 12, 2020

I'm not suggesting anything to fix it. Merely to document it. Drawbacks are inherent in almost any proposal and they should be documented in the RFC.

Usually, when I see a drawbacks section that says "None", it signals to me that the author hasn't thought hard enough about what the tradeoffs they are making actually are. I just posted the first drawback I could think of, but there are probably others, though nothing RFC-stopping to me.

@sivadeilra
Copy link
Contributor Author

I've updated the drawbacks section.

@sivadeilra
Copy link
Contributor Author

Hey folks, what's next? I think I've addressed all of the feedback.

@jyn514
Copy link
Member

jyn514 commented Nov 16, 2020

I think @scottmcm or another member of lang-team needs to start an FCP.

@scottmcm scottmcm added I-nominated T-lang Relevant to the language team, which will review and decide on the RFC. T-cargo Relevant to the Cargo team, which will review and decide on the RFC. labels Nov 18, 2020
@scottmcm scottmcm added T-compiler Relevant to the compiler team, which will review and decide on the RFC. and removed T-lang Relevant to the language team, which will review and decide on the RFC. labels Dec 1, 2020
@ehuss ehuss merged commit 3f1b129 into rust-lang:master Feb 23, 2021
@ehuss
Copy link
Contributor

ehuss commented Feb 23, 2021

Huzzah! The @rust-lang/compiler team has decided to accept this RFC.

To track further discussion, subscribe to the tracking issue here:
rust-lang/rust#82450

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 18, 2022
Implement --check-cfg option (RFC 3013), take 2

This pull-request implement RFC 3013: Checking conditional compilation at compile time (rust-lang/rfcs#3013) and is based on the previous attempt rust-lang#89346 by `@mwkmwkmwk` that was closed due to inactivity.

I have address all the review comments from the previous attempt and added some more tests.

cc rust-lang#82450
r? `@petrochenkov`
bors added a commit to rust-lang/cargo that referenced this pull request Apr 12, 2022
Add support for rustc --check-cfg well known names and values

This pull-request add support for `rustc` `--check-cfg` well known names and values.

### What does this PR try to resolve?

This pull-request add support for `rustc` `--check-cfg` well known names and values:

- `-Z check-cfg-well-known-names`: `--check-cfg names()` well known names
- `-Z check-cfg-well-known-values`: `--check-cfg values()` well known values

### How should we test and review this PR?

#### Testing

`cargo check -Z check-cfg-well-known-names` and
`cargo check -Z check-cfg-well-known-values`

#### Review

This PR contains one commit that split `features_args` into `check_cfg_args`, add the well known support in it, adds call to that new function and add documentation and test for those new flags.

### Additional information

This was implemented as two new additional flags because it's most likely that these flags will not be stabilize at the same time and also because some of these flags may become the default.

RFC: rust-lang/rfcs#3013
`-Z check-cfg-features`: #10408
`cargo doc` support: #10428
@Urgau
Copy link
Member

Urgau commented Feb 9, 2024

Call for Testing

This is in preparation for stabilizing cargo check -Zcheck-cfg as always-on in Cargo.

RFC 3013 adds validation to #[cfg] conditionals to help catch typos, stale conditionals, etc.

Note: the implementaion has drasticly changed since the RFC approval, please always refer to the cargo unstable docs and rustc unstable docs.

For cargo users

Testing instructions

Requires: nightly-2024-02-06 or newer

  1. On your projects, run cargo +nightly check -Zcheck-cfg
  2. Fix typos / mistakes
  3. Mark custom --cfgs as expected via build.rs (see docs)

Note: docs.rs now (automaticly) provides --cfg docsrs.

Feedback

We are looking for any feedback:

  • Where did the feature not work as expected?
  • How often did you end up having to use a build.rs directive?
    • Were there surprising cases?
    • How well can you model your needs with the check-cfg syntax
  • Any concerns with -Zcheck-cfg being the default?
  • Any concerns with -Zcheck-cfg being stabilized as-is?
  • How many typos did you have? ;-)

You can leave feedback on the Cargo tracking issue.

For maintainers of alternative build systems

Testing instructions

Look over rustc --check-cfg and evaluate how well it would integrate with your build system.
If you can prototype it out, even better!

Feedback

We are looking for any feedback:

  • Does the syntax meet your needs?
  • Did the documentation anwser all your questions?
  • Do you think you will have trouble making use of it?

You can leave feedback on the Rust tracking issue.


And thanks to @epage for this help in preparing this Call for Testing as well as his (and the Cargo team) support in helping move this feature forward.

@epage epage added the call-for-testing Add this label + test notes in comments to be included in TWiR `Call for Testing` section. label Feb 9, 2024
@sivadeilra
Copy link
Contributor Author

I'm the original author of the RFC for this feature. I want to thank @Urgau and @mwkmwkmwk for driving this work through its completion! I spent the time to write the original RFC and to "sell" the idea, but unfortunately other work required my focus. I am thankful that @mwkmwkmwk picked up this work and pushed it (the first cycle), and that @Urgau repeated that exercise and took it further, all the way to completion.

Thank you both! I'm looking forward to being able to test this and use it, myself.

@U007D U007D removed the call-for-testing Add this label + test notes in comments to be included in TWiR `Call for Testing` section. label Feb 13, 2024
@U007D
Copy link

U007D commented Feb 13, 2024

Hello,

This RFC will appear in the Call for Testing section of This Week in Rust # 534. I've removed the call-for-testing label. Please feel free to re-add it if you wish this RFC to appear in a future edition of TWiR.

fbq pushed a commit to Rust-for-Linux/linux that referenced this pull request Feb 17, 2024
This is the next upgrade to the Rust toolchain, from 1.76.0 to 1.77.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

The `offset_of` feature (single-field `offset_of!`) that we were using
got stabilized in Rust 1.77.0 [3].

Therefore, now the only unstable features allowed to be used outside the
`kernel` crate is `new_uninit`, though other code to be upstreamed may
increase the list.

Please see [4] for details.

# Required changes

Rust 1.77.0 merged the `unused_tuple_struct_fields` lint into `dead_code`,
thus upgrading it from `allow` to `warn` [5]. In turn, this makes `rustc`
complain about the `ThisModule`'s pointer field being never read. Thus
locally `allow` it for the moment, since we will have users later on
(e.g. Binder needs a `as_ptr` method [6]).

# Other changes

Rust 1.77.0 introduces the `--check-cfg` feature [7], for which there
is a Call for Testing going on [8]. We were requested to test it and
we found it useful [9] -- we will likely enable it in the future.

# `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-1770-2024-03-21 [1]
Link: https://rust-for-linux.com/rust-version-policy [2]
Link: rust-lang/rust#118799 [3]
Link: #2 [4]
Link: rust-lang/rust#118297 [5]
Link: https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-2-08ba9197f637@google.com/#Z31rust:kernel:lib.rs [6]
Link: https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html [7]
Link: rust-lang/rfcs#3013 (comment) [8]
Link: rust-lang/rust#82450 (comment) [9]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://lore.kernel.org/r/20240217002717.57507-1-ojeda@kernel.org
fbq pushed a commit to Rust-for-Linux/linux that referenced this pull request Feb 19, 2024
This is the next upgrade to the Rust toolchain, from 1.76.0 to 1.77.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").

The `offset_of` feature (single-field `offset_of!`) that we were using
got stabilized in Rust 1.77.0 [3].

Therefore, now the only unstable features allowed to be used outside the
`kernel` crate is `new_uninit`, though other code to be upstreamed may
increase the list.

Please see [4] for details.

Rust 1.77.0 merged the `unused_tuple_struct_fields` lint into `dead_code`,
thus upgrading it from `allow` to `warn` [5]. In turn, this makes `rustc`
complain about the `ThisModule`'s pointer field being never read. Thus
locally `allow` it for the moment, since we will have users later on
(e.g. Binder needs a `as_ptr` method [6]).

Rust 1.77.0 introduces the `--check-cfg` feature [7], for which there
is a Call for Testing going on [8]. We were requested to test it and
we found it useful [9] -- we will likely enable it in the future.

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-1770-2024-03-21 [1]
Link: https://rust-for-linux.com/rust-version-policy [2]
Link: rust-lang/rust#118799 [3]
Link: #2 [4]
Link: rust-lang/rust#118297 [5]
Link: https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-2-08ba9197f637@google.com/#Z31rust:kernel:lib.rs [6]
Link: https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html [7]
Link: rust-lang/rfcs#3013 (comment) [8]
Link: rust-lang/rust#82450 (comment) [9]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://lore.kernel.org/r/20240217002717.57507-1-ojeda@kernel.org
fbq pushed a commit to Rust-for-Linux/linux that referenced this pull request Mar 5, 2024
This is the next upgrade to the Rust toolchain, from 1.76.0 to 1.77.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").

The `offset_of` feature (single-field `offset_of!`) that we were using
got stabilized in Rust 1.77.0 [3].

Therefore, now the only unstable features allowed to be used outside the
`kernel` crate is `new_uninit`, though other code to be upstreamed may
increase the list.

Please see [4] for details.

Rust 1.77.0 merged the `unused_tuple_struct_fields` lint into `dead_code`,
thus upgrading it from `allow` to `warn` [5]. In turn, this makes `rustc`
complain about the `ThisModule`'s pointer field being never read. Thus
locally `allow` it for the moment, since we will have users later on
(e.g. Binder needs a `as_ptr` method [6]).

Rust 1.77.0 introduces the `--check-cfg` feature [7], for which there
is a Call for Testing going on [8]. We were requested to test it and
we found it useful [9] -- we will likely enable it in the future.

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-1770-2024-03-21 [1]
Link: https://rust-for-linux.com/rust-version-policy [2]
Link: rust-lang/rust#118799 [3]
Link: #2 [4]
Link: rust-lang/rust#118297 [5]
Link: https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-2-08ba9197f637@google.com/#Z31rust:kernel:lib.rs [6]
Link: https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html [7]
Link: rust-lang/rfcs#3013 (comment) [8]
Link: rust-lang/rust#82450 (comment) [9]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://lore.kernel.org/r/20240217002717.57507-1-ojeda@kernel.org
fbq pushed a commit to Rust-for-Linux/linux that referenced this pull request Mar 25, 2024
This is the next upgrade to the Rust toolchain, from 1.76.0 to 1.77.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").

The `offset_of` feature (single-field `offset_of!`) that we were using
got stabilized in Rust 1.77.0 [3].

Therefore, now the only unstable features allowed to be used outside the
`kernel` crate is `new_uninit`, though other code to be upstreamed may
increase the list.

Please see [4] for details.

Rust 1.77.0 merged the `unused_tuple_struct_fields` lint into `dead_code`,
thus upgrading it from `allow` to `warn` [5]. In turn, this makes `rustc`
complain about the `ThisModule`'s pointer field being never read. Thus
locally `allow` it for the moment, since we will have users later on
(e.g. Binder needs a `as_ptr` method [6]).

Rust 1.77.0 introduces the `--check-cfg` feature [7], for which there
is a Call for Testing going on [8]. We were requested to test it and
we found it useful [9] -- we will likely enable it in the future.

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-1770-2024-03-21 [1]
Link: https://rust-for-linux.com/rust-version-policy [2]
Link: rust-lang/rust#118799 [3]
Link: #2 [4]
Link: rust-lang/rust#118297 [5]
Link: https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-2-08ba9197f637@google.com/#Z31rust:kernel:lib.rs [6]
Link: https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html [7]
Link: rust-lang/rfcs#3013 (comment) [8]
Link: rust-lang/rust#82450 (comment) [9]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://lore.kernel.org/r/20240217002717.57507-1-ojeda@kernel.org
fbq pushed a commit to Rust-for-Linux/linux that referenced this pull request Mar 29, 2024
This is the next upgrade to the Rust toolchain, from 1.76.0 to 1.77.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").

The `offset_of` feature (single-field `offset_of!`) that we were using
got stabilized in Rust 1.77.0 [3].

Therefore, now the only unstable features allowed to be used outside the
`kernel` crate is `new_uninit`, though other code to be upstreamed may
increase the list.

Please see [4] for details.

Rust 1.77.0 merged the `unused_tuple_struct_fields` lint into `dead_code`,
thus upgrading it from `allow` to `warn` [5]. In turn, this makes `rustc`
complain about the `ThisModule`'s pointer field being never read. Thus
locally `allow` it for the moment, since we will have users later on
(e.g. Binder needs a `as_ptr` method [6]).

Rust 1.77.0 introduces the `--check-cfg` feature [7], for which there
is a Call for Testing going on [8]. We were requested to test it and
we found it useful [9] -- we will likely enable it in the future.

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-1770-2024-03-21 [1]
Link: https://rust-for-linux.com/rust-version-policy [2]
Link: rust-lang/rust#118799 [3]
Link: #2 [4]
Link: rust-lang/rust#118297 [5]
Link: https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-2-08ba9197f637@google.com/#Z31rust:kernel:lib.rs [6]
Link: https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html [7]
Link: rust-lang/rfcs#3013 (comment) [8]
Link: rust-lang/rust#82450 (comment) [9]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Link: https://lore.kernel.org/r/20240217002717.57507-1-ojeda@kernel.org
[boqun: Resolve the conflicts with using upstream alloc]
ojeda added a commit to ojeda/linux that referenced this pull request Mar 29, 2024
This is the next upgrade to the Rust toolchain, from 1.76.0 to 1.77.1
(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

The `offset_of` feature (single-field `offset_of!`) that we were using
got stabilized in Rust 1.77.0 [3].

Therefore, now the only unstable features allowed to be used outside the
`kernel` crate is `new_uninit`, though other code to be upstreamed may
increase the list.

Please see [4] for details.

# Required changes

Rust 1.77.0 merged the `unused_tuple_struct_fields` lint into `dead_code`,
thus upgrading it from `allow` to `warn` [5]. In turn, this made `rustc`
complain about the `ThisModule`'s pointer field being never read, but
the previous patch adds the `as_ptr` method to it, needed by Binder [6],
so that we do not need to locally `allow` it.

# Other changes

Rust 1.77.0 introduces the `--check-cfg` feature [7], for which there
is a Call for Testing going on [8]. We were requested to test it and
we found it useful [9] -- we will likely enable it in the future.

# `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-1770-2024-03-21 [1]
Link: https://rust-for-linux.com/rust-version-policy [2]
Link: rust-lang/rust#118799 [3]
Link: Rust-for-Linux#2 [4]
Link: rust-lang/rust#118297 [5]
Link: https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-2-08ba9197f637@google.com/#Z31rust:kernel:lib.rs [6]
Link: https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html [7]
Link: rust-lang/rfcs#3013 (comment) [8]
Link: rust-lang/rust#82450 (comment) [9]
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Tested-by: Boqun Feng <boqun.feng@gmail.com>
Link: https://lore.kernel.org/r/20240217002717.57507-1-ojeda@kernel.org
[ Upgraded to 1.77.1. No changed to `alloc` during the beta. ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
ojeda added a commit to Rust-for-Linux/linux that referenced this pull request Mar 29, 2024
This is the next upgrade to the Rust toolchain, from 1.76.0 to 1.77.1
(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

The `offset_of` feature (single-field `offset_of!`) that we were using
got stabilized in Rust 1.77.0 [3].

Therefore, now the only unstable features allowed to be used outside the
`kernel` crate is `new_uninit`, though other code to be upstreamed may
increase the list.

Please see [4] for details.

# Required changes

Rust 1.77.0 merged the `unused_tuple_struct_fields` lint into `dead_code`,
thus upgrading it from `allow` to `warn` [5]. In turn, this made `rustc`
complain about the `ThisModule`'s pointer field being never read, but
the previous patch adds the `as_ptr` method to it, needed by Binder [6],
so that we do not need to locally `allow` it.

# Other changes

Rust 1.77.0 introduces the `--check-cfg` feature [7], for which there
is a Call for Testing going on [8]. We were requested to test it and
we found it useful [9] -- we will likely enable it in the future.

# `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-1770-2024-03-21 [1]
Link: https://rust-for-linux.com/rust-version-policy [2]
Link: rust-lang/rust#118799 [3]
Link: #2 [4]
Link: rust-lang/rust#118297 [5]
Link: https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-2-08ba9197f637@google.com/#Z31rust:kernel:lib.rs [6]
Link: https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html [7]
Link: rust-lang/rfcs#3013 (comment) [8]
Link: rust-lang/rust#82450 (comment) [9]
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Tested-by: Boqun Feng <boqun.feng@gmail.com>
Link: https://lore.kernel.org/r/20240217002717.57507-1-ojeda@kernel.org
[ Upgraded to 1.77.1. Removed `allow(dead_code)` thanks to the previous
  patch. Reworded accordingly. No changes to `alloc` during the beta. ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
sweettea pushed a commit to sweettea/btrfs-fscrypt that referenced this pull request Apr 3, 2024
This is the next upgrade to the Rust toolchain, from 1.76.0 to 1.77.1
(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

The `offset_of` feature (single-field `offset_of!`) that we were using
got stabilized in Rust 1.77.0 [3].

Therefore, now the only unstable features allowed to be used outside the
`kernel` crate is `new_uninit`, though other code to be upstreamed may
increase the list.

Please see [4] for details.

# Required changes

Rust 1.77.0 merged the `unused_tuple_struct_fields` lint into `dead_code`,
thus upgrading it from `allow` to `warn` [5]. In turn, this made `rustc`
complain about the `ThisModule`'s pointer field being never read, but
the previous patch adds the `as_ptr` method to it, needed by Binder [6],
so that we do not need to locally `allow` it.

# Other changes

Rust 1.77.0 introduces the `--check-cfg` feature [7], for which there
is a Call for Testing going on [8]. We were requested to test it and
we found it useful [9] -- we will likely enable it in the future.

# `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-1770-2024-03-21 [1]
Link: https://rust-for-linux.com/rust-version-policy [2]
Link: rust-lang/rust#118799 [3]
Link: Rust-for-Linux#2 [4]
Link: rust-lang/rust#118297 [5]
Link: https://lore.kernel.org/rust-for-linux/20231101-rust-binder-v1-2-08ba9197f637@google.com/#Z31rust:kernel:lib.rs [6]
Link: https://doc.rust-lang.org/nightly/unstable-book/compiler-flags/check-cfg.html [7]
Link: rust-lang/rfcs#3013 (comment) [8]
Link: rust-lang/rust#82450 (comment) [9]
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Tested-by: Boqun Feng <boqun.feng@gmail.com>
Link: https://lore.kernel.org/r/20240217002717.57507-1-ojeda@kernel.org
[ Upgraded to 1.77.1. Removed `allow(dead_code)` thanks to the previous
  patch. Reworded accordingly. No changes to `alloc` during the beta. ]
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this RFC. T-cargo Relevant to the Cargo team, which will review and decide on the RFC. T-compiler Relevant to the compiler team, which will review and decide on the RFC. to-announce
Projects
No open projects
Status: Implemented
Development

Successfully merging this pull request may close these issues.