Haiku coding guidelines and haiku-format

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

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

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

2 Likes

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.

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);

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

1 Like

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

Yes, but only for Haiku though.

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

1 Like

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

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

The previous experiments with haiku-format showed that it was not quite up to the task yet, sometimes making strange decisions about the formatting or introducting formatting errors in previously good code. That’s why it was not integrated further in the CI yet.

For reference, the two test commits (with two different versions of haiku-format) allow to review the changes made to Input preferences:

https://review.haiku-os.org/c/haiku/+/3826

https://review.haiku-os.org/c/haiku/+/4325

But, since this new version fixes several of the problems, it’s probably time for another attempt. I think one of the two changes above can be abandoned, and the other replaced with the current haiku-format results? If the results on Input preferences are good, we can then expand it to larger parts of the code and see if there are problems with the results.

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.

Argument of for gets aligned instead of indented · Issue #16 · owenca/haiku-format · GitHub is likely a bug in clang-format, and std::nothrow gets extra spacing · Issue #17 · owenca/haiku-format · GitHub may be a reasonable option to request upstream. Other Haiku-specific style options are unlikely to get approved as they don’t seem to meet the bar.

As to making Haiku one of the default styles like LLVM, Google, etc., it won’t happen. I (along with the other code owner) even rejected Chromium’s request to set InsertBraces to true in the Chromium 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.

I have updated the installation script in dc7ccf5 to generate git-haiku-format. Instead of running haiku-format -i changed_file.cpp to reformat the entire file, now you can run git haiku-format changed_file.cpp to only reformat the diff.

Quick question: haiku-format is for .cpp files only, right?
Because .h files get pretty mangled when run through haiku-format, esp. WRT indentation.

1 Like

Both should work, there is no difference in syntqx between .cpp and .h files. But our indenting/aligment of types and fields in class declarations is probably not handled yet? Please report any issues you find (with examples) if they are not reported yet.

1 Like

Done: Messing up indention of .h files · Issue #30 · owenca/haiku-format · GitHub

This is a known issue. I also mentioned it here. The short answer is to either use // clang-format off or change the Haiku coding style (or not use clang-format). IMO, although this style may be good for the pre-C++11 language standards, it probably can’t handle the function declaration syntax in later standards.

2 Likes

See here for a demo.

Thank you all for the feedback! As I said here, I’ll update haiku-format to 17.0.5 in about a month when the clang-format of the same version is expected to be released.

I just noticed that haiku-format binary is 10MB while clang-format is only 128KB. Is it intended?

1 Like

haiku-format is larger because it’s statically linked to libLLVM and libclang-cpp. libLLVM would probably be ok as a dynamic link but I think the llvm{12,16,17}_clang packages have conflicts between them and can’t be installed at the same time. This would force the user to only use llvm17 if haiku-format was installed, and not llvm12 or llvm16.

2 Likes