cpplint / clang-format / clang-tidy

[HomesGH]

To keep the same style in this project and also to prevent some simple bugs, some kind of (static) code analysis should be added. This can be e.g. cpplint or clang-format and/or clang-tidy. A first test was done in PR #260.

Implement:

• clang-format -> see Modernize clang-format file #342
• clang-tidy or cpplint

[amartyads]

I've come up with a .clang-format file that mirrors the existing code as closely as possible, after the discussion in #339 . The intention is to use this for all new files in ls1, until we standardise something someday.

ls1-clang-format.txt

To use:

• copy the file to the folder where you want to run the command from
• rename file to .clang-format
• run clang-format -style=file -i <path to files you wan to format>

NB:

• -style=file is not path to config file, it means "hey clangformat, look for a file called .clang-format in current folder and use that"
• the output of clang-format is not consistent between (major) versions, and I'm using clang-format version 18.1.8.

Important features include:

• all unchanged options are LLVM defaults, all changed options have the default in a comment, so you can ctrl+F for # and see all changes
• tabs instead of spaces (joker laugh)
• line width 120
• try to have return type on same line as function name
• opening braces on same line
• have ending else and default on new line instead of } else {

Anyone else have any suggestions?

[FG-TUM]

Please create a PR with that file where you also apply it so we can have a closer look. I'd suggest we discuss this at the next in-person meeting and see if we can get everyone on board.

Which clang-format version are you using? (If I could choose freely I'd vote for 14 because that's what we use in AutoPas and thus makes things more straight forward for us but anything is fine :) )

Bonus points if you also add a:

• CMake target so we can simply do make clang-format.
• CI action that checks if the branch is properly formatted.

[amartyads]

Okay, so after looking at the style guide and the older clangformat, I think there's a few more points to discuss here. All of this is clangformat 18.

The most salient point seems to be that the older clangformat is based on Google, while the new one is based on LLVM. The most important differences I could see are below. However, the actual code has examples of both types of formatting.

1. Google wants the & and * of pointers and references to be attached to the typename, while LLVM wants them to be attached to the variable name (PointerAlignment: Left vs Right). The style guide agrees with Google (at least, that's my interpretation of point 2 in the pdf). I think we prefer LLVM. Left Right
2. Google indents case labels, while LLVM doesn't(IndentCaseLabels true vs false). I think we prefer google. NoIndent Indent
3. Google half-indents access modifiers, LLVM doesn't(AccessModifierOffset). I think we prefer LLVM. Aligned Not aligned
4. AllowShortLoopsOnASingleLine : true for Google, false for LLVM, I have no preferences
5. AllowShortIfStatementsOnASingleLine: true for Google, false for LLVM, I like LLVM
6. IncludeBlocks : Regroup for google, Preserve for llvm.

I don't think the other differences are relevant. However, if others are curious, they are AlignEscapedNewlines, AlwaysBreakBeforeMultilineStrings, AlwaysBreakTemplateDeclarations, DerivePointerAlignment, some stuff in IncludeCategories, IncludeIsMainRegex, KeepEmptyLinesAtTheStartOfBlocks, ObjCBinPackProtocolList, PackConstructorInitializers, PenaltyBreakBeforeFirstCallParameter, PenaltyReturnTypeOnItsOwnLine, SpacesBeforeTrailingComments and Standard.

Something that is contrary to both standards, is the newline before an else. The BreakBeforeBraces style is Attach for both, meaning that the else is on the same line. However, this is inconsistent in many parts of the code, such as here.

Now of course, it doesn't matter what we choose as the base style, because we can just edit one style into another. But I guess we should still agree on these specific details. I can keep the base style as google, to match the older clangformat file.

I relist the changes I originally made here. These will stay, regardless of base style.

AllowShortBlocksOnASingleLine: Empty
ColumnLimit:     120
PenaltyReturnTypeOnItsOwnLine: 2000000
TabWidth:        4
UseTab:          Always

So there's 5 settings already agreed upon, and 6 settings we have to talk about. What preferences does everyone have?

[FG-TUM]

1. I agree with you.
2. I agree with you.
3. I agree with you.
4. I'd opt for false / LLVM because this is then more consistent with 5.
5. I agree with you.
6. I prefer regrouping.

Can you add a comment in the style file on the purpose of PenaltyReturnTypeOnItsOwnLine? This is not the most intuitive setting...