[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240904204347.168520-19-ojeda@kernel.org>
Date: Wed, 4 Sep 2024 22:43:46 +0200
From: Miguel Ojeda <ojeda@...nel.org>
To: Miguel Ojeda <ojeda@...nel.org>,
Alex Gaynor <alex.gaynor@...il.com>,
Wedson Almeida Filho <wedsonaf@...il.com>
Cc: Boqun Feng <boqun.feng@...il.com>,
Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <benno.lossin@...ton.me>,
Andreas Hindborg <a.hindborg@...sung.com>,
Alice Ryhl <aliceryhl@...gle.com>,
Trevor Gross <tmgross@...ch.edu>,
rust-for-linux@...r.kernel.org,
linux-kernel@...r.kernel.org,
patches@...ts.linux.dev
Subject: [PATCH 18/19] Documentation: rust: discuss `#[expect(...)]` in the guidelines
Discuss `#[expect(...)]` in the Lints sections of the coding guidelines
document, which is an upcoming feature in Rust 1.81.0, and explain that
it is generally to be preferred over `allow` unless there is a reason
not to use it (e.g. conditional compilation being involved).
Signed-off-by: Miguel Ojeda <ojeda@...nel.org>
---
Documentation/rust/coding-guidelines.rst | 110 +++++++++++++++++++++++
1 file changed, 110 insertions(+)
diff --git a/Documentation/rust/coding-guidelines.rst b/Documentation/rust/coding-guidelines.rst
index 185d3b51117d..5d64bc69acae 100644
--- a/Documentation/rust/coding-guidelines.rst
+++ b/Documentation/rust/coding-guidelines.rst
@@ -256,3 +256,113 @@ By that virtue, it makes it possible to comfortably enable more diagnostics by
default (i.e. outside ``W=`` levels). In particular, those that may have some
false positives but that are otherwise quite useful to keep enabled to catch
potential mistakes.
+
+On top of that, Rust provides the ``expect`` attribute which takes this further.
+It makes the compiler warn if the warning was not produced. For instance, the
+following will ensure that, when ``f()`` is called somewhere, we will have to
+remove the attribute:
+
+.. code-block:: rust
+
+ #[expect(dead_code)]
+ fn f() {}
+
+If we do not, we get a warning from the compiler::
+
+ warning: this lint expectation is unfulfilled
+ --> x.rs:3:10
+ |
+ 3 | #[expect(dead_code)]
+ | ^^^^^^^^^
+ |
+ = note: `#[warn(unfulfilled_lint_expectations)]` on by default
+
+This means that ``expect``\ s do not get forgotten when they are not needed, which
+may happen in several situations, e.g.:
+
+- Temporary attributes added while developing.
+
+- Improvements in lints in the compiler, Clippy or custom tools which may
+ remove a false positive.
+
+- When the lint is not needed anymore because it was expected that it would be
+ removed at some point, such as the ``dead_code`` example above.
+
+It also increases the visibility of the remaining ``allow``\ s and reduces the
+chance of misapplying one.
+
+Thus prefer ``except`` over ``allow`` unless:
+
+- The lint attribute is intended to be temporary, e.g. while developing.
+
+- Conditional compilation triggers the warning in some cases but not others.
+
+ If there are only a few cases where the warning triggers (or does not
+ trigger) compared to the total number of cases, then one may consider using
+ a conditional ``expect`` (i.e. ``cfg_attr(..., expect(...))``). Otherwise,
+ it is likely simpler to just use ``allow``.
+
+- Inside macros, when the different invocations may create expanded code that
+ triggers the warning in some cases but not in others.
+
+- When code may trigger a warning for some architectures but not others, such
+ as an ``as`` cast to a C FFI type.
+
+As a more developed example, consider for instance this program:
+
+.. code-block:: rust
+
+ fn g() {}
+
+ fn main() {
+ #[cfg(CONFIG_X)]
+ g();
+ }
+
+Here, function ``g()`` is dead code if ``CONFIG_X`` is not set. Can we use
+``expect`` here?
+
+.. code-block:: rust
+
+ #[expect(dead_code)]
+ fn g() {}
+
+ fn main() {
+ #[cfg(CONFIG_X)]
+ g();
+ }
+
+This would emit a lint if ``CONFIG_X`` is set, since it is not dead code in that
+configuration. Therefore, in cases like this, we cannot use ``expect`` as-is.
+
+A simple possibility is using ``allow``:
+
+.. code-block:: rust
+
+ #[allow(dead_code)]
+ fn g() {}
+
+ fn main() {
+ #[cfg(CONFIG_X)]
+ g();
+ }
+
+An alternative would be using a conditional ``expect``:
+
+.. code-block:: rust
+
+ #[cfg_attr(not(CONFIG_X), expect(dead_code))]
+ fn g() {}
+
+ fn main() {
+ #[cfg(CONFIG_X)]
+ g();
+ }
+
+This would ensure that, if someone introduces another call to ``g()`` somewhere
+(e.g. unconditionally), then it would be spotted that it is not dead code
+anymore. However, the ``cfg_attr`` is more complex than a simple ``allow``.
+
+Therefore, it is likely that it is not worth using conditional ``expect``\ s when
+more than one or two configurations are involved or when the lint may be
+triggered due to non-local changes (such as ``dead_code``).
--
2.46.0
Powered by blists - more mailing lists