[flang-dev] Code formatting alignment with LLVM

[Mehdi AMINI via flang-dev]

On Mon, Mar 2, 2020 at 9:57 AM Michael Kruse via flang-dev <
flang-dev at lists.llvm.org> 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).
>

MLIR also use camelCase, mostly because we tried to anticipate where the
wind was going on this and since lld was getting renamed at the time... It
didn't pan out in LLVM so far unfortunately.


>
>
> 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).
>

In general this is correct, in practice lldb and lld both got a one-time
large scale renaming/formatting I believe.


> 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'?
>
>
> > 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.
>

I don't know, but I agree. This got merged unnoticed, otherwise I'd have
removed it when merging MLIR into LLVM (or before). I just sent
https://llvm.discourse.group/t/remove-mlir-custom-clang-format-configuration/647
FYI.

Thanks,

-- 
Mehdi


> 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
> _______________________________________________
> flang-dev mailing list
> flang-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/flang-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/flang-dev/attachments/20200302/97920e0c/attachment.html>