[PATCH] D95168: [clang-format] Add InsertBraces option

Owen Pan via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 3 01:33:31 PDT 2021


owenpan added a comment.

In D95168#3102511 <https://reviews.llvm.org/D95168#3102511>, @MyDeveloperDay wrote:

> Then this is definitely why we want to think about these now and NOT leave them to a separate review after the Insert case is committed.

We can just leave a placeholder for the RemoveBraces options and deal with them when we get there. Unless I misunderstood, your suggestion of the AutomaticBraces option (with `enum` values Insert, Remove, LLVM, etc) would accomplish that.

> If we think remove braces would be a struct then we'd better go for a struct now and not later so there is some symmetry, I'm slightly wondering what would be in the struct?

I already had an implementation of RemoveBraces for the LLVM style (minus some exceptions). I started with an `enum` to accommodate different levels of aggressiveness of removal with LLVM at the minimum end. After looking at several prominent styles, I realized that a `struct` would work better.

> 1. Do we want to remove {} from an else or elseif when the if needs braces
>
>   if (x==1) {
>      foo();
>      bar();
>   }
>   else  if (x==2)
>     baz();
>   else 
>     foo();
>
> 2. Do we want to remove braces from a commented if  (this can throw your eye out when looking)
>
>   if (x==1)
>      // some longish comment
>      return x;
>
> 3. Do we want sub scope to be removed of braces too?
>
>   if (x==1)
>       for (int i=0;i<10;i++)
>                 foo();
>
> 4. Do we want to prevent the remove for MultiLine scope, where what follows would be spread over multiple lines (a little like the comment really)
>
>       if (x== 1)
>           os << "This is a very very very long" 
>                << data
>                << "More data"
>                << "endl;"
>   `

I've seen both yes and no for all the above.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95168/new/

https://reviews.llvm.org/D95168



More information about the cfe-commits mailing list