Coding standard and clang-format file

We recently checked in and formatted the code with clang-format. It was brought to our attention that some of the changes we made were vague or absent in our published standard. I thought I would bring them up here so we could discuss. Sorry for the length of this post. The clang-format file is lengthy. My suggestions are mine alone, not representative of Altair. For more information than I provide: Clang-Format Style Options — Clang 13 documentation

This controls what to do with a line continuation. Should it be aligned with the open bracket/paren or a static indent. We chose to align with the open bracket, but this has a problem. One of the things our standard specifies is that we use tabs as our indent. This was because someone could change change the tab-width in their editor if they want. The only way for clang-format to align brackets properly is to assume a tab width (we chose the default of 8). This causes line continuations to not look right if you change the tab width to lets say 4.

With that in mind, I suggest we change this to a static indent of one tab.

This controls if you align the declaration assignments. We were inconsistent on this, so we chose false. All assignments will come right after the variable name.

This talks about what happens with an escaped newline. Most likely this is for a preprocessor macro. The options are to align them close or far from the macro in a line or don’t align them at all. Currently this is set to align them on the right far from the macro. It does add some space between the macro and the escapes, so it makes it obvious they are escaped. If we want to save on some horizontal space, we can change this.

This controls whether or not we align single line comments on lines after each other. This usually comes into play when there is a list of declarations at the top of a scope with comments describing each one. Currently this is set to true. This means things line up nicely, but it also adds horizontal space. I also noticed it isn’t perfect, so if you have a bunch of variables with comments, then one without, then more with comments, the second set will be at a different indent level.
I suggest we turn this off.

This will control if short one line functions are merged onto a single line. This is set to all functions.

Similarly this controls if short if statements are merged onto one line. This is set to never.

Similarly this controls if short loops are merged onto one line. This is set to false.

This controls if function arguments/parameters are bunched on one line or split one argument per line. This is set to true. I suggest we leave it. Having one line per function argument will eat up a lot of vertical space.

This means we’ll indent a case from the base switch (so indent the case, and then indent the block from that). It uses up more horizontal space, but I think it makes it easier to read.
It is set to true and I think we should leave it.

This keeps empty lines if they exist at the start of a block. This is set to true

This specifies the max number of empty lines to keep. It squishes things together. It is currently set to 1. That seems fine to me.

This will add a space after a logical not (e.g. ! foo vs !foo). It is currently set to false and I think we should keep it that way.

Sorry for the lengthy email. I listed the parts of the clang-format file that I felt were not expressed in our standard. We should clarify them if our clang-format file is going to format them that way.


Personally, I don’t like the backslashes way over at the right. Too easy to miss and forget when adding another line. The ENAS_Left option is more readable for me. But, it does have the same issues you mention for AlignTrailingComments.

Thanks for compiling this Bhroam. I agree with most of what you said, some comments:

What do you mean by “This is set to all functions.”? is it set to false ? That’s what I’d prefer.

Is there a middle ground here? some functions can have a lot of parameters, packing them on the same line might make it difficult to see them all. Can we make it so that it packs as many as would fit within a certain character length (say 100) and then move the rest onto the next line?

An opening block already has a line that’s empty except for an opening brace, why add one more empty line? I feel like this should be set to false.