mirror of
https://github.com/torvalds/linux.git
synced 2025-11-05 03:00:13 +02:00
`rustfmt`, by default, formats imports in a way that is prone to conflicts
while merging and rebasing, since in some cases it condenses several
items into the same line.
For instance, Linus mentioned [1] that the following case:
use crate::{
fmt,
page::AsPageIter,
};
is compressed by `rustfmt` into:
use crate::{fmt, page::AsPageIter};
which is undesirable.
Similarly, `rustfmt` may put several items in the same line even if the
braces span already multiple lines, e.g.:
use kernel::{
acpi, c_str,
device::{property, Core},
of, platform,
};
The options that control the formatting behavior around imports are
generally unstable, and `rustfmt` releases do not allow to use nightly
features, unlike the compiler and other Rust tooling [2].
For the moment, we can introduce a workaround to prevent `rustfmt`
from compressing the example above -- the "trailing empty comment":
use crate::{
fmt,
page::AsPageIter, //
};
which is reminiscent of the trailing comma behavior in other formatters.
We already used empty comments for formatting purposes in the past,
e.g. in commit b9b701fce4 ("rust: clarify the language unstable features
in use").
In addition, `rustfmt` actually reformats with a vertical layout (i.e. it
does not put two items in the same line) when seeing such a comment,
i.e. it doesn't just preserve the formatting, which is good in the sense
that we can use it to easily reformat some imports, since it matches
the style we generally want to have.
A Git merge driver would help (suggested by Gary and Wedson), though
maintainers would need to set it up, the diffs would still be larger
and the formatting rules for imports would remain hard to predict.
Thus document the style that we will follow in the coding guidelines
by introducing a new section and explain how the trailing empty comment
works there too.
We discussed the issue with upstream Rust in our usual Rust <-> Rust
for Linux meeting [3], and there have also been a few other discussions
in parallel in issues [4][5] and Zulip [6]. We will see what happens,
but upstream Rust has already created a subteam of `rustfmt` to try
to overcome the bandwidth issue [7], which is a good signal, and some
organization work has already started (e.g. tracking issues). We will
continue our discussions with them about it.
Cc: Caleb Cartwright <caleb.cartwright@outlook.com>
Cc: Yacin Tmimi <yacintmimi@gmail.com>
Cc: Manish Goregaokar <manishsmail@gmail.com>
Cc: Deadbeef <ent3rm4n@gmail.com>
Cc: Cameron Steffen <cam.steffen94@gmail.com>
Cc: Jieyou Xu <jieyouxu@outlook.com>
Link: https://lore.kernel.org/all/CAHk-=wgO7S_FZUSBbngG5vtejWOpzDfTTBkVvP3_yjJmFddbzA@mail.gmail.com/ [1]
Link: https://github.com/rust-lang/rustfmt/issues/4884 [2]
Link: https://hackmd.io/iSCyY3JTTz-g8YM-nnzTTA [3]
Link: https://github.com/rust-lang/rustfmt/issues/4991 [4]
Link: https://github.com/rust-lang/rustfmt/issues/3361 [5]
Link: https://rust-lang.zulipchat.com/#narrow/channel/392734-council/topic/rustfmt.20maintenance/near/543815381 [6]
Link: https://github.com/rust-lang/team/pull/2017 [7]
Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Miguel Ojeda <ojeda@kernel.org>
487 lines
15 KiB
ReStructuredText
487 lines
15 KiB
ReStructuredText
.. SPDX-License-Identifier: GPL-2.0
|
|
|
|
Coding Guidelines
|
|
=================
|
|
|
|
This document describes how to write Rust code in the kernel.
|
|
|
|
|
|
Style & formatting
|
|
------------------
|
|
|
|
The code should be formatted using ``rustfmt``. In this way, a person
|
|
contributing from time to time to the kernel does not need to learn and
|
|
remember one more style guide. More importantly, reviewers and maintainers
|
|
do not need to spend time pointing out style issues anymore, and thus
|
|
less patch roundtrips may be needed to land a change.
|
|
|
|
.. note:: Conventions on comments and documentation are not checked by
|
|
``rustfmt``. Thus those are still needed to be taken care of.
|
|
|
|
The default settings of ``rustfmt`` are used. This means the idiomatic Rust
|
|
style is followed. For instance, 4 spaces are used for indentation rather
|
|
than tabs.
|
|
|
|
It is convenient to instruct editors/IDEs to format while typing,
|
|
when saving or at commit time. However, if for some reason reformatting
|
|
the entire kernel Rust sources is needed at some point, the following can be
|
|
run::
|
|
|
|
make LLVM=1 rustfmt
|
|
|
|
It is also possible to check if everything is formatted (printing a diff
|
|
otherwise), for instance for a CI, with::
|
|
|
|
make LLVM=1 rustfmtcheck
|
|
|
|
Like ``clang-format`` for the rest of the kernel, ``rustfmt`` works on
|
|
individual files, and does not require a kernel configuration. Sometimes it may
|
|
even work with broken code.
|
|
|
|
Imports
|
|
~~~~~~~
|
|
|
|
``rustfmt``, by default, formats imports in a way that is prone to conflicts
|
|
while merging and rebasing, since in some cases it condenses several items into
|
|
the same line. For instance:
|
|
|
|
.. code-block:: rust
|
|
|
|
// Do not use this style.
|
|
use crate::{
|
|
example1,
|
|
example2::{example3, example4, example5},
|
|
example6, example7,
|
|
example8::example9,
|
|
};
|
|
|
|
Instead, the kernel uses a vertical layout that looks like this:
|
|
|
|
.. code-block:: rust
|
|
|
|
use crate::{
|
|
example1,
|
|
example2::{
|
|
example3,
|
|
example4,
|
|
example5, //
|
|
},
|
|
example6,
|
|
example7,
|
|
example8::example9, //
|
|
};
|
|
|
|
That is, each item goes into its own line, and braces are used as soon as there
|
|
is more than one item in a list.
|
|
|
|
The trailing empty comment allows to preserve this formatting. Not only that,
|
|
``rustfmt`` will actually reformat imports vertically when the empty comment is
|
|
added. That is, it is possible to easily reformat the original example into the
|
|
expected style by running ``rustfmt`` on an input like:
|
|
|
|
.. code-block:: rust
|
|
|
|
// Do not use this style.
|
|
use crate::{
|
|
example1,
|
|
example2::{example3, example4, example5, //
|
|
},
|
|
example6, example7,
|
|
example8::example9, //
|
|
};
|
|
|
|
The trailing empty comment works for nested imports, as shown above, as well as
|
|
for single item imports -- this can be useful to minimize diffs within patch
|
|
series:
|
|
|
|
.. code-block:: rust
|
|
|
|
use crate::{
|
|
example1, //
|
|
};
|
|
|
|
The trailing empty comment works in any of the lines within the braces, but it
|
|
is preferred to keep it in the last item, since it is reminiscent of the
|
|
trailing comma in other formatters. Sometimes it may be simpler to avoid moving
|
|
the comment several times within a patch series due to changes in the list.
|
|
|
|
There may be cases where exceptions may need to be made, i.e. none of this is
|
|
a hard rule. There is also code that is not migrated to this style yet, but
|
|
please do not introduce code in other styles.
|
|
|
|
Eventually, the goal is to get ``rustfmt`` to support this formatting style (or
|
|
a similar one) automatically in a stable release without requiring the trailing
|
|
empty comment. Thus, at some point, the goal is to remove those comments.
|
|
|
|
|
|
Comments
|
|
--------
|
|
|
|
"Normal" comments (i.e. ``//``, rather than code documentation which starts
|
|
with ``///`` or ``//!``) are written in Markdown the same way as documentation
|
|
comments are, even though they will not be rendered. This improves consistency,
|
|
simplifies the rules and allows to move content between the two kinds of
|
|
comments more easily. For instance:
|
|
|
|
.. code-block:: rust
|
|
|
|
// `object` is ready to be handled now.
|
|
f(object);
|
|
|
|
Furthermore, just like documentation, comments are capitalized at the beginning
|
|
of a sentence and ended with a period (even if it is a single sentence). This
|
|
includes ``// SAFETY:``, ``// TODO:`` and other "tagged" comments, e.g.:
|
|
|
|
.. code-block:: rust
|
|
|
|
// FIXME: The error should be handled properly.
|
|
|
|
Comments should not be used for documentation purposes: comments are intended
|
|
for implementation details, not users. This distinction is useful even if the
|
|
reader of the source file is both an implementor and a user of an API. In fact,
|
|
sometimes it is useful to use both comments and documentation at the same time.
|
|
For instance, for a ``TODO`` list or to comment on the documentation itself.
|
|
For the latter case, comments can be inserted in the middle; that is, closer to
|
|
the line of documentation to be commented. For any other case, comments are
|
|
written after the documentation, e.g.:
|
|
|
|
.. code-block:: rust
|
|
|
|
/// Returns a new [`Foo`].
|
|
///
|
|
/// # Examples
|
|
///
|
|
// TODO: Find a better example.
|
|
/// ```
|
|
/// let foo = f(42);
|
|
/// ```
|
|
// FIXME: Use fallible approach.
|
|
pub fn f(x: i32) -> Foo {
|
|
// ...
|
|
}
|
|
|
|
This applies to both public and private items. This increases consistency with
|
|
public items, allows changes to visibility with less changes involved and will
|
|
allow us to potentially generate the documentation for private items as well.
|
|
In other words, if documentation is written for a private item, then ``///``
|
|
should still be used. For instance:
|
|
|
|
.. code-block:: rust
|
|
|
|
/// My private function.
|
|
// TODO: ...
|
|
fn f() {}
|
|
|
|
One special kind of comments are the ``// SAFETY:`` comments. These must appear
|
|
before every ``unsafe`` block, and they explain why the code inside the block is
|
|
correct/sound, i.e. why it cannot trigger undefined behavior in any case, e.g.:
|
|
|
|
.. code-block:: rust
|
|
|
|
// SAFETY: `p` is valid by the safety requirements.
|
|
unsafe { *p = 0; }
|
|
|
|
``// SAFETY:`` comments are not to be confused with the ``# Safety`` sections
|
|
in code documentation. ``# Safety`` sections specify the contract that callers
|
|
(for functions) or implementors (for traits) need to abide by. ``// SAFETY:``
|
|
comments show why a call (for functions) or implementation (for traits) actually
|
|
respects the preconditions stated in a ``# Safety`` section or the language
|
|
reference.
|
|
|
|
|
|
Code documentation
|
|
------------------
|
|
|
|
Rust kernel code is not documented like C kernel code (i.e. via kernel-doc).
|
|
Instead, the usual system for documenting Rust code is used: the ``rustdoc``
|
|
tool, which uses Markdown (a lightweight markup language).
|
|
|
|
To learn Markdown, there are many guides available out there. For instance,
|
|
the one at:
|
|
|
|
https://commonmark.org/help/
|
|
|
|
This is how a well-documented Rust function may look like:
|
|
|
|
.. code-block:: rust
|
|
|
|
/// Returns the contained [`Some`] value, consuming the `self` value,
|
|
/// without checking that the value is not [`None`].
|
|
///
|
|
/// # Safety
|
|
///
|
|
/// Calling this method on [`None`] is *[undefined behavior]*.
|
|
///
|
|
/// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html
|
|
///
|
|
/// # Examples
|
|
///
|
|
/// ```
|
|
/// let x = Some("air");
|
|
/// assert_eq!(unsafe { x.unwrap_unchecked() }, "air");
|
|
/// ```
|
|
pub unsafe fn unwrap_unchecked(self) -> T {
|
|
match self {
|
|
Some(val) => val,
|
|
|
|
// SAFETY: The safety contract must be upheld by the caller.
|
|
None => unsafe { hint::unreachable_unchecked() },
|
|
}
|
|
}
|
|
|
|
This example showcases a few ``rustdoc`` features and some conventions followed
|
|
in the kernel:
|
|
|
|
- The first paragraph must be a single sentence briefly describing what
|
|
the documented item does. Further explanations must go in extra paragraphs.
|
|
|
|
- Unsafe functions must document their safety preconditions under
|
|
a ``# Safety`` section.
|
|
|
|
- While not shown here, if a function may panic, the conditions under which
|
|
that happens must be described under a ``# Panics`` section.
|
|
|
|
Please note that panicking should be very rare and used only with a good
|
|
reason. In almost all cases, a fallible approach should be used, typically
|
|
returning a ``Result``.
|
|
|
|
- If providing examples of usage would help readers, they must be written in
|
|
a section called ``# Examples``.
|
|
|
|
- Rust items (functions, types, constants...) must be linked appropriately
|
|
(``rustdoc`` will create a link automatically).
|
|
|
|
- Any ``unsafe`` block must be preceded by a ``// SAFETY:`` comment
|
|
describing why the code inside is sound.
|
|
|
|
While sometimes the reason might look trivial and therefore unneeded,
|
|
writing these comments is not just a good way of documenting what has been
|
|
taken into account, but most importantly, it provides a way to know that
|
|
there are no *extra* implicit constraints.
|
|
|
|
To learn more about how to write documentation for Rust and extra features,
|
|
please take a look at the ``rustdoc`` book at:
|
|
|
|
https://doc.rust-lang.org/rustdoc/how-to-write-documentation.html
|
|
|
|
In addition, the kernel supports creating links relative to the source tree by
|
|
prefixing the link destination with ``srctree/``. For instance:
|
|
|
|
.. code-block:: rust
|
|
|
|
//! C header: [`include/linux/printk.h`](srctree/include/linux/printk.h)
|
|
|
|
or:
|
|
|
|
.. code-block:: rust
|
|
|
|
/// [`struct mutex`]: srctree/include/linux/mutex.h
|
|
|
|
|
|
C FFI types
|
|
-----------
|
|
|
|
Rust kernel code refers to C types, such as ``int``, using type aliases such as
|
|
``c_int``, which are readily available from the ``kernel`` prelude. Please do
|
|
not use the aliases from ``core::ffi`` -- they may not map to the correct types.
|
|
|
|
These aliases should generally be referred directly by their identifier, i.e.
|
|
as a single segment path. For instance:
|
|
|
|
.. code-block:: rust
|
|
|
|
fn f(p: *const c_char) -> c_int {
|
|
// ...
|
|
}
|
|
|
|
|
|
Naming
|
|
------
|
|
|
|
Rust kernel code follows the usual Rust naming conventions:
|
|
|
|
https://rust-lang.github.io/api-guidelines/naming.html
|
|
|
|
When existing C concepts (e.g. macros, functions, objects...) are wrapped into
|
|
a Rust abstraction, a name as close as reasonably possible to the C side should
|
|
be used in order to avoid confusion and to improve readability when switching
|
|
back and forth between the C and Rust sides. For instance, macros such as
|
|
``pr_info`` from C are named the same in the Rust side.
|
|
|
|
Having said that, casing should be adjusted to follow the Rust naming
|
|
conventions, and namespacing introduced by modules and types should not be
|
|
repeated in the item names. For instance, when wrapping constants like:
|
|
|
|
.. code-block:: c
|
|
|
|
#define GPIO_LINE_DIRECTION_IN 0
|
|
#define GPIO_LINE_DIRECTION_OUT 1
|
|
|
|
The equivalent in Rust may look like (ignoring documentation):
|
|
|
|
.. code-block:: rust
|
|
|
|
pub mod gpio {
|
|
pub enum LineDirection {
|
|
In = bindings::GPIO_LINE_DIRECTION_IN as _,
|
|
Out = bindings::GPIO_LINE_DIRECTION_OUT as _,
|
|
}
|
|
}
|
|
|
|
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
|
|
|
|
.. [#] In this particular case, the kernel's ``__{always,maybe}_unused``
|
|
attributes (C23's ``[[maybe_unused]]``) may be used; however, the example
|
|
is meant to reflect the equivalent lint in Rust discussed afterwards.
|
|
|
|
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.
|
|
|
|
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 ``expect`` over ``allow`` unless:
|
|
|
|
- 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``).
|
|
|
|
For more information about diagnostics in Rust, please see:
|
|
|
|
https://doc.rust-lang.org/stable/reference/attributes/diagnostics.html
|
|
|
|
Error handling
|
|
--------------
|
|
|
|
For some background and guidelines about Rust for Linux specific error handling,
|
|
please see:
|
|
|
|
https://rust.docs.kernel.org/kernel/error/type.Result.html#error-codes-in-c-and-rust
|