[flang-dev] Code formatting alignment with LLVM

[David Truby via flang-dev]

Hi Michael,

Thanks for the reply, I’ve added some comments inline below.

David

> On 2 Mar 2020, at 17:55, Michael Kruse <llvm at meinersbur.de> wrote:
> 
> Thank you for the summary. Some more things to consider:
> 
> Other than the clang-format file, there are other code style
> differences not covered by clang-format. For instance, the naming
> scheme for local variables. LLVM uses PascalCase, but lld uses
> camelCase (although there were discussion to change this for LLVM as
> well).
We have discussed some of these differences too, for example no else after early return and no braces around single line if statements. My understanding is that these are harder to change automatically so we need to have a longer discussion about those which won’t happen in time for the merge. 
In this specific case we currently use camelCase for local variables, for the same reason as lld and mlir.
> 
> 
> Am Mo., 2. März 2020 um 09:08 Uhr schrieb David Truby via flang-dev
> <flang-dev at lists.llvm.org>:
>> Tim Keith and Peter Klausler from Nvidia replied to say that comment alignment (and code alignment in general) cause spurious changes to a appear when submitting diffs, making it more difficult to see what has changed. Tim Keith also pointed out that clang and llvm are not properly clang-formatted entirely themselves, which weakens the “common code style across the board” desire.
> 
> LLVM does not re-format code on a global scale. Large formatting-only
> changes are not done (also see https://reviews.llvm.org/D50055).
> Otherwise a change of clang-format would require re-formatting of the
> entire code base.
> 
> Only changed lines in patches are re-formatted, e.g. with 'git
> clang-format'. There should not be spurious changes in submitted
> diffs.
> 
> As far as I see, this is the only argument against a vanilla
> 'BasedOnStyle: LLVM'?
> 
I am hoping we can do a one-off global reformat, the same as lld and lldb did before merging, so that we match the LLVM style.
> 
>> Eric Schweitz replied to show MLIR’s clang-format file, which is also the one used for the lib/Lower subdirectory of f18. This style has only one small change from the LLVM style, but it is nonetheless a diversion. Eric also pointed out that clang-formatting for new code is enforced on Phabricator by pre-commit bots. This is the case not only for mlir, but for new code heading to clang and llvm too. This is consistent with the Coding Guidelines which do not require that all code is properly clang-formatted, only all new code. Eric also suggested that we could submit diffs without running clang-format, so that the changes are clear, and then once approved run clang-format to match the code with the rest of the project. This might mitigate the code alignment issues.
> 
> Is there a reason why MLIR added 'AlwaysBreakTemplateDeclarations:
> Yes'? I'd prefer to not deviate from any LLVM standards/policy unless
> there is a project-specific reason. If the reason is not
> project-specific, it should be discussed for the entire LLVM project.
> Not there is another current thread about deviating from the general
> LLVM policy: https://lists.llvm.org/pipermail/llvm-dev/2020-February/139381.html
> 
> Michael