If we're going to talk about wasted lines consider this:
we mandate comments as follows:
/*
* something
*/
I just did a test and we have something like 40K lines of white space
spent on that dangling */.
We've got something like 30K if not followed by {
So, were we to to get away from the comments with "wings" (as some
kernel people once called them) we could save all those lines
with just a /* or */ on them. This would more than make up for
additional white lines added by a dangling {.
I note that the changing our comment style would have zero impact on
code safety. The improvement of requiring a { on the ifs is known to
have positive impact; it's why Rust and Go both require it to my
understanding.
As for "the kernel" and its coding style, we are going to increasingly
see people coming from worlds where "the kernel" and its coding style
matter less and less. Maybe adherence to "the kernel" coding style
mattered ten years ago; I don't think it is as important now. In my
case, increasingly, "the kernel" coding style is looking a bit dated.
On Mon, Jun 24, 2019 at 8:14 PM Julius Werner <jwer...@chromium.org> wrote:
>
> Doesn't -Wmisleading-indentation already catch all of this? That's
> enabled by default on the coreboot gcc. I don't think "it's just a
> heuristic" should be a concern unless anyone knows of a real example
> that is otherwise valid coreboot code style but not caught by this
> heuristic. (If we're worried about examples that are not valid code
> style, then changing the code style to make them even more forbidden
> doesn't help... so I think weird cases that mix tab and space
> indentation or the like should count in favor of this.)
>
> If we're concerned that clang-format might cement errors automatically
> then that's a reason for not using clang-format that way, but I don't
> see how changing the coding style would solve it. clang-format's whole
> job is to take whatever input and transform it into the coding style,
> so the input is likely not style-compliant yet.
>
> Forcing braces on single-line statements adds an extra line of
> whitespace where it would otherwise not necessarily belong, which
> hurts readability. How much of a function can fit on a single screen
> is a real readability concern, and I think this style change would
> harm it. That's the same reason we write
>
> while {
>
> instead of
>
> while
> {
>
> like some other projects. (Of course blank lines can also help
> readability, but only in the *right* places and not randomly injected
> by style rules.) It would also move us yet again further away from
> kernel style, causing more issues for people coming from other
> projects.
>
> On Thu, Jun 20, 2019 at 2:54 PM ron minnich <rminn...@gmail.com> wrote:
> >
> > On Thu, Jun 20, 2019 at 1:12 PM Stefan Reinauer
> > <stefan.reina...@coreboot.org> wrote:
> > >
> > >
> > >
> > > On 20 Jun 2019 08:26, ron minnich <rminn...@gmail.com> wrote:
> > >
> > > clang-format is not a textual preprocessor. It is basically the ast
> > > builder of followed by output.
> > >
> > > So in your case, I just tried it
> > > main() {
> > >
> > > if (foo)
> > > bar();
> > > baz();
> > > }
> > >
> > > and got the right thing, i.e. braces around bar but not baz.
> > >
> > >
> > > The right thing (e.g. the obviously intended thing) would be too have
> > > braces around both.
> > >
> > > clang-format in this case masks the problem and makes it harder to
> > > identify by expanding the syntax of the unwanted behavior to look
> > > intentional.
> >
> > Nico and Stefan, you make a good point, but I would then argue for
> > even better tools, like clang-tidy:
> > /tmp/x.c:13:10: warning: statement should be inside braces
> > [readability-braces-around-statements]
> > if (foo)
> > ^
> > {
> >
> > In this case, there is a warning thrown, and the author has to clean it up.
> >
> > I don't believe, based on a lot of the history of this sort of problem
> > in C, that we should depend on human reviewers to catch mistakes like
> > this. These tools exist because of a demonstrated need. I think
> > coreboot could benefit from their proper application.
> >
> > You've presented a good case, but what about something like this:
> > if (foo)
> > bar();
> > baz();
> >
> > what's intended? There's an indent, but it's partial. I would not want
> > to guess. But I agree with you that an invisible fixup would be
> > inappropriate.
> > _______________________________________________
> > coreboot mailing list -- coreboot@coreboot.org
> > To unsubscribe send an email to coreboot-le...@coreboot.org
_______________________________________________
coreboot mailing list -- coreboot@coreboot.org
To unsubscribe send an email to coreboot-le...@coreboot.org
