Re: [PATCH 00/13] [RFC] Rust support
From: Olliver Schinagl
Date: Sat Oct 15 2022 - 10:35:59 EST
As this thread kind of went silent and as the 'big merge' for this
feature is getting closer, here a final plee, inspired by this slashdot
post [0].
The post in itself speaks of a new team forming on working on the Rust
styleguide, which in itself is still evolving. This makes sense, rust is
new, it's not very commonly in use and as with all good things, they evolve.
One comment in that slashdot post [1] I want to bring forward and quote
a piece of:
"i created a new repository, and thought i was being hip and modern, so
i started to evangelize spaces for the 'consistency across environments'
i get approached by not one, but TWO coworkers who unfortunately are
highly visually impaired and each has a different visual impairment
at that moment, i instantaneously conceded — there's just no
counter-argument that even comes close to outweighing the accessibility
needs of valued coworkers"
Visual impairness is a thing, it does not make someone smarter or
dumber. Helping those with visual impairments should be concidered, and
not shunted off by saying 'but the rust guys came up with the perfect
style, so we should use it'.
Find attached, a diff to the .rustfmt.toml, that should keep things more
consistent with the current kernel style.
I'll leave it now to Linus and Greg to concsider this, and will keep my
peace (though I hope they actually read it :p).
Olliver
[0]:
https://developers.slashdot.org/story/22/10/07/2351222/rust-programming-language-announces-new-team-to-evolve-official-coding-style
[1]: https://developers.slashdot.org/comments.pl?sid=22182701&cid=62949323
On 28-07-2022 22:43, Olliver Schinagl wrote:
Hey Gary,
On 28-07-2022 12:21, Gary Guo wrote:
Hi Olliver,
On Wed, 27 Jul 2022 10:05:31 +0200
Olliver Schinagl <oliver+list@xxxxxxxxxxx> wrote:
Consitency is absolutly important! Zero argument there. My argument
is, the consistency should be within the kernel tree, not 'but the
rest of the world is using style X/Y/Z, lets be consistent with that.
In an utopia, maybe, but the real world doesn't work that way, sadly.
So in an attempt to standardize (rustfmt) they just "invented" a new
standard. Which btw is common, we see this happening every so often,
right?
Difference languages have different characteristics and I don't think
it's necessarily good (and often isn't) to blindly apply coding style
of one language onto another. So I don't see rustfmt as "inventing yet
another standard" really, because there aren't many conflicting coding
style standards in Rust world; almost everyone just settled on using
rustfmt, mostly using the default, maybe with a few small
project-specific configuration tweaks.
I was mostly arguing about a) lets look at this and b) having said
configuration tweaks, rather then blindly (pun not really intended)
going with rust's defaults :)
A small example for C and Rust differences:
Rust requires braces around branches of if expression, and C doesn't.
So in kernel coding style you often have:
if (condition) do_something();
Or
if (condition)
do_something();
But in Rust it will be:
if condition {
do_something();
}
So kernel style kind of says 'no braces around single statements'; but
if your rust compiler doesn't allow this; well then there's nothing to
do. You could even argue to update the kernel C style on this to make it
consistent again. BUT, this inconsistency makes it cognative 'hard'. If
this if a C or a rust function? for example during a review. During
authoring, when writing both C and rust code (due to nececity, not
constant context switching) you cognitivly constantly have to go
back/foward. While I'm sure there's people here that can do this all day
without problem, some of of find this harder then needs to be. Hence the
request to _try_ to keep consistency within the kernel tree.
That's just an example of one control flow constructions. There are
differences between Rust match and C switch, etc. Rust's official
coding style takes properties of Rust into consideration, so in many
regards it's a more suitable coding style for Rust code in kernel, then
applying kernel's C coding standard directly on kernel's Rust code.
Your earlier email in the thread also mentions about indentation, and I
have a few things to point out as well.
First, Rust code typically requires more levels of indentation than C
code. For example, many functions might be methods and they are inside
an impl block, which creates one extra level of indentation.
Statements inside match arms' block are two levels more indented than
the match statement itself, as opposed to C's switch (as kernel coding
style doesn't indent the case labels). As a result, 8 spaces for 1 level
can be a bit excessive for Rust code, and thus the 4 space indentation
used in rustfmt default.
Secondly, I don't think the argument about tabs being customisable
holds; in kernel coding style tabs are strictly 8 characters. For line
Sure, this rule implies that for alignment, tabs should be set to 8 so
things align nicely. However, nobody forces people to set their editor
to 8 character width. Not doing so, doesn't break anything. At worst,
you may commit something that is poorly aligned (but we _should_ be
using tabs to indent, spaces to align anyway :p, tab == indent has meaning).
With non-tab indentation, this is no longer really possible, or at
least, editors haven't solved that problem yet, as it tends to still
break (due to the mixing of indentation and alignment using a single
character). Maybe once we have AI and ML in our editors though :p
continuation it's not uncommon to use a series of tabs followed by a
few spaces, e.g.
int function_name(int first_argument,
< tab >< tab >..int second_argument)
changing tab into 4 spaces will break the layout. (and I'll not go into
well-known reasons about non-4-space-tab messing up code in terminal
etc).
Copy/pasting is known to cause bugs. There's actually research from
NASA on that. Code-reuse (libraries/functions) are not bad. But
(worst kind of example) copy paste from stack-overflow, or
copy/pasting stuff without actually looking at the content and
forgetting to rename something, causes bugs. Why is this relevant?
The whole 'lets be consistent with the rust codebase of the wrold'
argument. E.g. if everybody uses the same style (which is idealistic
and great) then copy/pasting becomes consistent. Where I say, try to
be careful when copy/pasting code.
When we vendor in code as a whole (e.g. like we currently do for
alloc crate), it is proper code reuse. With different coding style the
vendored code either diverges from upstream (which makes upstreaming
much more difficult) or diverge from rest of kernel's Rust code base.
Very fair point of course. Though really, we should fix the upstream
rust preferred format, but there it was already stated, that 'too bad,
sorry' which from a developer point of view is fine, your project, your
choice. From a disabilities point of view, sucks of course.
But if that is the case, why not try to follow the kernels existing
code-style as close as possible with the rust-fmt configuration? I
know code-style has been discussed a few times over the decades; but
not many changes have been done, surely, if there's some codestyle
changes that are best for the kernel, they would have been 'advised'?
'4 space indents are better then 8-size tabs, on new code, try to use
them' for example :p
You do realize that you are creating a new coding style by doing this,
right? It feels like creating problems rather than solving problems.
My personal feeling is that it's easier for me to adapt to different
coding style when switching between languages, but it's rather awkward
for me when trying to use different coding styles with the same
language. I find myself no problem switching between 2 spaces when
coding JavaScript to 4 spaces when coding Rust to 8 spaces(tab) when
coding C, but it's rather painful to switch between C projects with
different coding styles. I certainly don't want to switch between Rust
projects with vastly different coding styles.
And I'm happy for you that you can easily take in 2 and 4 spaces. For
me, it is extremly hard to read. So it's not a 'personal preference'
thing. But I suggest to read the earlier posted links, where others at
length explain it as well, how it is like to feel excluded becaues its
just hard to read.
But why? Why should we not be consistent with the kernels' code-base
(while yes, that is not rust, but C, but we can follow the same
style?)
Difference languages have different characteristics, and one size
doesn't fit them all :)
I'm not even arguing this at all :)
I think the biggest issues i'm speaking of really are the braces and the
spaces really, where the braces can be argued for/against, it's
cognitive harder, but can be dealth with (and we can expect
inconsitencies; but the sapces vs tabs thing, personal configuration vs
forced with is the point I was trying to raise.
As said before, 'every building is different, some offer wheelchair
ramps, others do' kind of point, not like 'this building is red, and
that one is blue, and not every color fits all :p
Sadly, I've seen so much vendor code (yeah, I know) which doesn't
even have consistency in their own files ...
That's very true. So when all other Rust code currently follow
(roughly) the same coding style and this situation doesn't currently
exist, let's not make it worse...
Best,
Gary
From 0ac1e56435e27adddef9806dbf1134f909e99dd4 Mon Sep 17 00:00:00 2001
From: Olliver Schinagl <oliver@xxxxxxxxxxx>
Date: Sat, 15 Oct 2022 16:09:06 +0200
Subject: [PATCH] rustfmt: Match style with kernel codestyle
Signed-off-by: Olliver Schinagl <oliver@xxxxxxxxxxx>
---
.rustfmt.toml | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/.rustfmt.toml b/.rustfmt.toml
index 3de5cc497465..290b88c00265 100644
--- a/.rustfmt.toml
+++ b/.rustfmt.toml
@@ -1,5 +1,13 @@
+binop_separator = "Back"
+brace_style = "AlwaysNextLine"
+control_brace_style = "AlwaysSameLine"
edition = "2021"
+hard_tabs = "true"
+indent_style = "Visual"
+match_block_trailing_comma = "true"
newline_style = "Unix"
+struct_lit_single_line = "false"
+tab_spaces = "8"
# Unstable options that help catching some mistakes in formatting and that we may want to enable
# when they become stable.
--
2.38.0