linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Olliver Schinagl <oliver+list@schinagl.nl>
To: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Cc: Miguel Ojeda <ojeda@kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	rust-for-linux <rust-for-linux@vger.kernel.org>,
	Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	Linux Doc Mailing List <linux-doc@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 00/13] [RFC] Rust support
Date: Mon, 18 Jul 2022 08:56:15 +0200	[thread overview]
Message-ID: <ba8cb315-9d73-2f45-8bf9-d9473d369dca@schinagl.nl> (raw)
In-Reply-To: <CANiq72mRxM-7griYF+0FWqYoSoNL8ad=L-i6a2-GsaCeb0C6qQ@mail.gmail.com>

Hey Miguel,

Sorry for the late reply ;)

On 27-06-2022 19:44, Miguel Ojeda wrote:
> Hi Olliver,
> 
> On Mon, Jun 20, 2022 at 5:11 PM Olliver Schinagl <oliver@schinagl.nl> wrote:
>>
>> I apologize for being late to the party and for potentially using the
>> wrong thread, but I recall somewhere in v5 that it was best to respond
>> to the RFC for general comments.
> 
> No need to apologize! Feel free to use the latest threads or a new
> thread in e.g. the rust-for-linux ML.
> 
>> On 14-04-2021 20:45, ojeda@kernel.org wrote:
>>> From: Miguel Ojeda <ojeda@kernel.org>
>>>
>>> Moreover, as explained above, we are taking the chance to enforce
>>> some documentation guidelines. We are also enforcing automatic code
>>> formatting, a set of Clippy lints, etc. We decided to go with Rust's
>>> idiomatic style, i.e. keeping `rustfmt` defaults. For instance, this
>>> means 4 spaces are used for indentation, rather than a tab. We are
>>> happy to change that if needed -- we think what is important is
>>> keeping the formatting automated
>>
>> Enforcing this is great, but how will you enforce this 'everywhere'?
>> Right now, you can easily 'bypass' any CI put in place, and while 'for
>> now' this is only about the Rust infra, where this can be strongly
>> enforced, once we see actual drivers pop-up; these won't go through the
>> Rust CI before merging CI forever? A maintainer can 'just merge'
>> something still, right?
> 
> Indeed, but there are workarounds, for instance, we could have a bot
> checking -next.
Absolutly, but with the many luitenants, many tree's, and not a single 
CI source, this would still be tricky in the end; but certainly possible.

> 
> Or we could put it in an opt-in compilation mode (i.e. not for users)
> where extra things are checked (like `W=`) that maintainers use so
> that e.g. `allmodconfig` builds are kept clean.
> 
>> Anyway, what I wanted to criticize, is the so called "keeping with
>> `rustfmt` defaults". It has been known, that, well Rust's defaults are
>> pretty biased and opinionated. For the Rust project, that's fair of
>> course, their code, their rules.
>>
>> However, there's two arguments against that. For one, using the Rust
>> 'style', now means there's 2 different code styles in the Kernel.
>> Cognitively alone, that can be quite frustrating and annoying. Having to
>> go back and forth between two styles can be mentally challenging which
>> only causes mistakes and frustration. So why change something that
>> already exists? Also, see my first point. Having to constantly
>> remember/switch to 'in this file/function the curly brace is on a
>> different line'. Lets try to stay consistent, the rules may not be
>> perfect (80 columns ;), but so far consistency is tried. OCD and Autism
>> etc doesn't help with this ;)
> 
> Note that the point of using `rustfmt` is that one does not need to
> care about the details -- one can e.g. run the tool on file save. So
> no need to remember how to do it when writing Rust.
And that's great of course, I was mearly speaking of the configuration 
of rustfmt. I think as a tool it's pretty great!

> 
> Now, it is true that the Rust syntax resembles C in many cases, so
> things like the curly braces for function definitions are similar
> enough that we could do the same thing in both sides.
> 
> However, most Rust code uses `rustfmt` and typically also follow most
> of its defaults, including the standard library, books, etc.; which
> helps when reading and reusing other code. This is different from C
> and C++, where as you know there is no single style (at least as
> prevalent as `rustfmt`), thus one needs to become accustomed to each
> project's C style (or ideally use `clang-format` to avoid having to
> learn it). So while this is not relevant for C, in the case of Rust,
> there is value in using the `rustfmt` style.
I think this is a pretty poor argument for following Rust's opinionated 
view of the world. E.g. it's generally bad to copy/paste code to begin 
with. How many 'bugs' that we know of are copy/paste bugs?

Secondly, and more importantly so; you argue 'who cares about people 
with disablements, atleast its equally hard to read everywhere' which is 
a very poor argument :p

Finally, it must of course be mentioned, that rust is really trying to 
do an XKCD here, https://xkcd.com/927/ though I'm sure we'll get it 
right this time around ;)

> 
> As for consistency, one could argue that by using `rustfmt` we are
> being consistent with the rest of the Rust code out there.
But you are not, only those that follow rust's biased view. Everybody 
else that has a different opinion (like die-hard C programmers) that 
care enough (I'm sure there's plenty) would setup their rustfmt config 
file to resemble their C code; and thus the entire premisis is broken. 
Though; yes, in a perfect world it could have worked like this, but xkcd 
again :)

> This may be
> important for those that have expressed interest on sharing some code
> between kernel and userspace; as well as if we end up vendoring some
> external crates (similar to what we do with `alloc` now).
This though is a fair argument I understand, it would be weird in having 
2 styles in user-space and kernel-space code; though I see this 
happening today as well; where developers follow kernel style for kernel 
code (obviously) but use their preferred 2 or 3 space style on their 
userland code. Trying to 'force' this, usually however never gets the 
intended result ...

> 
>> Secondly, and this is really far more important, the Rust default style
>> is not very inclusive, as it makes readability harder. This has been
>> brought up by many others in plenty of places, including the `rustfmt`
>> issue tracker under bug #4067 [0]. While the discussion eventually only
>> led to the 'fmt-rfcs' [1], where it was basically said 'you could be on
>> to something, but this ship has sailed 3 years ago (when nobody was
>> looking caring), and while we hear you, we're not going to change our
>> defaults anymore.
>>
>> But I also agree and share these commenters pain. When the tab character
>> is used for indenting (and not alignment mind you), then visually
>> impaired (who can still be amazing coders) can more easily read code by
>> adjusting the width what works best to them.
>>
>> With even git renaming `master` to `main` to be more inclusive, can we
>> also be more inclusive to us that have a hard time distinguishing narrow
>> indentations?
> 
> As noted in the RFC, we are happy to tweak the style to whatever
> kernel developers prefer. We think the particular style is not that
> important. Absent other reasons, the defaults seem OK, so we chose
> that for simplicity and consistency with as most existing Rust code as
> possible.
> 
> As for accessibility, I am no expert, so that may be a good point,
> especially if editors cannot solve this on their end (so that everyone
> could program in all languages/projects regardless of style).
Yeah, this is a common reasoning. People without disabilities often 
oversee cases to those with. E.g. Traffic lights being red and green is 
horrible for colorblind people; luckily enough we have 'order' to help 
distinguish there for example. While I'm not colorblind myself, I often 
have to remind UX designers, with their fancy LED based UI's, to think 
of others as well, which always strikes them as odd first, then of 
course they only start to realize this.

I'm with you that style is the least important for the functionality, no 
argument there. Long-term though; this will matter of course, to those 
like me, have hard times here.

> 
>> Thanks, and sorry for rubbing any ones nerves, but to "some of us" this
>> actually matters a great deal.
> 
> No nerves were damaged :) Thanks for all the input!
> 
>> P.S. would we expect inline C/Rust code mixed? What then?
> 
> Everything is possible, e.g. we could have Rust proc macros that parse
> C and things like that. But if we ended up with such a thing, the
> solution would be to format each accordingly to its style (indentation
> could be an exception, I guess).
The first exception to the rule starts here already :p

Thanks for your thoughts,

Olliver
> 
> Cheers,
> Miguel


  reply	other threads:[~2022-07-18  6:56 UTC|newest]

Thread overview: 201+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-14 18:45 ojeda
2021-04-14 18:45 ` [PATCH 01/13] kallsyms: Support "big" kernel symbols (2-byte lengths) ojeda
2021-04-14 19:44   ` Matthew Wilcox
2021-04-14 19:59     ` Miguel Ojeda
2021-04-14 18:45 ` [PATCH 02/13] kallsyms: Increase maximum kernel symbol length to 512 ojeda
2021-04-14 23:48   ` Nick Desaulniers
2021-04-14 18:45 ` [PATCH 03/13] Makefile: Generate CLANG_FLAGS even in GCC builds ojeda
2021-04-14 18:59   ` Nathan Chancellor
2021-04-15 10:18     ` Miguel Ojeda
2021-04-14 23:46   ` Nick Desaulniers
2021-04-15  0:47     ` Miguel Ojeda
2021-04-14 18:45 ` [PATCH 04/13] Kbuild: Rust support ojeda
2021-04-14 23:19   ` Nick Desaulniers
2021-04-15  0:43     ` Miguel Ojeda
2021-04-15 18:03       ` Nick Desaulniers
2021-04-16 12:23         ` Miguel Ojeda
2021-04-17 19:35       ` Masahiro Yamada
2021-04-16 13:38   ` Peter Zijlstra
2021-04-16 17:05     ` Linus Torvalds
2021-04-16 17:47       ` Miguel Ojeda
2021-04-16 18:09         ` Al Viro
2021-04-16 18:57           ` Miguel Ojeda
2021-04-16 20:22             ` Willy Tarreau
2021-04-16 20:34               ` Connor Kuehl
2021-04-16 20:58                 ` Willy Tarreau
2021-04-16 21:39                   ` Miguel Ojeda
2021-04-16 22:04                     ` Willy Tarreau
2021-04-16 22:45                       ` Al Viro
2021-04-16 23:46                       ` Miguel Ojeda
2021-04-17  4:24                         ` Willy Tarreau
2021-04-17 15:38                           ` Miguel Ojeda
2021-04-16 21:19               ` Miguel Ojeda
2021-04-16 17:34     ` Miguel Ojeda
2021-04-19 19:58       ` David Sterba
2021-04-19 20:17         ` Matthew Wilcox
2021-04-19 21:03           ` Miguel Ojeda
2021-04-19 20:54         ` Miguel Ojeda
2021-04-14 18:45 ` [PATCH 05/13] Rust: Compiler builtins crate ojeda
2021-04-14 19:19   ` Linus Torvalds
2021-04-14 19:34     ` Miguel Ojeda
2021-04-14 18:45 ` [PATCH 06/13] Rust: Module crate ojeda
2021-04-14 18:45 ` [PATCH 07/13] Rust: Kernel crate ojeda
2021-04-14 19:31   ` Linus Torvalds
2021-04-14 19:50     ` Miguel Ojeda
2021-04-14 18:45 ` [PATCH 08/13] Rust: Export generated symbols ojeda
2021-04-14 18:46 ` [PATCH 09/13] Samples: Rust examples ojeda
2021-04-14 19:34   ` Linus Torvalds
2021-04-14 19:42     ` Miguel Ojeda
2021-04-14 19:49       ` Matthew Wilcox
2021-04-16 11:46       ` Andrej Shadura
2021-04-14 23:24     ` Nick Desaulniers
2021-04-15  7:10       ` Greg Kroah-Hartman
2021-04-15  7:39         ` Nick Desaulniers
2021-04-15 12:42         ` Miguel Ojeda
2021-04-16 13:07         ` Sven Van Asbroeck
2021-04-16 13:20           ` Greg Kroah-Hartman
2021-04-14 18:46 ` [PATCH 10/13] Documentation: Rust general information ojeda
2021-04-14 22:17   ` Nick Desaulniers
2021-04-14 23:34     ` Miguel Ojeda
2021-04-14 18:46 ` [PATCH 11/13] MAINTAINERS: Rust ojeda
2021-04-14 21:55   ` Nick Desaulniers
2021-04-14 22:02     ` Miguel Ojeda
2021-04-14 22:36   ` Nick Desaulniers
2021-04-14 18:46 ` [PATCH 12/13] Rust: add abstractions for Binder (WIP) ojeda
2021-04-14 18:46 ` [PATCH 13/13] Android: Binder IPC in Rust (WIP) ojeda
2021-04-14 19:44 ` [PATCH 00/13] [RFC] Rust support Linus Torvalds
2021-04-14 20:20   ` Miguel Ojeda
2021-04-15  1:38     ` Kees Cook
2021-04-15  8:26       ` David Laight
2021-04-15 18:08         ` Kees Cook
2021-04-15 12:39       ` Miguel Ojeda
2021-04-14 20:09 ` Matthew Wilcox
2021-04-14 20:21   ` Linus Torvalds
2021-04-14 20:35     ` Josh Triplett
2021-04-14 22:08     ` David Laight
2021-04-14 20:29   ` Miguel Ojeda
2021-04-18 15:31   ` Wedson Almeida Filho
2021-04-15  0:22 ` Nick Desaulniers
2021-04-15 10:05   ` Miguel Ojeda
2021-04-15 18:58 ` Peter Zijlstra
2021-04-16  2:22   ` Wedson Almeida Filho
2021-04-16  4:25     ` Al Viro
2021-04-16  5:02       ` Wedson Almeida Filho
2021-04-16  5:39         ` Paul Zimmerman
2021-04-16  7:46         ` Peter Zijlstra
2021-04-16  7:09     ` Peter Zijlstra
2021-04-17  5:23       ` comex
2021-04-17 12:46       ` David Laight
2021-04-17 14:51       ` Paolo Bonzini
2021-04-19  7:32         ` Peter Zijlstra
2021-04-19  7:53           ` Paolo Bonzini
2021-04-19  8:26             ` Peter Zijlstra
2021-04-19  8:35               ` Peter Zijlstra
2021-04-19  9:02               ` Paolo Bonzini
2021-04-19  9:36                 ` Peter Zijlstra
2021-04-19  9:40                   ` Paolo Bonzini
2021-04-19 11:01                     ` Will Deacon
2021-04-19 17:14                   ` Linus Torvalds
2021-04-19 18:38                     ` Paolo Bonzini
2021-04-19 18:50                       ` Linus Torvalds
2021-04-22 10:03     ` Linus Walleij
2021-04-22 14:09       ` David Laight
2021-04-22 15:24       ` Wedson Almeida Filho
2021-04-26  0:18         ` Linus Walleij
2021-04-26 14:26           ` Miguel Ojeda
2021-04-26 14:40           ` Wedson Almeida Filho
2021-04-26 16:03             ` Miguel Ojeda
2021-04-27 10:54             ` Linus Walleij
2021-04-27 11:13               ` Robin Randhawa
2021-04-29  1:52               ` Wedson Almeida Filho
2021-04-26 18:01           ` Miguel Ojeda
2021-04-22 21:28       ` Miguel Ojeda
2021-04-26  0:31         ` Linus Walleij
2021-04-26 18:18           ` Miguel Ojeda
2021-04-27 11:13             ` Linus Walleij
2021-04-28  2:51               ` Kyle Strand
2021-04-28  3:10               ` Miguel Ojeda
2021-05-04 21:21                 ` Linus Walleij
2021-05-04 23:30                   ` Miguel Ojeda
2021-05-05 11:34                     ` Linus Walleij
2021-05-05 14:17                       ` Miguel Ojeda
2021-05-05 15:13                         ` Enrico Weigelt, metux IT consult
2021-05-06 12:47                         ` Linus Walleij
2021-05-07 18:23                           ` Miguel Ojeda
2021-04-16  4:27   ` Boqun Feng
2021-04-16  6:04     ` Nick Desaulniers
2021-04-16 18:47       ` Paul E. McKenney
2021-04-19 20:35         ` Nick Desaulniers
2021-04-19 21:37           ` Paul E. McKenney
2021-04-19 22:03           ` Miguel Ojeda
2021-04-16 20:48     ` Josh Triplett
2021-04-16  8:16   ` Michal Kubecek
2021-04-16  9:29     ` Willy Tarreau
2021-04-16 11:24 ` Peter Zijlstra
2021-04-16 13:07   ` Wedson Almeida Filho
2021-04-16 14:19     ` Peter Zijlstra
2021-04-16 15:04       ` Miguel Ojeda
2021-04-16 15:43         ` Peter Zijlstra
2021-04-16 16:21           ` Miguel Ojeda
2021-04-16 15:33       ` Wedson Almeida Filho
2021-04-16 16:14         ` Willy Tarreau
2021-04-16 17:10           ` Miguel Ojeda
2021-04-16 17:18             ` Peter Zijlstra
2021-04-16 18:08               ` Matthew Wilcox
2021-04-17 11:17                 ` Peter Zijlstra
2021-04-17 11:46                   ` Willy Tarreau
2021-04-17 14:24                     ` Peter Zijlstra
2021-04-17 14:36                       ` Willy Tarreau
2021-04-17 13:46                   ` David Laight
2021-04-16 17:37             ` Willy Tarreau
2021-04-16 17:46               ` Connor Kuehl
2021-04-20  0:24               ` Nick Desaulniers
2021-04-20  3:47                 ` Willy Tarreau
2021-04-20  5:56                 ` Greg Kroah-Hartman
2021-04-20  6:16                   ` Willy Tarreau
2021-04-29 15:38                     ` peter enderborg
2021-04-17 13:53           ` Wedson Almeida Filho
2021-04-17 14:21             ` Willy Tarreau
2021-04-17 15:23               ` Miguel Ojeda
2021-04-18 15:51               ` Wedson Almeida Filho
2021-04-17 12:41       ` David Laight
2021-04-17 13:01         ` Wedson Almeida Filho
2021-04-16 15:03     ` Matthew Wilcox
2021-04-17 13:29       ` Wedson Almeida Filho
2021-04-16 15:58     ` Theodore Ts'o
2021-04-16 16:21       ` Wedson Almeida Filho
2021-04-17 15:11       ` Paolo Bonzini
2021-04-16 14:21   ` Miguel Ojeda
2021-04-17 20:42 ` Richard Weinberger
2021-04-28 18:34 ` Mariusz Ceier
2021-04-28 20:25   ` Nick Desaulniers
2021-04-28 21:21   ` David Laight
2021-04-29 11:14     ` Kajetan Puchalski
2021-04-29 11:25   ` Kajetan Puchalski
2021-04-29 14:06     ` Mariusz Ceier
2021-04-29 14:13       ` Sven Van Asbroeck
2021-04-29 14:26         ` Willy Tarreau
2021-04-29 15:06       ` Al Viro
2021-04-29 16:09         ` Mariusz Ceier
2021-04-30  6:39     ` Thomas Schoebel-Theuer
2021-04-30  8:30       ` David Laight
2021-05-05 13:58       ` Enrico Weigelt, metux IT consult
2021-05-05 14:41         ` Miguel Ojeda
2022-06-20 15:11 ` Olliver Schinagl
2022-06-27 17:44   ` Miguel Ojeda
2022-07-18  6:56     ` Olliver Schinagl [this message]
2022-07-20 19:23       ` Miguel Ojeda
2022-07-20 20:21         ` Nicolas Pitre
2022-07-27  7:47           ` Olliver Schinagl
2022-07-27 13:32             ` Nicolas Pitre
2022-07-27  8:05         ` Olliver Schinagl
2022-07-28 10:21           ` Gary Guo
2022-07-28 12:09             ` Greg Kroah-Hartman
2022-07-28 12:28               ` Gary Guo
2022-07-28 20:45               ` Olliver Schinagl
2022-07-29  8:04                 ` Greg Kroah-Hartman
2022-07-28 20:43             ` Olliver Schinagl
2021-04-29  5:20 Mariusz Ceier
2021-04-29  5:21 Mariusz Ceier
2021-04-29  8:18 ` David Laight
2021-07-30 23:22 Dillan Jackson

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=ba8cb315-9d73-2f45-8bf9-d9473d369dca@schinagl.nl \
    --to=oliver+list@schinagl.nl \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miguel.ojeda.sandonis@gmail.com \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --subject='Re: [PATCH 00/13] [RFC] Rust support' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).