Haiku coding guidelines and haiku-format

Development
1

I’m in the process of finalizing haiku-format 17.0.1 which is based on the same version of clang-format. Before I can do that, a few haiku-format 10.0.1 issues not mentioned in the Haiku coding guidelines should be decided on by the Haiku developers:

Another issue is about inserting/removing braces. My impression is that Haiku’s is closely aligned with LLVM’s and can take advantage of the InsertBraces and RemoveBracesLLVM options, with only minor customizations to the latter.

Finally, does Haiku want to use the IntegerLiteralSeparator option?

7 Likes
2

The two changes proposed by nielx were not discussed with other members of the team.

I think the BeforeCatch one makes sense.

However, I think removing the space in range-based for loops is strange, because we always have space after the other control structure keywords (if, while, non-range for, …). What is the reasonig for introducing a special case here?

As for IntegerLiteralSeparator: using it would break gcc2, so I think it should not be enabled.

Finally for the space between new and std::nothrow: that is indeed not in the current guidelines, but is what’s mostly used in Haiku (2596 instances). The version with space is used 300 times, so, some people do like it. There is also a further variant now that I look at the result:

    new(std::nothrow) type();
    new (std::nothrow) type();
    new (std::nothrow)type();

and also sometimes the final parentheses are omitted when the constructor doesn’t need parameters.

Personally I’m fine with either the first or second, but for haiku-format I think we have to make a choice. Any opinions either way? I can add it to the coding guidelines once there is an agreement.

3 Likes
3

I prefer this one.

There are also some discussions about lambda syntax: https://review.haiku-os.org/c/haiku/+/6976.

3 Likes
4

Agreed, I think we should keep the space.

Seconded.

2 Likes
5

I think removing the space in range-based for loops is strange

I think @nielx was proposing that the space after the colon be removed. The lack of a space after for was probably a typo.

1 Like
6

I prefer the space there. Removing it makes it look like a label, and is asymmetrical for no apparent reason :slight_smile:

2 Likes
7

Ah, yes, before the colon :wink: . That makes sense, I see how both choices can be defended.

I guess with the current Haiku rules, there should be a space before and after, just like other operators? But we already have some exceptions (the unary operators &, * and -) so we could add more.

I’d get used to either choice, especially if haiku-format takes care of it for me!

1 Like
8

I’ve updated haiku-format with the following:

(SpaceBeforeRangeBasedForLoopColon is not turned on because @axeld didn’t like it.)

Now what about inserting/removing braces? The coding guidelines have nothing specific about nested single-line statements. For example:

while (i--) // depth 0
	if (a)
		f(); // depth 2

If braces should be omitted as shown, will there be an upper limit on the nesting depth?

2 Likes
9

I think braces should be added on the while in a case like that.

1 Like
10

Our coding style is pretty explicit there: any multi-line statement needs to be enclosed in braces. The ‘if’ does not need them, but the ‘while’ does.

2 Likes
11

I have just added insertion and removal of braces based on feedback from @waddlesplash and @axeld.

One last question: can the line ending requirement be added to the guidelines? It’s set to the Unix line ending (\n) in haiku-format right now, i.e. LineEnding set to LF.

1 Like
12

That seems obvious enough that we didn’t think of writing it in the guidelines :slight_smile:

2 Likes
13

I was really just trying to confirm whether Haiku wanted to convert all line endings to \n or to leave them alone. This is no more obvious to me than whether to insert the missing \n before EOF, which is mentioned in the guidelines.

14

I think there is a distinction between multiple statements (simple or compound) and a multiline statement. Inserting/removing braces only applies to a single statement (either simple or compound), whether this single statement is split into multiple lines.

For example, braces should be inserted for the if (but not the while) statement below even though the inserted braces would only enclose a single line:

if (a)
	while (b);
15

In this case the ; for the while belongs on its own line anyway.

1 Like
16

For multiple statements, if I understand correctly, the C and C++ specifications already require to have braces (otherwise the second statement is not inside the if/whole/for).

The Haiku guidelines require braces, additionally to that already required by the language, also for a single statement that spans multiple lines.

1 Like
17

Yes, but only for Haiku though.

18

Exactly! That’s why I said “inserting/removing braces only applies to a single statement”. :wink:

1 Like
19

I’m done with upgrading haiku-format to v17.0.1. See GitHub - owenca/haiku-format and Release haiku-17.0.1 · owenca/llvm-project · GitHub.

6 Likes
20

Thanks for updating haiku-format. I have used it previously on my libnetservices2 (and I don’t think anyone had any comments on the style, so that’s good ;-))

I wonder for longer term how many patches for individual style choices we can upstream, as well as if it is worth trying to submit the Haiku style as a default style.

And for the other way around: if there are particular parts of the Haiku style that are too controversial for the maintainers of clang-format, or if we think they might be a bit exotic, what are the chances that we are willing to change our coding style?

I think it would be interesting to start using the tool as part of our automated checks when patches get uploaded to Gerrit. It would save a lot of comments about formatting, and automate checking a decent part of our style guide.

3 Likes