C code auto-formatting

[Dmitry Vyukov]

Hi,

I am looking into using clang-format for auto-formatting of C code in
runtime and commands:
http://clang.llvm.org/docs/ClangFormat.html

Here is an example of what it currently does with runtime:
https://codereview.appspot.com/9222047/

I've talked to developers and they are willing to accept more style tweaks.
After briefly looking at diffs, the main things are:
- return type on function definition must be on a separate line
- no space between "if ("
But most likely there are still will be differences, because some
formatting rules that we use are difficult to express, e.g. spacing in
"n*m + x". But I think benefits of stable auto-formatting outweigh it.

If we incorporate it into presubmit, all C hackers will need
clang-format in PATH.

What do you think?

[Ian Lance Taylor]

I am very much in favor of using clang-format.

However, I'm not sure we quite want to take the step of making it a
presubmit requirement, at least not yet. I think it's a good idea in
principle but I don't want to force all the runtime developers to
install it. We can always do a periodic formatting sweep. Once we
are more familiar with it perhaps we can enforce it.

The formatting it produces is definitely pretty different. We'll need
to make sure rsc and ken are comfortable with it.

Ian

[Dmitry Vyukov]

On Wed, May 15, 2013 at 5:16 PM, Ian Lance Taylor <ia...@golang.org> wrote:
> On Wed, May 15, 2013 at 2:38 AM, Dmitry Vyukov <dvy...@google.com> wrote:
>>
>> I am looking into using clang-format for auto-formatting of C code in
>> runtime and commands:
>> http://clang.llvm.org/docs/ClangFormat.html
>>
>> Here is an example of what it currently does with runtime:
>> https://codereview.appspot.com/9222047/
>>
>> I've talked to developers and they are willing to accept more style tweaks.
>> After briefly looking at diffs, the main things are:
>> - return type on function definition must be on a separate line
>> - no space between "if ("
>> But most likely there are still will be differences, because some
>> formatting rules that we use are difficult to express, e.g. spacing in
>> "n*m + x". But I think benefits of stable auto-formatting outweigh it.
>>
>> If we incorporate it into presubmit, all C hackers will need
>> clang-format in PATH.
>>
>> What do you think?
>
> I am very much in favor of using clang-format.

Great!

> However, I'm not sure we quite want to take the step of making it a
> presubmit requirement, at least not yet. I think it's a good idea in
> principle but I don't want to force all the runtime developers to
> install it. We can always do a periodic formatting sweep. Once we
> are more familiar with it perhaps we can enforce it.

SGTM

> The formatting it produces is definitely pretty different. We'll need
> to make sure rsc and ken are comfortable with it.

I've just tuned currently available parameters to match our code style
as much as possible. clang-format was developed with Google/Chromium
code style in mind, so there is just that much tuning as of now. We
can add the style tuning we need, at least the 2 points I noted above.
Then the code will look much closer to what we have now.

[Anthony Martin]

Here's the things I don't like about it. You already called out
most of them.

1. In struct definitions it doesn't align the field names and it
sticks the '*' for pointers onto the field instead of the type
where it belongs.

3. It inserts too many spaces. I'd rather it be like gofmt and
format "a + x * y" as "a + x*y".

2. The return type of a function definition is on the same line.
This is really annoying if you're using Acme. You can no longer
get to a function by plumbing filename:/^function.

4. Again with the spaces. This time after "if", "while", "for",
and "switch".

5. Comments are handled in an odd manner. See line 158 of the
malloc.goc file in your CL.

Other than that, I think it's a fine idea.

There's a program from Plan 9 called cb which already formats C
code into our style. Alternatively, we could just use that.

Anthony

[Aram Hăvărneanu]

Passing the C compilers through a formatter will probably make

importing changes back into Plan 9 more annoying.

I would love a formatter though, why not just use Plan 9 cb(1)?

[Dmitry Vyukov]

On Wed, May 15, 2013 at 5:43 PM, Anthony Martin <al...@pbrane.org> wrote:
> Dmitry Vyukov <dvy...@google.com> once said:
>> I am looking into using clang-format for auto-formatting of C code in
>> runtime and commands:
>> http://clang.llvm.org/docs/ClangFormat.html
>>
>> Here is an example of what it currently does with runtime:
>> https://codereview.appspot.com/9222047/
>>
>> I've talked to developers and they are willing to accept more style tweaks.
>> After briefly looking at diffs, the main things are:
>> - return type on function definition must be on a separate line
>> - no space between "if ("
>> But most likely there are still will be differences, because some
>> formatting rules that we use are difficult to express, e.g. spacing in
>> "n*m + x". But I think benefits of stable auto-formatting outweigh it.
>>
>> If we incorporate it into presubmit, all C hackers will need
>> clang-format in PATH.
>>
>> What do you think?
>
> Here's the things I don't like about it. You already called out
> most of them.
>
> 1. In struct definitions it doesn't align the field names and it
> sticks the '*' for pointers onto the field instead of the type
> where it belongs.

Probably we can introduce a parameter for this.

> 3. It inserts too many spaces. I'd rather it be like gofmt and
> format "a + x * y" as "a + x*y".

WIll it do if we just split all operators into 2 groups -- insert
space around, and do not insert spaces around?

> 2. The return type of a function definition is on the same line.
> This is really annoying if you're using Acme. You can no longer
> get to a function by plumbing filename:/^function.

We need a parameter for this. It is quite common style.

> 4. Again with the spaces. This time after "if", "while", "for",
> and "switch".

We need a parameter for this.

> 5. Comments are handled in an odd manner. See line 158 of the
> malloc.goc file in your CL.

This actually looks like a bug. I will ping developers.

> Other than that, I think it's a fine idea.
>
> There's a program from Plan 9 called cb which already formats C
> code into our style. Alternatively, we could just use that.

Can you post a link to more info about cb? Does it work on other OSes?

Btw, is there some "official" name for the style? Plan9?

[Dmitry Vyukov]

Does it work on other OSes? Link?

[Dmitry Vyukov]

Yes, there is an opened bug for this.

[Aram Hăvărneanu]

> Can you post a link to more info about cb? Does it work on other

> OSes?

Manual: http://plan9.bell-labs.com/magic/man2html/1/cb

Source: http://plan9.bell-labs.com/sources/plan9/sys/src/cmd/cb/

It's also part of plan9port, it would be trivial to make it work

outside of plan9port.

[Dmitry Vyukov]

Can somebody who has it installed reformat some files in runtime and
upload the diffs?
If it's style is close to what we have now, I think it's a better idea
than clang-format.

[Anthony Martin]

Dmitry Vyukov <dvy...@google.com> once said:

I tried using cb but it has a few formatting bugs we'd have to
work out (mainly having to do with structure initializers). It
might be less trouble to use clang-format. I'll see how easy
it'll be to fix the bugs.

Anthony

[Robert Griesemer]

I'm probably the odd man out here (given that I've written gofmt) - but it seems to me we shouldn't make building (better: working on) the runtime system more complex by requiring a dependency on clang-format. For one, the number of people working on the runtime is small compared to the number of people working in Go. This feels like busy-work to some extent.

Instead, I'd rather see the energy going intro trying to write much of the runtime in Go. That would be a qualitative difference.

- gri

--

---
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Russ Cox]

I agree with Robert. There's not enough C code to be building new complex mechanisms here. Please let this go.

Russ

[Rob Pike]

The three R's agree.

-rob

[mortdeus]

Ill  port cb to go asap. Its already something that Ive been meaning to do for https://github.com/mortdeus/goblin anyways.