Clang-format: RemoveBracesLLVM

Clang Frontend
1

@owenpan Thanks to LLVM weekly I’ve spotted RemoveBracesLLVM in clang-format. I know it says experimental, though it really looks promising/useful for our code as we have a similar style guide rule.

Your documentation states: This option will be renamed and expanded to support other styles. however it ain’t clear to me which options you have in mind.

Reading through the unit tests, I feel the only divination we have from this rule is the allowed level of nesting before {} are needed. Which in our case is at 1 while you have it at 2 levels deep. Let me make it practical, see examples below.

Is this an option you already had on your list? If not, can this be added?

LLVM:

		  verifyFormat("if (isa<FunctionDecl>(D)) {\n"
               "  for (auto *A : D.attrs())\n"
               "    for (ssize_t i : llvm::seq<ssize_t>(count))\n"
               "      handleAttrOnDecl(D, A, i);\n"
               "}",
               "if (isa<FunctionDecl>(D)) {\n"
               "  for (auto *A : D.attrs()) {\n"
               "    for (ssize_t i : llvm::seq<ssize_t>(count)) {\n"
               "      handleAttrOnDecl(D, A, i);\n"
               "    }\n"
               "  }\n"
               "}",
               Style);

Ours:

		  verifyFormat("if (isa<FunctionDecl>(D)) {\n"
               "  for (auto *A : D.attrs()) {\n"
               "    for (ssize_t i : llvm::seq<ssize_t>(count))\n"
               "      handleAttrOnDecl(D, A, i);\n"
               "  }\n"
               "}",
               "if (isa<FunctionDecl>(D)) {\n"
               "  for (auto *A : D.attrs()) {\n"
               "    for (ssize_t i : llvm::seq<ssize_t>(count)) {\n"
               "      handleAttrOnDecl(D, A, i);\n"
               "    }\n"
               "  }\n"
               "}",
               Style);

LLVM:

verifyFormat("if (a)\n"
               "  if (b)\n"
               "    c;\n"
               "  else\n"
               "    d;\n"
               "else\n"
               "  e;",
               "if (a) {\n"
               "  if (b) {\n"
               "    c;\n"
               "  } else {\n"
               "    d;\n"
               "  }\n"
               "} else {\n"
               "  e;\n"
               "}",
               Style);

Ours:

verifyFormat("if (a) {\n"
               "  if (b)\n"
               "    c;\n"
               "  else\n"
               "    d;\n"
               "  }\n"
               "else {\n"
               "  e;"
               "}\n",
               "if (a) {\n"
               "  if (b) {\n"
               "    c;\n"
               "  } else {\n"
               "    d;\n"
               "  }\n"
               "} else {\n"
               "  e;\n"
               "}",
               Style);

LLVM:

  verifyFormat("if (isa<FunctionDecl>(D))\n"
               "  for (auto *A : D.attrs())\n"
               "    handleAttr(A);",
               "if (isa<FunctionDecl>(D)) {\n"
               "  for (auto *A : D.attrs()) {\n"
               "    handleAttr(A);\n"
               "  }\n"
               "}",
               Style);

Ours:

  verifyFormat("if (isa<FunctionDecl>(D)) {\n"
               "  for (auto *A : D.attrs())\n"
               "    handleAttr(A);"
               "}\n",
               "if (isa<FunctionDecl>(D)) {\n"
               "  for (auto *A : D.attrs()) {\n"
               "    handleAttr(A);\n"
               "  }\n"
               "}",
               Style);
1 Like
2

We have issues in flight for the addition of braces via clang-format

https://reviews.llvm.org/D95168

We decided to break apart the “Insertion” and “Removal” aspects on the grounds that they are likely used in a mutually exclusive manner.

With addition as well as removal there could be cases where you do or don’t ALWAYS want to remove the braces.

For example (but not limited to)

if (x) {
   // keep the brace
   return true;
}

vs

if (x) 
   // don't keep the brace
   return true;
1 Like
3

Sounds reasonable. From what I could see in the unit tests, the behavior that’s currently is exactly how we currently do it manually, except for the indent level. So I’m fan to start using it if it’s available and has that 1 option to adapt to our case.

4

I’ve been working on InsertBraces (⚙ D120217 [clang-format] Add an option to insert braces after control statements). After it lands, I plan to clean up RemoveBracesLLVM and replace it with RemoveBraces to support additional styles. Although I can’t tell you exactly what suboptions RemoveBraces will have, the number of maximum nesting levels will definitely be there. :slight_smile:

Meanwhile, you can modify the line below if you don’t want to wait: