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

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

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

> @tiagoma are you still interested in pursuing this? I have some suggestions
>
> 1. I'd like to move the BraceInserter Into its own .cpp and .h files (like I did with the QualifierAlignmentFixer)
> 2. I'd like to move the unit tests into their own .cpp file  (because I think we need to actually unit tests their functions of BraceInserter more than just testing if via verfiyFormat and I think its cleaner as FormatTest.cpp is very large)
> 3. I'd like to see what it would take to remove braces, (eliding the braces on small ifs and control statements is about the number one review comments in LLVM)

Eliding braces would be much more complicated and should be tackled separately. Below are just some examples:

  void f() {
    // Braces can be removed:
    if (a) {
      b;
    } else if (c) {
      d;
    } else {
      e;
    }
  
    if (a) {
      b;
      c;
    } else if (d) { // Braces may be removed depending on the style.
  #undef NDEBUG
      e;
    } else if (g) {
      // comment
    } else {
      {
        h;
      }
    }
  
    if (a) {
      if (b) { // Braces can be removed.
        c;
      }
    } else if (d) { // Braces may be removed depending on the style.
      e;
    }
  
    if (a) { // Braces can be removed if the dangling-else warning can be ignored.
      if (b) { // Braces may be removed depending on the style.
        c;
        // comment
      } else if (d) { // Braces may be removed depending on the style.
        e;
      }
    }
  
    // Braces can be removed:
    if (a) {
      if (b) {
        c;
      }
    }
  
    // Braces may be removed depending on the style:
    if (a) {
      if (b) {
        c;
      } else {
        d;
      }
    } else {
      e;
    }
  
    if (a) { // Braces may be removed depending on the style.
      // Braces can be removed:
      if (b) {
        c;
      } else if (d) {
        e;
      }
    } else { // Braces may be removed depending on the style.
      g;
    }
  
    // Braces can be removed:
    if (a) {
      b;
    } else {
      if (c) {
        d;
      } else {
        e;
      }
    }

I have an experimental implementation that reformats the above to the following (avoiding the dangling-else warning, matching braces of `if` and `else`, etc.):

  void f() {
    // Braces can be removed:
    if (a)
      b;
    else if (c)
      d;
    else
      e;
  
    if (a) {
      b;
      c;
    } else if (d) { // Braces may be removed depending on the style.
  #undef NDEBUG
      e;
    } else if (g) {
      // comment
    } else {
      { h; }
    }
  
    if (a) {
      if (b) // Braces can be removed.
        c;
    } else if (d) { // Braces may be removed depending on the style.
      e;
    }
  
    if (a) { // Braces can be removed if the dangling-else warning can be ignored.
      if (b) { // Braces may be removed depending on the style.
        c;
        // comment
      } else if (d) { // Braces may be removed depending on the style.
        e;
      }
    }
  
    // Braces can be removed:
    if (a)
      if (b)
        c;
  
    // Braces may be removed depending on the style:
    if (a)
      if (b)
        c;
      else
        d;
    else
      e;
  
    if (a) { // Braces may be removed depending on the style.
      // Braces can be removed:
      if (b)
        c;
      else if (d)
        e;
    } else { // Braces may be removed depending on the style.
      g;
    }
  
    // Braces can be removed:
    if (a)
      b;
    else if (c)
      d;
    else
      e;


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95168