RFC Coding Guidelines: Formatting Class Member Declarations | Haiku Project

Blog
45

I started haiku-format as an unofficial GSoC project back in 2018 because I didn’t want to wait another few years before I would become eligible. Many ideas I had then eventually made it into clang-format itself and thus ended up in the current haiku-format after I made it a superset of clang-format. As I already started a full-time job last year, I probably can’t add more customizations to haiku-format. (Luckily, I think customizing haiku-format further would have diminishing or even negative returns.) Nevertheless, I’ll upgrade haiku-format again after LLVM/clang-format 18.1 is released next month, and keep maintaining it for the foreseeable future.

Here is a preview of what’s new in haiku-format 18.1.x:

2 Likes
46

Hi everybody. Thanks for the comments and discussion. I want to let everyone know that I have updated the proposal:

  • The proposal now covers all function and variable declarations, so including the ones outside of the class definition. This is both for pragmatic reasons (changing haiku-format would automatically apply to non-class members) as well as the finding that currently this is also not consistent.
  • In course of the community discussion, there was a desire to see if there was an alternative that kept the spirit of the aligned declarations. This proposal therefore gives two options, namely the Normalized Declaration Style and the Aligned Declaration Style. Both can be implemented with haiku-format as it currently is.
  • The discussion also reviewed the implementation process. My takeaway was that there is a strong preference for a single style (and to not have a legacy option), so the implementation kill the exception for the legacy style. There was no consensus on whether a big one time reformat was in order, so this proposal punts on that point and only applies to new code.

I am going to leave this open for a day or two for comments, and then I will progress it for a vote. No matter what the outcome, your input has been valuable!

2 Likes
47

A new option in clang-format might take care of this. See here.

  • Running it over Window.h gives some strange results (i.e. suddenly a new block starts and the alignment changes without any reason), but that must be a bug so that can be fixed.

The clang-format AlignConsecutiveDeclarations option has a number of bugs, some of which are long-standing. It’s safer to assume that they won’t get fixed anytime soon.

So the Aligned Declaration Style doesn’t seem to be a viable option.

48

Maybe I’m misunderatanding how the tool is supposed to work but I have a fundamental dislike of tools that emit only one option, and that is the source of truth.

Say we would allow two options in the style of code, but clang format always picks one, and thus that is the only allowed style makes no sense to me.

I’ve had that recently with inline declarations of one-liner functions in class headers.

I want to put those on 4 lines for readability, clang-format wants to put it in one line.

49

Current Haiku code guidelines are also very rigid and I got rejected when submitting patches with custom formatting for better readability. Cor example table aligned switch-case statements. It also dictate 2 blank lines between each function, but in my personal C++ code style I usually use one blank line and 2 blank lines to separate function groups.

2 Likes
50

I think it really depends on precedent. We should have a somewhat consistent style, but if it in edge cases makes reading code easier I’m all for it.

The two blank lines is mandatory, yes. And should not be changed. Something like that helps “reading” code visually, to find functions. and only works when consistently applied.

51

I think this really is one of the fundamental debates, and there is no one true answer. At the one extreme end there is the point of view that the style is whatever the tool says it is, at the other end there is the position that readable code is a human endeavor and a tool can merely advise (or do the most basic of formatting corrections).

In the former camp are tools like rustfmt (which formats the code based on the AST output that the compiler creates) and python black, which really preach ‘there is one true code style’ and leave very little to no formatting choices for a developer.

I think clang-format is slightly less fundamentalist about the uniformity than the other two, though it is still quite opinionated about a lot of things.

To your point, part of this proposal is that it assumes that we want to go for uniformity (and the advantages that it brings) over giving an individual developer to make style choices that are subjectively and/or objectively more readable in their particular instance.

My personal opinion is that both overall uniformity of code and the value automation adds is greater than the places where custom formatting gives the best readability. But your opinion can be different and it can be a reason to be against this proposal.

52

Thanks everybody for the contributions in this thread! These discussions have sharpened my thinking on the topic, and they have been invaluable in refining the change proposal. I am closing the comment period, and I have moved forward with a vote on the haiku-development mailing list. Outcome in 15 days!

53

In Haiku we use two blank lines between functions and #pragma mark to separate function groups (possibly in a comment because compilers don’t implement this pragma anymore, sadly).

In GCC they use an ASCII page break character between function groups, so if you print out the code, it will start each group on a new page.

I’m not usre what your point is here. There are different coding styles in different projects? Yes, we already knew that.

And yes, the coding style is strict. That’s the point of having a coding style. Having all the code formatted the same way makes it easier to read if you are used to this style. Any deviation from it requires extra effort to understand the code. Of course, if you are used to another style, you think the opposite is true. There is no perfect solution here.

There isn’t, but there are compromises possible. In the case of clang-format, if you find the automated formatting not satisfying, you can use some markers in the sourcecode (clang-format-disable) to disable it and have it ignore these sections of the code. If you do this a lot, it will surely raise some eyebrows from the reviewers. But if you do it sparsely, I think it is acceptable. We can also periodically look into these and see if we need to tweak some clang-format settings.

Well that’s unfortunate. In that case, the automated formatting degrades readability. I’m not sure I’m willing to compromise readability so much to automated formatting…

54

It is my objection to Waddlesplash’s opinion:

There are no “flexibility for human” in current Haiku style. It is strict in the same way as clang-format style. Differences are only in personal sense of taste, it is not objectively better in any way comparing to clang-format. Both styles do not allow case-by-case formatting to improve readability.

Examples of potential case-by case formatting to improve readability:

	if ((attributes & B_USER_PROTECTION) != 0) {
		pte->flags |= (1 << pteUser);
		if ((attributes & B_READ_AREA)    != 0) pte->flags |= (1 << pteRead);
		if ((attributes & B_WRITE_AREA)   != 0) pte->flags |= (1 << pteWrite);
		if ((attributes & B_EXECUTE_AREA) != 0) pte->flags |= (1 << pteExec);
	} else {
		if ((attributes & B_KERNEL_READ_AREA)    != 0) pte->flags |= (1 << pteRead);
		if ((attributes & B_KERNEL_WRITE_AREA)   != 0) pte->flags |= (1 << pteWrite);
		if ((attributes & B_KERNEL_EXECUTE_AREA) != 0) pte->flags |= (1 << pteExec);
	}

void KeyboardHandler::KeyString(uint32 code, char *str, size_t len)
{
	uint32 i;
	char *ch;
	switch (modifiers & (B_SHIFT_KEY | B_CONTROL_KEY | B_OPTION_KEY | B_CAPS_LOCK)) {
		case B_OPTION_KEY | B_CAPS_LOCK | B_SHIFT_KEY: ch = chars.Get() + keyMap->option_caps_shift_map[code]; break;
		case B_OPTION_KEY | B_CAPS_LOCK:               ch = chars.Get() + keyMap->option_caps_map[code];       break;
		case B_OPTION_KEY | B_SHIFT_KEY:               ch = chars.Get() + keyMap->option_shift_map[code];      break;
		case B_OPTION_KEY:                             ch = chars.Get() + keyMap->option_map[code];            break;
		case B_CAPS_LOCK  | B_SHIFT_KEY:               ch = chars.Get() + keyMap->caps_shift_map[code];        break;
		case B_CAPS_LOCK:                              ch = chars.Get() + keyMap->caps_map[code];              break;
		case B_SHIFT_KEY:                              ch = chars.Get() + keyMap->shift_map[code];             break;
		default:
			if (modifiers & B_CONTROL_KEY)               ch = chars.Get() + keyMap->control_map[code];
			else                                         ch = chars.Get() + keyMap->normal_map[code];
	}
	if (len > 0) {
		for (i = 0; (i < (uint32)ch[0]) && (i < len-1); ++i)
			str[i] = ch[i+1];
		str[i] = '\0';
	}
}
55

In the Haiku style bot of these would require newlines after the case or the clising parenthese of the if. The code after the linebreak will then be properly aligned, and it won’t be so wide that it can’t fit on screen (especially if using multiple columns, for example in a side-by-side diff or if you just like to have multiple files visible at once).

It is not perfect, but it gives good enough results. And yes, I think it really helps that all the code is formatted the same.

You could make the same argument about syntax highlighting. Do you think that people should edit their code in StyledEdit and manually apply formatting? They could colorize different parts of the code differently, put really important code in bold or larger text, and so on. But, soon enough, you want some common rules so that all the code looks the same. And maybe you want some way to personalize it.

For synrax highlighting, this seems a strange question to ask. Why is it different for indentation?

And again, as I said, with or without tool, deviations from the style could be allowed. But there has to be a really good reason for it. I am not convinced by your example. I have to think about the syntax of the code a lot more. For example here is how I read your second example: my first thought is, is the break in that swich/case missing? Ah no, it’s there, but it’s in a strange place on the right. While thinking about that, I lose track of the semantics.

After years and years of reading code always formatted the same way, I am trained to subconsciously parse it. But this training is not as good as that of a C++ compiler, and I rely a lot on the indentation, the variable naming convention, etc. This allows me to navigate the codebase and understand the code quickly. Everything different from that convention disrupts that flow and requires me to switch to a slower, more careful parsing of the code to understand it.

Again, I am not absolutely opposed to deviations. But it has to be really, really worth that extra effort. In your examples, personally I think it isn’t.

56

It is literally done in this way for many Oberon dialects (I have a lot of experience of working with them). Source code is stored in special binary format that support formatting. This approach have some advantages, for example mark some code parts with a different color or insert illustrations directly in source code. Biggest disadvantage is a bad integration with source version control systems such as Git.

image
image792×909 33.7 KB

See: GitHub - X547/BlackBox-Haiku: BlackBox Component Builder port to Haiku operating system.

57

While I have explicitly taken the position to only suggest styles that are supported by clang-format, I would not rule this out because of bugs. It is a valid configuration flag, so if this is your preferred style, then I’d be more than happy to raise issues with the llvm project, and maybe even look into if it is fixable myself. I just don’t want to develop/maintain customizations.

58

FYI: the proposal has been reviewed on the haiku-development mailing list.

Based on the low number of responses, I will withdraw the proposal. This also means that I will abandon the effort to work on automated code formatting. I have archived the haiku-format-bot repository and I have marked the RFC blog post as withdrawn.

59

That’s a shame. I’ll hold off upgrading haiku-format then.

60

I am a bit confused by nielx decision to abandon it all. I have found the bot very useful as a way to improve code formatting, save time on code reviews (even in its current imperfect version), and also show inconsistencies between clang-format and the coding style (of which some will require improving clang-format, and some will require changing the coding style).

Even if there is no short term decision of changing the coding style, I think the availability of the bot was a net improvement, and that it should not be archived, but kept running, and we should keep improving it. I’m happy to help where I can.

7 Likes
61

I agree, the work is useful and brings many benefits to code review!

62

I can understand the frustration.

I have found the bot very useful as a way to improve code formatting, save time on code reviews (even in its current imperfect version), and also show inconsistencies between clang-format and the coding style (of which some will require improving clang-format, and some will require changing the coding style).

You mean haiku-format?

Even if there is no short term decision of changing the coding style, I think the availability of the bot was a net improvement, and that it should not be archived, but kept running, and we should keep improving it.

Before putting more effort into haiku-format-bot, Haiku devs must come to terms with haiku-format or at least reach a consensus on what to do with the two 2-year-old blocking issues, one of which this RFC attempted to address. Even without the bot, haiku-format can be used with git before (or after) patches are submitted on gerrit. See a demo here.

63

It is unfortunate that many developers are still stay for current old-fashioned table based class declaration that is not seen in modern C++ projects and incompatible with new C++ features. It is also less readable IMHO. Negative responses kill motivation to do something.

3 Likes
64

What is that response supposed to be? You aren‘t allowed to say no on a vote because it kills motivation? Then why have a vote in the first place?

For my part I made it pretty clear why I voted no: changing this to conform to a tool that always reformats everything even if it doesn‘t know how to do it correctly isn‘t the answer. This leaves us in this all-or-nothing situation of now. All obviously doesn‘t work when the developers involved say they don‘t wish to change clang-format further.

So now instead of taking the parts that work and beeing happy with it (like the bot making some parts much easier in code review) We now pick nothing. Fun.

Also old-fashioned and modern don‘t mean anything. Those are just words to attack things you don‘t like personally.

1 Like