[clang-format] Don't break lines after pragma region

[thieta]

Summary

We have autogenerated pragma regions in our code
which where awkwardly broken up like this:

#pragma region foo(bar : hello)

becomes

#pragma region foo(bar \
                   : hello)

This fixes the problem by adding region as a keyword
and handling it the same way as pragma mark

[wanders]

There are other pragmas which include colon. How do they fare?

Example https://docs.microsoft.com/en-us/cpp/preprocessor/warning?view=msvc-170

#pragma warning( disable : 4507 34; once : 4385; error : 164 )

[thieta]

Added additional tests to check other pragma statements

[thieta]

In D125961#3526347, @wanders wrote:

There are other pragmas which include colon. How do they fare?

Example https://docs.microsoft.com/en-us/cpp/preprocessor/warning?view=msvc-170

#pragma warning( disable : 4507 34; once : 4385; error : 164 )

I checked this and added an additional test to test for this. It works fine without any changes - I think the difference is when there is a additional token between pragma and the "text".

[curdeius]

Thanks for the patch!
Could you please add a unit test in unittest/Format/FormatTest.cpp instead of in lit-based test/?
Is there a bug report that your patch fixes?

clang/lib/Format/TokenAnnotator.cpp

1228

Please fix (remove) this comment that consumes not only mark.

[thieta]

In D125961#3527447, @curdeius wrote:

Thanks for the patch!
Could you please add a unit test in unittest/Format/FormatTest.cpp instead of in lit-based test/?
Is there a bug report that your patch fixes?

Sure - I didn't know clang-format used unittests instead. Maybe there should be some documentation around this for new people used to adding lit tests?

I never filed a bug for it - I fixed it instead.

Thanks

[curdeius]

In D125961#3527452, @thieta wrote:

In D125961#3527447, @curdeius wrote:

Thanks for the patch!
Could you please add a unit test in unittest/Format/FormatTest.cpp instead of in lit-based test/?
Is there a bug report that your patch fixes?

Sure - I didn't know clang-format used unittests instead.

Yes, non-unit tests serve meeting for the config tests IIRC.

Maybe there should be some documentation around this for new people used to adding lit tests?

That would certainly help. Feel free to do it if you
Otherwise I'll do it but only later (in ten days or so).

[thieta]

Switched to unit test over lit test

[thieta]

Removed comment

[curdeius]

Thanks! LGTM.

[MyDeveloperDay]

clang/unittests/Format/FormatTest.cpp

18832

do we need to consider endregion at all? it would be nice to have some tests that included both.

clang/lib/Format/FormatToken.h

AdditionalKeywords(IdentifierTable &IdentTable) {
     kw_package = &IdentTable.get("package");
     kw_synchronized = &IdentTable.get("synchronized");
     kw_throws = &IdentTable.get("throws");
     kw___except = &IdentTable.get("__except");
     kw___has_include = &IdentTable.get("__has_include");
     kw___has_include_next = &IdentTable.get("__has_include_next");
 
     kw_mark = &IdentTable.get("mark");
+    kw_region = &IdentTable.get("region");
 
     kw_extend = &IdentTable.get("extend");
     kw_option = &IdentTable.get("option");
     kw_optional = &IdentTable.get("optional");
     kw_repeated = &IdentTable.get("repeated");
     kw_required = &IdentTable.get("required");
     kw_returns = &IdentTable.get("returns");
 
struct AdditionalKeywords {
   IdentifierInfo *kw_interface;
   IdentifierInfo *kw_native;
   IdentifierInfo *kw_package;
   IdentifierInfo *kw_synchronized;
   IdentifierInfo *kw_throws;
 
   // Pragma keywords.
   IdentifierInfo *kw_mark;
+  IdentifierInfo *kw_region;
 
   // Proto keywords.
   IdentifierInfo *kw_extend;
   IdentifierInfo *kw_option;
   IdentifierInfo *kw_optional;
   IdentifierInfo *kw_repeated;
   IdentifierInfo *kw_required;
   IdentifierInfo *kw_returns;

clang/lib/Format/TokenAnnotator.cpp

while (CurrentToken) {
       CurrentToken->setType(TT_ImplicitStringLiteral);
       next();
     }
   }
 
   void parsePragma() {
     next(); // Consume "pragma".
     if (CurrentToken &&
-        CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option)) {
+        CurrentToken->isOneOf(Keywords.kw_mark, Keywords.kw_option, Keywords.kw_region)) {
       bool IsMark = CurrentToken->is(Keywords.kw_mark);
-      next(); // Consume "mark".
+      next();
       next(); // Consume first token (so we fix leading whitespace).
       while (CurrentToken) {
         if (IsMark || CurrentToken->Previous->is(TT_BinaryOperator))
           CurrentToken->setType(TT_ImplicitStringLiteral);
         next();
       }
     }
   }

clang/unittests/Format/FormatTest.cpp

 }
 
 TEST_F(FormatTest, UnderstandPragmaOption) {
   verifyFormat("#pragma option -C -A");
 
   EXPECT_EQ("#pragma option -C -A", format("#pragma    option   -C   -A"));
 }
 
+TEST_F(FormatTest, UnderstandPragmaRegion) {
+  auto Style = getLLVMStyleWithColumns(0);
+  verifyFormat("#pragma region TEST(FOO : BAR)", Style);
+
+  EXPECT_EQ("#pragma region TEST(FOO : BAR)", format("#pragma region TEST(FOO : BAR)", Style));
+}
+
 TEST_F(FormatTest, OptimizeBreakPenaltyVsExcess) {
   FormatStyle Style = getLLVMStyleWithColumns(20);
 
   // See PR41213
   EXPECT_EQ("/*\n"
             " *\t9012345\n"
             " * /8901\n"
             " */",