[clang] clang-format Fix InsertBraces inserting braces around directives (PR #224947)

[via cfe-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-format

Author: Andre Sun (and2049)

<details>
<summary>Changes</summary>

Skip brace insertion when the pair would enclose preprocessor directives, avoiding unmatched braces across conditional branches.
This also leaves bodies containing balanced directives unbraced, even where insertion previously produced valid code, consistent with the documented behavior.
Fixes #<!-- -->224942.


---
Full diff: https://github.com/llvm/llvm-project/pull/224947.diff


2 Files Affected:

- (modified) clang/lib/Format/UnwrappedLineParser.cpp (+19-2) 
- (modified) clang/unittests/Format/BracesInserterTest.cpp (+45) 


``````````diff
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index 2d2fe729fbc2f..86445cc02cfa6 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -2910,6 +2910,7 @@ void UnwrappedLineParser::parseUnbracedBody(bool CheckEOF) {
   }
 
   addUnwrappedLine();
+  const size_t BodyStart = CurrentLines->size();
   ++Line->Level;
   ++Line->UnbracedBodyLevel;
   parseStructuralElement();
@@ -2917,15 +2918,31 @@ void UnwrappedLineParser::parseUnbracedBody(bool CheckEOF) {
 
   if (Tok) {
     assert(!Line->InPPDirective);
+    FormatToken *const LBraceTok = Tok;
     Tok = nullptr;
-    for (const auto &L : llvm::reverse(*CurrentLines)) {
+    size_t BodyEnd = CurrentLines->size();
+    while (BodyEnd > 0) {
+      const auto &L = (*CurrentLines)[--BodyEnd];
       if (!L.InPPDirective && getLastNonComment(L)) {
         Tok = L.Tokens.back().Tok;
         break;
       }
     }
     assert(Tok);
-    ++Tok->BraceCount;
+    // Don't insert the braces if they would enclose preprocessor directives.
+    bool EnclosesPPDirective = false;
+    for (size_t I = BodyStart; I <= BodyEnd && !EnclosesPPDirective; ++I) {
+      const auto &L = (*CurrentLines)[I];
+      EnclosesPPDirective =
+          L.InPPDirective ||
+          llvm::any_of(L.Tokens, [](const UnwrappedLineNode &Node) {
+            return Node.Tok->FirstAfterPPLine;
+          });
+    }
+    if (EnclosesPPDirective)
+      LBraceTok->BraceCount = 0;
+    else
+      ++Tok->BraceCount;
   }
 
   if (CheckEOF && eof())
diff --git a/clang/unittests/Format/BracesInserterTest.cpp b/clang/unittests/Format/BracesInserterTest.cpp
index 572e53e595e37..11c7c17b9aeb4 100644
--- a/clang/unittests/Format/BracesInserterTest.cpp
+++ b/clang/unittests/Format/BracesInserterTest.cpp
@@ -195,6 +195,51 @@ TEST_F(BracesInserterTest, InsertBraces) {
                "#endif",
                Style);
 
+  verifyFormat("if (a)\n"
+               "  if (b)\n"
+               "#ifdef A\n"
+               "    f();\n"
+               "#else\n"
+               "    g();\n"
+               "#endif",
+               Style);
+
+  verifyFormat("while (a)\n"
+               "  if (b)\n"
+               "    f(\n"
+               "#ifdef A\n"
+               "        1);\n"
+               "#else\n"
+               "        2);\n"
+               "#endif",
+               Style);
+
+  verifyFormat("if (a)\n"
+               "  f(\n"
+               "#ifdef A\n"
+               "      1\n"
+               "#else\n"
+               "      2\n"
+               "#endif\n"
+               "  );",
+               Style);
+
+  verifyFormat("#if A\n"
+               "if (a) {\n"
+               "  f();\n"
+               "}\n"
+               "#else\n"
+               "if (b) {\n"
+               "  g();\n"
+               "}\n"
+               "#endif",
+               "#if A\n"
+               "if (a) f();\n"
+               "#else\n"
+               "if (b) g();\n"
+               "#endif",
+               Style);
+
   verifyFormat("#if 0\n"
                "#elif 1\n"
                "#endif\n"

``````````

</details>


https://github.com/llvm/llvm-project/pull/224947