All of lore.kernel.org
 help / color / mirror / Atom feed
From: Miguel Ojeda <ojeda@kernel.org>
To: Miguel Ojeda <ojeda@kernel.org>,
	Alex Gaynor <alex.gaynor@gmail.com>,
	Wedson Almeida Filho <wedsonaf@gmail.com>
Cc: "Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <benno.lossin@proton.me>,
	"Andreas Hindborg" <a.hindborg@samsung.com>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	rust-for-linux@vger.kernel.org, linux-kernel@vger.kernel.org,
	patches@lists.linux.dev
Subject: [PATCH 18/19] Documentation: rust: discuss `#[expect(...)]` in the guidelines
Date: Wed,  4 Sep 2024 22:43:46 +0200	[thread overview]
Message-ID: <20240904204347.168520-19-ojeda@kernel.org> (raw)
In-Reply-To: <20240904204347.168520-1-ojeda@kernel.org>

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@kernel.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


  parent reply	other threads:[~2024-09-04 20:45 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-04 20:43 [PATCH 00/19] rust: lint improvements Miguel Ojeda
2024-09-04 20:43 ` [PATCH 01/19] rust: workqueue: remove unneeded ``#[allow(clippy::new_ret_no_self)]` Miguel Ojeda
2024-09-05  7:57   ` Alice Ryhl
2024-09-29  4:17   ` Trevor Gross
2024-09-04 20:43 ` [PATCH 02/19] rust: sort global Rust flags Miguel Ojeda
2024-09-05  7:57   ` Alice Ryhl
2024-09-29  4:18   ` Trevor Gross
2024-09-04 20:43 ` [PATCH 03/19] rust: types: avoid repetition in `{As,From}Bytes` impls Miguel Ojeda
2024-09-05  7:58   ` Alice Ryhl
2024-09-29  4:20   ` Trevor Gross
2024-09-04 20:43 ` [PATCH 04/19] rust: enable `clippy::undocumented_unsafe_blocks` lint Miguel Ojeda
2024-09-05  8:04   ` Alice Ryhl
2024-09-29  4:33   ` Trevor Gross
2024-10-03 18:45     ` Miguel Ojeda
2024-09-04 20:43 ` [PATCH 05/19] rust: enable `clippy::unnecessary_safety_comment` lint Miguel Ojeda
2024-09-05  8:07   ` Alice Ryhl
2024-09-08  7:38   ` Vincenzo Palazzo
2024-09-29  4:35   ` Trevor Gross
2024-09-04 20:43 ` [PATCH 06/19] rust: enable `clippy::unnecessary_safety_doc` lint Miguel Ojeda
2024-09-05  8:07   ` Alice Ryhl
2024-09-29  4:35   ` Trevor Gross
2024-09-04 20:43 ` [PATCH 07/19] rust: enable `clippy::ignored_unit_patterns` lint Miguel Ojeda
2024-09-05  8:08   ` Alice Ryhl
2024-09-29  4:37   ` Trevor Gross
2024-09-04 20:43 ` [PATCH 08/19] rust: enable `rustdoc::unescaped_backticks` lint Miguel Ojeda
2024-09-05  8:09   ` Alice Ryhl
2024-09-29  4:40   ` Trevor Gross
2024-10-01 17:06     ` Miguel Ojeda
2024-09-04 20:43 ` [PATCH 09/19] rust: init: remove unneeded `#[allow(clippy::disallowed_names)]` Miguel Ojeda
2024-09-05  8:10   ` Alice Ryhl
2024-09-29  4:41   ` Trevor Gross
2024-09-04 20:43 ` [PATCH 10/19] rust: sync: remove unneeded `#[allow(clippy::non_send_fields_in_send_ty)]` Miguel Ojeda
2024-09-05  8:10   ` Alice Ryhl
2024-09-29  4:41   ` Trevor Gross
2024-09-04 20:43 ` [PATCH 11/19] rust: introduce `.clippy.toml` Miguel Ojeda
2024-09-05  8:12   ` Alice Ryhl
2024-09-29  4:48   ` Trevor Gross
2024-09-04 20:43 ` [PATCH 12/19] rust: replace `clippy::dbg_macro` with `disallowed_macros` Miguel Ojeda
2024-09-05  5:20   ` Geert Stappers
2024-09-14 23:10     ` Gary Guo
2024-10-03 18:43       ` Miguel Ojeda
2024-09-04 20:43 ` [PATCH 13/19] rust: rbtree: fix `SAFETY` comments that should be `# Safety` sections Miguel Ojeda
2024-09-05  8:12   ` Alice Ryhl
2024-09-29  4:51   ` Trevor Gross
2024-09-04 20:43 ` [PATCH 14/19] rust: provide proper code documentation titles Miguel Ojeda
2024-09-05  8:13   ` Alice Ryhl
2024-09-29  4:56   ` Trevor Gross
2024-10-03 18:44     ` Miguel Ojeda
2024-09-04 20:43 ` [PATCH 15/19] rust: enable Clippy's `check-private-items` Miguel Ojeda
2024-09-05  8:14   ` Alice Ryhl
2024-09-29  4:57   ` Trevor Gross
2024-09-04 20:43 ` [PATCH 16/19] Documentation: rust: add coding guidelines on lints Miguel Ojeda
2024-09-05  8:15   ` Alice Ryhl
2024-09-05  9:45     ` Miguel Ojeda
2024-09-07 22:22       ` comex
2024-10-01 17:07         ` Miguel Ojeda
2024-10-03 20:00       ` Miguel Ojeda
2024-09-29  5:03   ` Trevor Gross
2024-10-03 18:47     ` Miguel Ojeda
2024-09-04 20:43 ` [PATCH 17/19] rust: start using the `#[expect(...)]` attribute Miguel Ojeda
2024-09-05  8:17   ` Alice Ryhl
2024-09-29  5:05   ` Trevor Gross
2024-09-04 20:43 ` Miguel Ojeda [this message]
2024-09-29  5:10   ` [PATCH 18/19] Documentation: rust: discuss `#[expect(...)]` in the guidelines Trevor Gross
2024-10-01 17:11     ` Miguel Ojeda
2024-09-04 20:43 ` [PATCH 19/19] rust: std_vendor: simplify `{ .. macro! .. }` with inner attributes Miguel Ojeda
2024-09-05  8:19   ` Alice Ryhl
2024-09-05  9:27     ` Miguel Ojeda
2024-09-05  9:41       ` Alice Ryhl
2024-09-08  8:50   ` Vincenzo Palazzo
2024-09-14 23:19 ` [PATCH 00/19] rust: lint improvements Gary Guo
2024-10-03 21:51 ` Miguel Ojeda

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240904204347.168520-19-ojeda@kernel.org \
    --to=ojeda@kernel.org \
    --cc=a.hindborg@samsung.com \
    --cc=alex.gaynor@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=benno.lossin@proton.me \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=gary@garyguo.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tmgross@umich.edu \
    --cc=wedsonaf@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.