[flang-dev] Flang Coding Style

[Johannes Doerfert via flang-dev]

On 4/2/21 10:40 AM, Steve Scalpone wrote:
> Hi Johannes,
>
> I believe every file committed to flang runs through clang-format. There are three formats being used.  The front-end files use the llvm style with some modifications.  The files related to MLIR follow the MLIR style.  And the files closest to LLVM follow the LLVM style.  This all seems reasonable to me.
The webpage say "clang format of llvm 7" which is not what phabricator 
runs and not what we should announce as style guide.

Three formats doesn't strike me as reasonable. What is the benefit over 
two, or one, either would make reviews and code writing easier.
To play devils advocate, why stop at three, every folder could have a 
distinct style, where do we draw the line?

Note that this is not only about clang format here but also naming and 
other things (some of which clang-tidy would flag).


>
> There was a huge file-renaming pass done over the code before the initial merge.  Do you have an example that doesn't follow the convention?

I just run:
     for folder in {llvm,mlir,clang,flang}; do echo -n "Folder $folder, 
lower case:"; find $folder/lib -name '[a-z]*.cpp' | wc -l; echo -n 
"              upper case: "; find $folder/lib -name '[A-Z]*.cpp' | wc 
-l; done

Folder llvm,  lower case:6
               upper case: 2226
Folder mlir,  lower case:0
               upper case: 381
Folder clang, lower case:0
               upper case: 758
Folder flang, lower case:97
               upper case: 36

So llvm, mlir, and clang have basically no files starting with a lower 
case letter, flang has a 3:1 ratio of them.
Note that this is only checking /lib, other places need to be checked too.

Similarly, the advocated use of `-` (over `_`) neither is really used 
outside of flang:

Folder llvm,  # of '_' uses:7
               # of '-' uses: 0
Folder mlir,  # of '_' uses:0
               # of '-' uses: 0
Folder clang, # of '_' uses:0
               # of '-' uses: 4
Folder flang, # of '_' uses:0
               # of '-' uses: 64

So these are obvious differences when it comes to naming conventions, I 
did not spend too much time on it though.


>
> The compilation slowness may be related to heavy use of std::variant.  I'm not trying to be difficult here but perhaps you can bring up the compilation speed issue with cfe?  Michael Kruse measured the time required to compile flang with gcc, vc++, and clang; my recollection is that clang was slowest of the batch.

We can "blame" clang for being slow and even try to improve it, that 
doesn't change the fact that the "suggestions" online are in direct
contradiction to common practices across the project. It happens that 
these "suggestion" are also a likely candidate for the build time
issues and a reasonable reaction would be to backtrack and follow the 
path plotted by the rest of LLVM to avoid them.

~ Johannes


>
>   - Steve
>
> On 4/1/21, 1:51 PM, "flang-dev on behalf of Johannes Doerfert via flang-dev" <flang-dev-bounces at lists.llvm.org on behalf of flang-dev at lists.llvm.org> wrote:
>
>      External email: Use caution opening links or attachments
>
>      I was struggling with myself because I don't want to derail the current
>      progress,
>      however, I figured it is still worth pointing this out (again), the
>      earlier the better.
>
>      ---
>
>      I read over Flang patches regularly and I got more and more annoyed by
>      the coding style. I don't
>      know who designed it and why but it causes actual burden for people that
>      otherwise work with the LLVM style.
>
>      I am aware this was discussed before but I vaguely remember we wanted to
>      get closer to LLVM style
>      and I am worried that goal has been forgotten by now.
>
>      My initial suggestion is to change the online style guide, highlighted
>      with *ABC*.
>
>        * Use /clang-format/ from llvm *trunk* on all C++ source and header
>          files before every merge to master. All code layout should be
>          determined by means of clang-format.
>        * *Where LLVM’s C++ style guide
>          <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fllvm.org%2Fdocs%2FCodingStandards.html%23style-issues&data=04%7C01%7Csscalpone%40nvidia.com%7C07c2f559151e4a5efb2108d8f54ff435%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637529071045010597%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=8LdRWMwrS3gqSyGDqlGZM2ZvQbiD9Jpd6iCEDPave%2BY%3D&reserved=0> is clear
>          on usage, follow it.*
>        * *Otherwise,* where a clear precedent exists in the project, follow it.
>
>      Filenames should follow the LLVM naming style.
>
>      The "Overall design preferences" should be rewritten. The content is not
>      only add odds with existing
>      practices in LLVM (& MLIR, OpenACC, OpenMP, ...) but probably also one
>      of the reasons we have to discuss
>      Flang compile times regularly. I mean, I love really crafty C++
>      meta-programming solutions as much as the next
>      person but we should rethink the trade-off we are advocating for.
>
>      Once we redefine the rules we can adopt the LLVM style piece by piece or
>      in one big swoop. Either way, let's
>      actually follow through on those discussions we had Feb 2020.
>
>      ~ Johannes
>
>
>      --
>      ───────────────────
>      ∽ Johannes (he/his)
>
>      _______________________________________________
>      flang-dev mailing list
>      flang-dev at lists.llvm.org
>      https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.llvm.org%2Fcgi-bin%2Fmailman%2Flistinfo%2Fflang-dev&data=04%7C01%7Csscalpone%40nvidia.com%7C07c2f559151e4a5efb2108d8f54ff435%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637529071045010597%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=L2mtxInFuEdlgJkLr51%2FtKNtXUwIAKHNr1L9R2yOv%2FI%3D&reserved=0
>