Enforce clang-format on the entire code base.

[fruffy]

This PR enforces clang-format on the entire P4C code base. This is a one-and-done procedure since we already have an automated script There are pros and cons to this approach.
Pros:

• Once this is done the entire code base will have an consistent style that can be tweaked with a simple config file.
• Style can be checked on every PR, which reduces the amount of effort needed for manual style comments.
• clang-format is aligned with cpplint and will automatically fix cpplint complaints

Cons:

• This PR definitely will pollute git history and it will make it harder to git blame specific sections of the code going forward.
• Any potential clones/forks of P4C will have a hell of a time trying to resolve merge conflicts.

Note that the ir folder is not checked yet. The reason is that the includes there are set up in a very specific manner. I have a PR that fixes this, which I can merge in case this PR is approved.

[davidbolvansky]

This PR definitely will pollute git history and it will make it harder to git blame specific sections of the code going forward.

Well, not a huge problem in practice with .git-blame-ignore-revs. If you use docker images to develop p4c, this "issue" is easy to fully work around, as you need to run git config blame.ignoreRevsFile .git-blame-ignore-revs and done.

https://www.stefanjudis.com/today-i-learned/how-to-exclude-commits-from-git-blame/

[fruffy]

This PR definitely will pollute git history and it will make it harder to git blame specific sections of the code going forward.

Well, not a huge problem in practice with .git-blame-ignore-revs. If you use docker images to develop p4c, this "issue" is easy to fully work around, as you need to run git config blame.ignoreRevsFile .git-blame-ignore-revs and done.

https://www.stefanjudis.com/today-i-learned/how-to-exclude-commits-from-git-blame/

Oh interesting, I did not know about this. So should we add this file after we have merged this PR? With the appropriate commit ID.

[davidbolvansky]

Yes, +1

[mihaibudiu]

It's not a good use of human time to review this kind of change.
If you guarantee it's equivalent that's good enough.
But you have to automate this going forward, otherwise we'll have again at some point a similar PR.

[fruffy]

It's not a good use of human time to review this kind of change. If you guarantee it's equivalent that's good enough. But you have to automate this going forward, otherwise we'll have again at some point a similar PR.

Yes, there is no expectation to review every single file. The clang-format changes are expected to be semantically equivalent.

This check actually does automatically enforce the right format on every file going forward. So a big PR like this should not be necessary anymore, unless we change the format file.

[jnfoster]

This is good and an improvement to our hygiene. Let's wait to merge it until we have clear consensus though.

[fruffy]

@mbudiu-vmw Any other thoughts on this? Once this is in, all PRs should be checked with clang-format. #3701 is a follow-up that cleans up the includes of the IR files so that clang-format can be enforced there.

[mihaibudiu]

No other comments.

[mihaibudiu]

Prehaps a pre-commit hook should check the format, otherwise we'll have lots of failed jobs.

[jnfoster]

🙌