In the C side, disabling diagnostics locally, i.e. within the source code,
is rare (at least in the kernel). Sometimes warnings are manipulated
via the flags at the translation unit level, but that is about it.

In Rust, it is easier to change locally the "level" of lints
(e.g. allowing them locally). In turn, this means it is easier to
globally enable more lints that may trigger a few false positives here
and there that need to be allowed ocally, but that generally can spot
issues or bugs.

Thus document this.

Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
---
 Documentation/rust/coding-guidelines.rst | 29 ++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/Documentation/rust/coding-guidelines.rst b/Documentation/rust/coding-guidelines.rst
index 05542840b16c..185d3b51117d 100644
--- a/Documentation/rust/coding-guidelines.rst
+++ b/Documentation/rust/coding-guidelines.rst
@@ -227,3 +227,32 @@ The equivalent in Rust may look like (ignoring documentation):
 That is, the equivalent of ``GPIO_LINE_DIRECTION_IN`` would be referred to as
 ``gpio::LineDirection::In``. In particular, it should not be named
 ``gpio::gpio_line_direction::GPIO_LINE_DIRECTION_IN``.
+
+
+Lints
+-----
+
+In Rust, it is possible to ``allow`` particular warnings (diagnostics, lints)
+locally, making the compiler ignore instances of a given warning within a given
+function, module, block, etc.
+
+It is similar to ``#pragma GCC diagnostic push`` + ``ignored`` + ``pop`` in C:
+
+.. code-block:: c
+
+	#pragma GCC diagnostic push
+	#pragma GCC diagnostic ignored "-Wunused-function"
+	static void f(void) {}
+	#pragma GCC diagnostic pop
+
+But way less verbose:
+
+.. code-block:: rust
+
+	#[allow(dead_code)]
+	fn f() {}
+
+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.
-- 
2.46.0

On Wed, Sep 4, 2024 at 4:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In the C side, disabling diagnostics locally, i.e. within the source code,
> is rare (at least in the kernel). Sometimes warnings are manipulated
> via the flags at the translation unit level, but that is about it.
>
> In Rust, it is easier to change locally the "level" of lints
> (e.g. allowing them locally). In turn, this means it is easier to
> globally enable more lints that may trigger a few false positives here
> and there that need to be allowed ocally, but that generally can spot
> issues or bugs.

s/ocally/locally

> Thus document this.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
> ---
>  Documentation/rust/coding-guidelines.rst | 29 ++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/Documentation/rust/coding-guidelines.rst b/Documentation/rust/coding-guidelines.rst
> index 05542840b16c..185d3b51117d 100644
> --- a/Documentation/rust/coding-guidelines.rst
> +++ b/Documentation/rust/coding-guidelines.rst
> @@ -227,3 +227,32 @@ The equivalent in Rust may look like (ignoring documentation):
>  That is, the equivalent of ``GPIO_LINE_DIRECTION_IN`` would be referred to as
>  ``gpio::LineDirection::In``. In particular, it should not be named
>  ``gpio::gpio_line_direction::GPIO_LINE_DIRECTION_IN``.
> +
> +
> +Lints
> +-----
> +
> +In Rust, it is possible to ``allow`` particular warnings (diagnostics, lints)
> +locally, making the compiler ignore instances of a given warning within a given
> +function, module, block, etc.
> +
> +It is similar to ``#pragma GCC diagnostic push`` + ``ignored`` + ``pop`` in C:
> +
> +.. code-block:: c
> +
> +       #pragma GCC diagnostic push
> +       #pragma GCC diagnostic ignored "-Wunused-function"
> +       static void f(void) {}
> +       #pragma GCC diagnostic pop
> +
> +But way less verbose:
> +
> +.. code-block:: rust
> +
> +       #[allow(dead_code)]
> +       fn f() {}
> +
> +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.

It may be good to link to the docs on this,
https://doc.rust-lang.org/stable/reference/attributes/diagnostics.html.
Either way:

Reviewed-by: Trevor Gross <tmgross@umich.edu>

On Sun, Sep 29, 2024 at 7:03 AM Trevor Gross <tmgross@umich.edu> wrote:
>
> s/ocally/locally

Fixed in the version I am applying -- thanks!

> It may be good to link to the docs on this,
> https://doc.rust-lang.org/stable/reference/attributes/diagnostics.html.

Done too!

Cheers,
Miguel

On Wed, Sep 4, 2024 at 10:45 PM Miguel Ojeda <ojeda@kernel.org> wrote:
>
> In the C side, disabling diagnostics locally, i.e. within the source code,
> is rare (at least in the kernel). Sometimes warnings are manipulated
> via the flags at the translation unit level, but that is about it.
>
> In Rust, it is easier to change locally the "level" of lints
> (e.g. allowing them locally). In turn, this means it is easier to
> globally enable more lints that may trigger a few false positives here
> and there that need to be allowed ocally, but that generally can spot
> issues or bugs.
>
> Thus document this.
>
> Signed-off-by: Miguel Ojeda <ojeda@kernel.org>

Wow, does C really not have an easier way to do it?

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

On Thu, Sep 5, 2024 at 10:16 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> Wow, does C really not have an easier way to do it?

Yeah, it would be nice to get a similar system in C.

There are targeted attributes that can annotate certain things, like
`[[maybe_unused]]` in C23 (and vendor attributes too like our
`__maybe_unused` macro), so `-Wunused-function` is not really the best
example in that sense -- I will think of a better one (it was nice to
use the same as in the other examples I wrote for `expect` later on,
which is why I used it).

But, as far as I am aware, there is no way to handle lints (and levels
and so on) in a simple and consistent way like Rust does.

Cheers,
Miguel

On Thu, Sep 5, 2024 at 11:45 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> example in that sense -- I will think of a better one (it was nice to
> use the same as in the other examples I wrote for `expect` later on,
> which is why I used it).

I added a footnote in the version I am applying to be able to keep
using the same example (lint) everywhere.

Cheers,
Miguel

> On Sep 5, 2024, at 2:45 AM, Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote:
> 
> On Thu, Sep 5, 2024 at 10:16 AM Alice Ryhl <aliceryhl@google.com> wrote:
>> 
>> Wow, does C really not have an easier way to do it?
> 
> Yeah, it would be nice to get a similar system in C.
> 
> There are targeted attributes that can annotate certain things, like
> `[[maybe_unused]]` in C23 (and vendor attributes too like our
> `__maybe_unused` macro), so `-Wunused-function` is not really the best
> example in that sense -- I will think of a better one (it was nice to
> use the same as in the other examples I wrote for `expect` later on,
> which is why I used it).
> 
> But, as far as I am aware, there is no way to handle lints (and levels
> and so on) in a simple and consistent way like Rust does.

You can always hide the pragmas behind a macro:

https://gcc.godbolt.org/z/WTEaYWW8c

It’s not perfect, because warning names sometimes differ between GCC and Clang, among other reasons.

On Sun, Sep 8, 2024 at 12:22 AM comex <comexk@gmail.com> wrote:
>
> You can always hide the pragmas behind a macro:
>
> https://gcc.godbolt.org/z/WTEaYWW8c
>
> It’s not perfect, because warning names sometimes differ between GCC and Clang, among other reasons.

That could be an interesting option for some projects, but yeah, as
you say, I think it is far from ideal. It breaks `clang-format`,
wrapping functions/structs/... would look weird, and generally the
syntax gets on the way of the "normal" code (so even for statements
one would probably want to keep the start and end on their own lines).

Cheers,
Miguel