Add an auto formatter to composite + auto format existing code

[Others]

Summary of this Pull Request (PR)

This commit adds an auto formatter script to composite format in the root directory. It is based on clang format, and uses the rules in the .clang-format file. This might need a little bikeshedding to get right, and perhaps even some style guide changes. However I think it will be super valuable to get this merged, so we can start automatically formatting our composite code.

For the purposes of reviewing this, looking at .clang-format and format are absolutely key. And it might be a good idea to scan through some code to see if anything looks weird/not stylistically correct.

Intent for your PR

Choose one (Mandatory):

• [ ] This PR is for a code-review and is intended to get feedback, but not to be pulled yet.
• [x] This PR is mature, and ready to be integrated into the repo.

Reviewers (Mandatory):

(Specify @<github.com username(s)> of the reviewers. Ex: @user1, @user2) @gparmer

Code Quality

As part of this pull request, I've considered the following:

Style:

• [x] Comments adhere to the Style Guide (SG)
• [x] Spacing adhere's to the SG
• [x] Naming adhere's to the SG
• [x] All other aspects of the SG are adhered to, or exceptions are justified in this pull request

Code Craftsmanship:

• [x] I've made an attempt to remove all redundant code
• [x] I've considered ways in which my changes might impact existing code, and cleaned it up
• [x] I've formatted the code in an effort to make it easier to read (proper error handling, function use, etc...)
• [x] I've commented appropriately where code is tricky
• [x] I agree that there is no "throw-away" code, and that code in this PR is of high quality

Testing

N/A. I just ran the current runscripts to verify everything still is semantically correct.

[Others]

Limitations:

• Breaks the way we currently do short functions on a single line
• Doesn't enforce / / comment style

Broken things:

• Spaces after ;s in for loops
• Lining up two subsequent bodies of conditionals
• enums format different from before (although consistently)
• Weird assignments like free = lower = m->id_boundary; format strangely (although, again, consistently)
• Macro definitions don't auto align (I suspect this is because it could cause semantic changes in some corner case)

[gparmer]

One more thing it breaks: assembly strings in macros. Perhaps we can manually surround these with /* clang-format off */ assembly bs /* clang-format */

[gparmer]

You should also add documentation for this. Likely:

• A file in https://github.com/gparmer/composite/tree/ppos/doc/style_guide/ that spells out 1. how to install llvm, and 2. apply the formating. It should have a link to https://clang.llvm.org/docs/ClangFormatStyleOptions.html
• Add a link to that file in https://github.com/gparmer/composite/blob/ppos/.github/CONTRIBUTING.md
• add a link after "Comments adhere to the Style Guide (SG)" in https://github.com/gparmer/composite/blob/ppos/.github/PULL_REQUEST_TEMPLATE.md,

I'm sorry this is a pain, but there isn't much reason to do all of this if we don't integrate it into the process to some degree. I agree that the build system can be later, but some documentation is required.

Also, add an issue to update the style guide with the style as determined by llvm. I can do that later.

[gparmer]

Possible changes:

BinPackParameters: true Cpp11BracedListStyle: true MaxEmptyLinesToKeep: 1 SpacesInSquareBrackets:false

[gparmer]

Last comment: format should be format.sh and should likely be in tools/

[Others]

@gparmer: Okay, pushed some changes, but still have a few remaining comments:

• I left MaxEmptyLinesToKeep at 2, so you can have two spaces between functions (which is current style I think)
• Since format is meant to be used from the command line ./format, I think the .sh postfix is against bash convention in this case
• I've refrained against putting it in tools for now as well, since I want to save that sort of change until we do the build system integration Of course all three of these notes are negotiable. I'd like to get this merged now, but I'm happy to change more things :)

[gparmer]

@Others

• maxempty lines: fine.
• google style guide says .sh is fine, and required when it is useful for documentation (i.e. for libraries). In our case, I see that file mainly as documentation, so I'd rather make it clear it is a text file.
• So pollute top-level with a file that is a tool? I don't understand this at all. Make sure to change the documentation to say to run the formatter as ./tools/format.sh

Did you remove ck from the formatting? Doesn't look like it; not sure?

Did you add the comments to avoid formatting around assembly definitions? Looks like no? At least add documentation to the autoformatter file for how to do this, and note that it might be required around defines that include assembly.

[Others]

@gparmer I fixed the assembly by excluding them in the script. Is that fine or do you want them specifically excluded with comment pragmas?

I'll fix the rest of your notes ASAP

[gparmer]

Are we talking about the same thing. You obviously didn't apply this to assembly files.

But you did apply it to C files that have #defines that include and generate assembly. And for the assembly contents of those defines, the indentation gets messed up. I noted this in my original comments.

[Others]

@gparmer yes I know. I specifically excluded the .h files that had the assembly in question. But this is probably the wrong thing to do now that I think about it. I'll switch to using comment pragmas instead.

[Others]

(Also ck was never formatted as far as I can tell by diffing against upstream/ppos.)

[gparmer]

This is brilliant! Thanks @Others for automating this important aspect of our development. Very very much appreciated.