[clang-format] extern with new line brace without indentation

[MyDeveloperDay]

Summary

https://github.com/llvm/llvm-project/issues/49804

Interaction between IndentExternBlock and AfterExternBlock means you cannot have AfterExternBlock = true and IndentExternBlock = NoIndent/Indent

This patch resolves that

BraceWrapping:
  AfterExternBlock: true
IndentExternBlock: AfterExternBlock

Fixes: #49804

[curdeius]

clang/unittests/Format/FormatTest.cpp

3824

This line is unnecessary and misleading (copied from above?), as IndentExternBlock is overridden at line 3818. Please delete.

3828

I'd prefer that you split strings after \n. I find it easier to read that way.
The same below.

3834

Ditto. Remove.

[owenpan]

clang/unittests/Format/FormatTest.cpp

3828

It's everywhere in this function. We should fix them either now or in a separate patch.

[owenpan]

clang/lib/Format/UnwrappedLineParser.cpp

1282–1301

This case is kind of messy and some cleanup may be in order. A related question: why do we make IEBS_AfterExternBlock depend on BraceWrapping.AfterExternBlock? Can't we only have true/false for IndentExternBlock and let the user set it independent of BraceWrapping.AfterExternBlock?

[owenpan]

clang/unittests/Format/FormatTest.cpp

3825

This line is unnecessary too.

3835–3836

I would remove these too.

[curdeius]

clang/unittests/Format/FormatTest.cpp

3828

Ok, let's do it in another patch.

[MyDeveloperDay]

As always you are both correct

• tidy up the unit tests to follow our pattern
• reduce unnecessary style setting
• try and tidy up the indent selection a little

[MyDeveloperDay]

clang/lib/Format/UnwrappedLineParser.cpp

1282–1301

This a a backwards compatibility thing, (its needed for the google style by the looks of things)

[curdeius]

LGTM!

clang/lib/Format/UnwrappedLineParser.cpp

1285–1287

Nit.

[owenpan]

clang/lib/Format/UnwrappedLineParser.cpp

1291–1293

Nit: the outer parens are unnecessary?

clang/unittests/Format/FormatTest.cpp

3804

I would remove either line 3801 or 3804.

3825

Nit: you can remove this line.

[MyDeveloperDay]

Address nits and review comments (remove unnecessary lines)

[owenpan]

LGTM!

clang/lib/Format/UnwrappedLineParser.cpp

void UnwrappedLineParser::parseStructuralElement(bool IsTopLevel) {
   case tok::kw___try:
     if (Style.Language == FormatStyle::LK_JavaScript && Line->MustBeDeclaration)
       // field/method declaration.
       break;
     parseTryCatch();
     return;
   case tok::kw_extern:
     nextToken();
     if (FormatTok->Tok.is(tok::string_literal)) {
       nextToken();
       if (FormatTok->Tok.is(tok::l_brace)) {
-        if (!Style.IndentExternBlock) {
-          if (Style.BraceWrapping.AfterExternBlock) {
-            addUnwrappedLine();
-          }
-          unsigned AddLevels = Style.BraceWrapping.AfterExternBlock ? 1u : 0u;
-          parseBlock(/*MustBeDeclaration=*/true, AddLevels);
-        } else {
-          unsigned AddLevels =
-              Style.IndentExternBlock == FormatStyle::IEBS_Indent ? 1u : 0u;
-          parseBlock(/*MustBeDeclaration=*/true, AddLevels);
-        }
+        // Either we indent or for backwards compatibility we follow the
+        // AfterExternBlock style.
+            ((Style.IndentExternBlock == FormatStyle::IEBS_Indent) ||
+             (Style.BraceWrapping.AfterExternBlock &&
+              Style.IndentExternBlock == FormatStyle::IEBS_AfterExternBlock))
+                ? 1u
+                : 0u;
         addUnwrappedLine();
         return;
       }
     }
     break;
   case tok::kw_export:
     if (Style.Language == FormatStyle::LK_JavaScript) {
       parseJavaScriptEs6ImportExport();
       return;
     }
     if (!Style.isCpp())
       break;
     // Handle C++ "(inline|export) namespace".

clang/unittests/Format/FormatTest.cpp

verifyFormat("extern \"C\"\n"
                Style);
 }
 
 TEST_F(FormatTest, IndentExternBlockStyle) {
   FormatStyle Style = getLLVMStyle();
   Style.IndentWidth = 2;
 
   Style.IndentExternBlock = FormatStyle::IEBS_Indent;
-  verifyFormat("extern \"C\" { /*9*/\n}", Style);
+  verifyFormat("extern \"C\" { /*9*/\n"
+               "}",
+               Style);
   verifyFormat("extern \"C\" {\n"
                "  int foo10();\n"
                "}",
                Style);
 
   Style.IndentExternBlock = FormatStyle::IEBS_NoIndent;
-  verifyFormat("extern \"C\" { /*11*/\n}", Style);
+  verifyFormat("extern \"C\" { /*11*/\n"
+               "}",
+               Style);
   verifyFormat("extern \"C\" {\n"
                "int foo12();\n"
                "}",
                Style);
 
   Style.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterExternBlock = true;
-  verifyFormat("extern \"C\"\n{ /*13*/\n}", Style);
+  Style.IndentExternBlock = FormatStyle::IEBS_Indent;
+  verifyFormat("extern \"C\"\n"
+               "{ /*13*/\n"
+               "}",
+               Style);
   verifyFormat("extern \"C\"\n{\n"
                "  int foo14();\n"
                "}",
                Style);
 
-  Style.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock;
-  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterExternBlock = false;
-  verifyFormat("extern \"C\" { /*15*/\n}", Style);
+  Style.IndentExternBlock = FormatStyle::IEBS_NoIndent;
+  verifyFormat("extern \"C\" { /*15*/\n"
+               "}",
+               Style);
   verifyFormat("extern \"C\" {\n"
                "int foo16();\n"
                "}",
                Style);
+
+  Style.BraceWrapping.AfterExternBlock = true;
+  Style.IndentExternBlock = FormatStyle::IEBS_NoIndent;
+  verifyFormat("extern \"C\"\n"
+               "{ /*13*/\n"
+               "}",
+               Style);
+  verifyFormat("extern \"C\"\n"
+               "{\n"
+               "int foo14();\n"
+               "}",
+               Style);
+
+  Style.IndentExternBlock = FormatStyle::IEBS_Indent;
+  verifyFormat("extern \"C\"\n"
+               "{ /*13*/\n"
+               "}",
+               Style);
+  verifyFormat("extern \"C\"\n"
+               "{\n"
+               "  int foo14();\n"
+               "}",
+               Style);
 }
 
 TEST_F(FormatTest, FormatsInlineASM) {
   verifyFormat("asm(\"xyz\" : \"=a\"(a), \"=d\"(b) : \"a\"(data));");
   verifyFormat("asm(\"nop\" ::: \"memory\");");
   verifyFormat(
       "asm(\"movq\\t%%rbx, %%rsi\\n\\t\"\n"
       "    \"cpuid\\n\\t\"\n"