thieta created this revision.
thieta added reviewers: MyDeveloperDay, curdeius, owenpan.
Herald added a project: All.
thieta requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

We had some code that looked like this:

  int foo()
  {
    #pragma region hello(foo)
    #pragma endregion
  }
  
  foo* bar() {}

This was confusingly indented like:

  int foo()
    {
      #pragma region...
    }

After some investigation I noticed that this is because
`parseBracedList()` thought that this was a initializer list.

The check here: 
https://github.com/tru/llvm-project/blob/main/clang/lib/Format/UnwrappedLineParser.cpp#L709
will mark the code above as ProbablyBracedList and then it
will format it totally wrong depending on your style settings.

My initial fix was to change the check above, but it became
really complicated to keep both initializer lists and my code
working.

My approach here instead is to discard any line that starts with #
since that is a pre-processor statement we shouldn't really care
about it in this case.

This fix passes all the unittests and our internal code-base, so
I am fairly confident in it, but I am no clang-format expert.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136337

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19970,6 +19970,14 @@
             format("#pragma region TEST(FOO : BAR)", Style));
 }
 
+TEST_F(FormatTest, PragmaRegionShouldNotBeBraceInitializers) {
+  auto Style = getLLVMStyleWithColumns(0);
+  verifyFormat("CxxClass::CxxClass() {\n#pragma region test(hello)\n}", Style);
+  EXPECT_EQ("CxxClass::CxxClass()\n#pragma region TEST(bar: foo)\n{\n#pragma 
region TEST(foo: bar)\n#pragma endregion\n}\nCxxClass *Hello() {}",
+            format("CxxClass::CxxClass()\n#pragma region TEST(bar: foo)\n{\n  
#pragma region TEST(foo: bar)\n  #pragma endregion\n}\nCxxClass* Hello()\n{}", 
Style));
+
+}
+
 TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
   FormatStyle Style = getLLVMStyleWithColumns(20);
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -751,6 +751,11 @@
       if (!LBraceStack.empty() && LBraceStack.back()->is(BK_Unknown))
         LBraceStack.back()->setBlockKind(BK_Block);
       break;
+    case tok::hash:
+      do {
+        NextTok = Tokens->getNextToken();
+      } while (NextTok->isNot(tok::eof));
+      break;
     default:
       break;
     }


Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -19970,6 +19970,14 @@
             format("#pragma region TEST(FOO : BAR)", Style));
 }
 
+TEST_F(FormatTest, PragmaRegionShouldNotBeBraceInitializers) {
+  auto Style = getLLVMStyleWithColumns(0);
+  verifyFormat("CxxClass::CxxClass() {\n#pragma region test(hello)\n}", Style);
+  EXPECT_EQ("CxxClass::CxxClass()\n#pragma region TEST(bar: foo)\n{\n#pragma region TEST(foo: bar)\n#pragma endregion\n}\nCxxClass *Hello() {}",
+            format("CxxClass::CxxClass()\n#pragma region TEST(bar: foo)\n{\n  #pragma region TEST(foo: bar)\n  #pragma endregion\n}\nCxxClass* Hello()\n{}", Style));
+
+}
+
 TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
   FormatStyle Style = getLLVMStyleWithColumns(20);
 
Index: clang/lib/Format/UnwrappedLineParser.cpp
===================================================================
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -751,6 +751,11 @@
       if (!LBraceStack.empty() && LBraceStack.back()->is(BK_Unknown))
         LBraceStack.back()->setBlockKind(BK_Block);
       break;
+    case tok::hash:
+      do {
+        NextTok = Tokens->getNextToken();
+      } while (NextTok->isNot(tok::eof));
+      break;
     default:
       break;
     }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to