[PATCH] D68296: clang-format: Add ability to wrap braces after multi-line control statements

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar created this revision.
mitchell-stellar added reviewers: sammccall, owenpan, reuk.
mitchell-stellar added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Change the BraceWrappingFlags' AfterControlStatement from a bool to an enum 
with three values:

- "Never": This is the default, and does not do any brace wrapping after 
control statements.
- "OnlyMultiLine": This only wraps braces after multi-line control statements 
(this really only happens when a ColumnLimit is specified).
- "Always": This always wraps braces after control statements.

The first and last options are backwards-compatible with "false" and "true", 
respectively.

The new "OnlyMultiLine" option is useful for when a wrapped control statement's 
indentation matches the subsequent block's indentation. It makes it easier to 
see at a glance where the control statement ends and where the block's code 
begins. For example:

  if (
foo
&& bar )
  {
baz();
  }

vs.

  if (
foo
&& bar ) {
baz();
  }


Repository:
  rC Clang

https://reviews.llvm.org/D68296

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestObjC.cpp

Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -207,7 +207,7 @@
"  f();\n"
"}\n");
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
-  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
   verifyFormat("@autoreleasepool\n"
"{\n"
"  f();\n"
@@ -237,7 +237,7 @@
"  f();\n"
"}\n");
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
-  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
   verifyFormat("@synchronized(self)\n"
"{\n"
"  f();\n"
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -644,7 +644,8 @@
   AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine =
   FormatStyle::SIS_WithoutElse;
   AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;
-  AllowSimpleBracedStatements.BraceWrapping.AfterControlStatement = true;
+  AllowSimpleBracedStatements.BraceWrapping.AfterControlStatement =
+  FormatStyle::BWACS_Always;
 
   verifyFormat("if (true) {}", AllowSimpleBracedStatements);
   verifyFormat("if constexpr (true) {}", AllowSimpleBracedStatements);
@@ -1168,7 +1169,7 @@
   Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Never;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterCaseLabel = true;
-  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
   EXPECT_EQ("switch (n)\n"
 "{\n"
 "  case 0:\n"
@@ -1370,7 +1371,7 @@
   Style.AllowShortCaseLabelsOnASingleLine = true;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterCaseLabel = true;
-  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
   EXPECT_EQ("switch (n)\n"
 "{\n"
 "  case 0:\n"
@@ -1441,6 +1442,126 @@
"}");
 }
 
+TEST_F(FormatTest, MultiLineControlStatements) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BraceBreakingStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_OnlyMultiLine;
+  Style.ColumnLimit = 20;
+  // Short lines should keep opening brace on same line.
+  EXPECT_EQ("if (foo) {\n"
+"  bar();\n"
+"}",
+format("if(foo){bar();}", Style));
+  EXPECT_EQ("if (foo) {\n"
+"  bar();\n"
+"} else {\n"
+"  baz();\n"
+"}",
+format("if(foo){bar();}else{baz();}", Style));
+  EXPECT_EQ("if (foo && bar) {\n"
+"  baz();\n"
+"}",
+format("if(foo&&bar){baz();}", Style));
+  EXPECT_EQ("if (foo) {\n"
+"  bar();\n"
+"} else if (baz) {\n"
+"  quux();\n"
+"}",
+format("if(foo){bar();}else if(baz){quux();}", Style));
+  EXPECT_EQ(
+  "if (foo) {\n"
+  "  bar();\n"
+  "} else if (baz) {\n"
+  "  quux();\n"
+  "} else {\n"
+  "  foobar();\n"
+  "}",
+  format("if(foo){bar();}else if(baz){quux();}else{foobar();}", Style));
+  EXPECT_EQ("for (

[PATCH] D68296: clang-format: Add ability to wrap braces after multi-line control statements

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar updated this revision to Diff 222835.
mitchell-stellar edited the summary of this revision.
mitchell-stellar added a comment.

Thanks for the review. I have added documentation updates.

I do not have a public style guide to reference. My company just switched to 
auto-clang-formatting all of our code, and this patch addresses one of our 
biggest pain points: readability. When the opening brace of a multi-line 
control statement is not wrapped, we simply cannot tell at first glance where 
the control statement ends and where the body begins due to our indentation 
settings.

I think the fact that you yourself see the benefit of this change speaks to its 
viability.

I cannot think of any other brace wrapping rules that need this level of 
control. Our code base consists of millions of lines of code, so I'm pretty 
sure we would have found another instance.

I would be willing to support this patch and further revisions if this gets 
into master, as our company will depend on this feature.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68296/new/

https://reviews.llvm.org/D68296

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestObjC.cpp

Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -207,7 +207,7 @@
"  f();\n"
"}\n");
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
-  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
   verifyFormat("@autoreleasepool\n"
"{\n"
"  f();\n"
@@ -237,7 +237,7 @@
"  f();\n"
"}\n");
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
-  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
   verifyFormat("@synchronized(self)\n"
"{\n"
"  f();\n"
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -644,7 +644,8 @@
   AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine =
   FormatStyle::SIS_WithoutElse;
   AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;
-  AllowSimpleBracedStatements.BraceWrapping.AfterControlStatement = true;
+  AllowSimpleBracedStatements.BraceWrapping.AfterControlStatement =
+  FormatStyle::BWACS_Always;
 
   verifyFormat("if (true) {}", AllowSimpleBracedStatements);
   verifyFormat("if constexpr (true) {}", AllowSimpleBracedStatements);
@@ -1168,7 +1169,7 @@
   Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Never;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterCaseLabel = true;
-  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
   EXPECT_EQ("switch (n)\n"
 "{\n"
 "  case 0:\n"
@@ -1370,7 +1371,7 @@
   Style.AllowShortCaseLabelsOnASingleLine = true;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterCaseLabel = true;
-  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
   EXPECT_EQ("switch (n)\n"
 "{\n"
 "  case 0:\n"
@@ -1441,6 +1442,126 @@
"}");
 }
 
+TEST_F(FormatTest, MultiLineControlStatements) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BraceBreakingStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_OnlyMultiLine;
+  Style.ColumnLimit = 20;
+  // Short lines should keep opening brace on same line.
+  EXPECT_EQ("if (foo) {\n"
+"  bar();\n"
+"}",
+format("if(foo){bar();}", Style));
+  EXPECT_EQ("if (foo) {\n"
+"  bar();\n"
+"} else {\n"
+"  baz();\n"
+"}",
+format("if(foo){bar();}else{baz();}", Style));
+  EXPECT_EQ("if (foo && bar) {\n"
+"  baz();\n"
+"}",
+format("if(foo&&bar){baz();}", Style));
+  EXPECT_EQ("if (foo) {\n"
+"  bar();\n"
+"} else if (baz) {\n"
+"  quux();\n"
+"}",
+format("if(foo){bar();}else if(baz){quux();}", Style));
+  EXPECT_EQ(
+  "if (foo) {\n"
+  "  bar();\n"
+  "} else if (baz) {\n"
+  "  quux();\n"
+  "} else {\n"
+  "  foobar();\n"
+  "}",
+  format("if(foo){bar();}els

[PATCH] D68296: [clang-format] Add ability to wrap braces after multi-line control statements

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

@MyDeveloperDay I am not wedded to "OnlyMultiLine". I picked it, as it seemed 
reasonable to me, but if you think another string expresses the intent better, 
then feel free to use it.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68296/new/

https://reviews.llvm.org/D68296



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68296: [clang-format] Add ability to wrap braces after multi-line control statements

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar updated this revision to Diff 222907.
mitchell-stellar added a comment.

Renamed "OnlyMultiLine" option to "MultiLine", per reviewer request.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68296/new/

https://reviews.llvm.org/D68296

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1445,7 +1445,7 @@
 TEST_F(FormatTest, MultiLineControlStatements) {
   FormatStyle Style = getLLVMStyle();
   Style.BreakBeforeBraces = FormatStyle::BraceBreakingStyle::BS_Custom;
-  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_OnlyMultiLine;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_MultiLine;
   Style.ColumnLimit = 20;
   // Short lines should keep opening brace on same line.
   EXPECT_EQ("if (foo) {\n"
@@ -12571,9 +12571,9 @@
 
   Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Never;
   CHECK_PARSE("BraceWrapping:\n"
-  "  AfterControlStatement: OnlyMultiLine",
+  "  AfterControlStatement: MultiLine",
   BraceWrapping.AfterControlStatement,
-  FormatStyle::BWACS_OnlyMultiLine);
+  FormatStyle::BWACS_MultiLine);
   CHECK_PARSE("BraceWrapping:\n"
   "  AfterControlStatement: Always",
   BraceWrapping.AfterControlStatement, FormatStyle::BWACS_Always);
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -311,7 +311,7 @@
  (TheLine->First->is(tok::r_brace) && TheLine->First->Next &&
   TheLine->First->Next->isOneOf(tok::kw_else, tok::kw_catch))) &&
 Style.BraceWrapping.AfterControlStatement ==
-FormatStyle::BWACS_OnlyMultiLine) {
+FormatStyle::BWACS_MultiLine) {
   // If possible, merge the next line's wrapped left brace with the current
   // line. Otherwise, leave it on the next line, as this is a multi-line
   // control statement.
@@ -624,7 +624,7 @@
 // }
 if (Line.First == Line.Last &&
 Style.BraceWrapping.AfterControlStatement ==
-FormatStyle::BWACS_OnlyMultiLine)
+FormatStyle::BWACS_MultiLine)
   return 0;
 
 return 2;
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -181,7 +181,7 @@
 IO.enumCase(Value, "false", FormatStyle::BWACS_Never);
 IO.enumCase(Value, "true", FormatStyle::BWACS_Always);
 IO.enumCase(Value, "Never", FormatStyle::BWACS_Never);
-IO.enumCase(Value, "OnlyMultiLine", FormatStyle::BWACS_OnlyMultiLine);
+IO.enumCase(Value, "MultiLine", FormatStyle::BWACS_MultiLine);
 IO.enumCase(Value, "Always", FormatStyle::BWACS_Always);
   }
 };
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -803,7 +803,7 @@
 ///   while (foo || bar) {
 ///   }
 /// \endcode
-BWACS_OnlyMultiLine,
+BWACS_MultiLine,
 /// Always wrap braces after a control statement.
 /// \code
 ///   if (foo())
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -794,7 +794,7 @@
 for (int i = 0; i < 10; ++i) {
 }
 
-* ``BWACS_OnlyMultiLine`` (in configuration: ``OnlyMultiLine``)
+* ``BWACS_MultiLine`` (in configuration: ``MultiLine``)
   Only wrap braces after a multi-line control statement.
 
   .. code-block:: c++


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1445,7 +1445,7 @@
 TEST_F(FormatTest, MultiLineControlStatements) {
   FormatStyle Style = getLLVMStyle();
   Style.BreakBeforeBraces = FormatStyle::BraceBreakingStyle::BS_Custom;
-  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_OnlyMultiLine;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_MultiLine;
   Style.ColumnLimit = 20;
   // Short lines should keep opening brace on same line.
   EXPECT_EQ("if (foo) {\n"
@@ -12571,9 +12571,9 @@
 
   Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Never;
   CHECK_PARSE("BraceWrapping:\n"
-  "  AfterControlStatemen

[PATCH] D68296: [clang-format] Add ability to wrap braces after multi-line control statements

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar marked 4 inline comments as done.
mitchell-stellar added inline comments.



Comment at: clang/lib/Format/Format.cpp:643
+true};
   switch (Style.BreakBeforeBraces) {
   case FormatStyle::BS_Linux:

MyDeveloperDay wrote:
> Nit: this a drive by comment, but I feel this is really hard to read.. and 
> not something that needs to be done in this revision but this feels like it 
> needs to be formatted differently
> 
> {
> /*AfterClass=*/false,
> /*AfterEnum=*/false
> /*AfterControlStatement=*/FormatStyle::BWACS_Never,
> .
> }
> 
> because without going back and checking that the AfterControl statement is 
> the 3rd bool I simply can't see what is on and what is off.
I'm going to leave this as-is, because before, I was still forced to look-up 
what each `true` or `false` value was assigned to.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68296/new/

https://reviews.llvm.org/D68296



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68296: [clang-format] Add ability to wrap braces after multi-line control statements

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar updated this revision to Diff 223058.
mitchell-stellar added a comment.

Rebased against master, and also added test case for 'do' statements.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68296/new/

https://reviews.llvm.org/D68296

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestObjC.cpp

Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -207,7 +207,7 @@
"  f();\n"
"}\n");
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
-  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
   verifyFormat("@autoreleasepool\n"
"{\n"
"  f();\n"
@@ -237,7 +237,7 @@
"  f();\n"
"}\n");
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
-  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
   verifyFormat("@synchronized(self)\n"
"{\n"
"  f();\n"
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -644,7 +644,8 @@
   AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine =
   FormatStyle::SIS_WithoutElse;
   AllowSimpleBracedStatements.AllowShortLoopsOnASingleLine = true;
-  AllowSimpleBracedStatements.BraceWrapping.AfterControlStatement = true;
+  AllowSimpleBracedStatements.BraceWrapping.AfterControlStatement =
+  FormatStyle::BWACS_Always;
 
   verifyFormat("if (true) {}", AllowSimpleBracedStatements);
   verifyFormat("if constexpr (true) {}", AllowSimpleBracedStatements);
@@ -1168,7 +1169,7 @@
   Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Never;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterCaseLabel = true;
-  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
   EXPECT_EQ("switch (n)\n"
 "{\n"
 "  case 0:\n"
@@ -1370,7 +1371,7 @@
   Style.AllowShortCaseLabelsOnASingleLine = true;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterCaseLabel = true;
-  Style.BraceWrapping.AfterControlStatement = true;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_Always;
   EXPECT_EQ("switch (n)\n"
 "{\n"
 "  case 0:\n"
@@ -1441,6 +1442,131 @@
"}");
 }
 
+TEST_F(FormatTest, MultiLineControlStatements) {
+  FormatStyle Style = getLLVMStyle();
+  Style.BreakBeforeBraces = FormatStyle::BraceBreakingStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = FormatStyle::BWACS_MultiLine;
+  Style.ColumnLimit = 20;
+  // Short lines should keep opening brace on same line.
+  EXPECT_EQ("if (foo) {\n"
+"  bar();\n"
+"}",
+format("if(foo){bar();}", Style));
+  EXPECT_EQ("if (foo) {\n"
+"  bar();\n"
+"} else {\n"
+"  baz();\n"
+"}",
+format("if(foo){bar();}else{baz();}", Style));
+  EXPECT_EQ("if (foo && bar) {\n"
+"  baz();\n"
+"}",
+format("if(foo&&bar){baz();}", Style));
+  EXPECT_EQ("if (foo) {\n"
+"  bar();\n"
+"} else if (baz) {\n"
+"  quux();\n"
+"}",
+format("if(foo){bar();}else if(baz){quux();}", Style));
+  EXPECT_EQ(
+  "if (foo) {\n"
+  "  bar();\n"
+  "} else if (baz) {\n"
+  "  quux();\n"
+  "} else {\n"
+  "  foobar();\n"
+  "}",
+  format("if(foo){bar();}else if(baz){quux();}else{foobar();}", Style));
+  EXPECT_EQ("for (;;) {\n"
+"  foo();\n"
+"}",
+format("for(;;){foo();}"));
+  EXPECT_EQ("while (1) {\n"
+"  foo();\n"
+"}",
+format("while(1){foo();}", Style));
+  EXPECT_EQ("switch (foo) {\n"
+"case bar:\n"
+"  return;\n"
+"}",
+format("switch(foo){case bar:return;}", Style));
+  EXPECT_EQ("try {\n"
+"  foo();\n"
+"} catch (...) {\n"
+"  bar();\n"
+"}",
+format("try{foo();}catch(...){bar();}", Style));
+  EXPECT_EQ("do {\n"
+"  foo();\n"
+"} while (bar &&\n"
+" baz);",
+format("do{foo();}while(bar&&baz);", Style));
+  // Long lines should put opening brace on new line.
+  EX

[PATCH] D68242: [clang-format] [PR42417] clang-format inserts a space after '->' for operator->() overloading

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

LGTM


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68242/new/

https://reviews.llvm.org/D68242



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67660: [clang-format] [PR43338] C# clang format has space issues betweern C# only keywords

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67660/new/

https://reviews.llvm.org/D67660



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67629: [clang-format] [PR43333] Fix C# breaking before function name when using Attributes

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67629/new/

https://reviews.llvm.org/D67629



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68415: [clang-format] C++11 braced lists should respect the SpacesInParentheses setting

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar created this revision.
mitchell-stellar added reviewers: MyDeveloperDay, reuk, owenpan.
mitchell-stellar added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

According to the clang-format documentation, "Fundamentally, C++11 braced lists 
are formatted exactly like function calls would be formatted in their place. If 
the braced list follows a name (e.g. a type or variable name), clang-format 
formats as if the `{}` were the parentheses of a function call with that name."

This patch furthers the treatment of C++11 braced list braces as parentheses by 
respecting the `SpacesInParentheses` setting.


Repository:
  rC Clang

https://reviews.llvm.org/D68415

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8210,6 +8210,31 @@
   SpaceBeforeBrace.SpaceBeforeCpp11BracedList = true;
   verifyFormat("vector x {1, 2, 3, 4};", SpaceBeforeBrace);
   verifyFormat("f({}, {{}, {}}, MyMap[{k, v}]);", SpaceBeforeBrace);
+
+  FormatStyle SpaceBetweenBraces = getLLVMStyle();
+  SpaceBetweenBraces.SpacesInAngles = true;
+  SpaceBetweenBraces.SpacesInParentheses = true;
+  SpaceBetweenBraces.SpacesInSquareBrackets = true;
+  verifyFormat("vector< int > x{ 1, 2, 3, 4 };", SpaceBetweenBraces);
+  verifyFormat("f( {}, { {}, {} }, MyMap[ { k, v } ] );", SpaceBetweenBraces);
+  verifyFormat("vector< int > x{ // comment 1\n"
+   " 1, 2, 3, 4 };",
+   SpaceBetweenBraces);
+  SpaceBetweenBraces.ColumnLimit = 20;
+  EXPECT_EQ("vector< int > x{\n"
+"1, 2, 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  SpaceBetweenBraces.ColumnLimit = 24;
+  EXPECT_EQ("vector< int > x{ 1, 2,\n"
+" 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  EXPECT_EQ("vector< int > x{\n"
+"1,\n"
+"2,\n"
+"3,\n"
+"4,\n"
+"};",
+format("vectorx{1,2,3,4,};", SpaceBetweenBraces));
 }
 
 TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2188,7 +2188,8 @@
 if (Current->is(TT_LineComment)) {
   if (Current->Previous->BlockKind == BK_BracedInit &&
   Current->Previous->opensScope())
-Current->SpacesRequiredBefore = Style.Cpp11BracedListStyle ? 0 : 1;
+Current->SpacesRequiredBefore =
+(Style.Cpp11BracedListStyle && !Style.SpacesInParentheses) ? 0 : 1;
   else
 Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
 
@@ -2628,7 +2629,7 @@
   if ((Left.is(tok::l_brace) && Left.BlockKind != BK_Block) ||
   (Right.is(tok::r_brace) && Right.MatchingParen &&
Right.MatchingParen->BlockKind != BK_Block))
-return !Style.Cpp11BracedListStyle;
+return Style.Cpp11BracedListStyle ? Style.SpacesInParentheses : true;
   if (Left.is(TT_BlockComment))
 // No whitespace in x(/*foo=*/1), except for JavaScript.
 return Style.Language == FormatStyle::LK_JavaScript ||


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8210,6 +8210,31 @@
   SpaceBeforeBrace.SpaceBeforeCpp11BracedList = true;
   verifyFormat("vector x {1, 2, 3, 4};", SpaceBeforeBrace);
   verifyFormat("f({}, {{}, {}}, MyMap[{k, v}]);", SpaceBeforeBrace);
+
+  FormatStyle SpaceBetweenBraces = getLLVMStyle();
+  SpaceBetweenBraces.SpacesInAngles = true;
+  SpaceBetweenBraces.SpacesInParentheses = true;
+  SpaceBetweenBraces.SpacesInSquareBrackets = true;
+  verifyFormat("vector< int > x{ 1, 2, 3, 4 };", SpaceBetweenBraces);
+  verifyFormat("f( {}, { {}, {} }, MyMap[ { k, v } ] );", SpaceBetweenBraces);
+  verifyFormat("vector< int > x{ // comment 1\n"
+   " 1, 2, 3, 4 };",
+   SpaceBetweenBraces);
+  SpaceBetweenBraces.ColumnLimit = 20;
+  EXPECT_EQ("vector< int > x{\n"
+"1, 2, 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  SpaceBetweenBraces.ColumnLimit = 24;
+  EXPECT_EQ("vector< int > x{ 1, 2,\n"
+" 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  EXPECT_EQ("vector< int > x{\n"
+"1,\n"
+"2,\n"
+"3,\n"
+"4,\n"
+"};",
+format("vectorx{1,2,3,4,};", SpaceBetweenBraces));
 }
 
 TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
Index: clang/li

[PATCH] D68332: [clang-format] [PR43531] clang-format damages "alternative representations" for operators

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar requested changes to this revision.
mitchell-stellar added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2892
+  if (Left.is(TT_UnaryOperator)) {
+// Don't combine the unary operators !~ into "compl5" and "not5"
+// when using alternative operators "compl" and "not"

Without context, "compl5" and "not5" do not make sense. Can you try to rephrase 
this comment?



Comment at: clang/lib/Format/TokenAnnotator.cpp:2896
+if (!Right.is(tok::l_paren)) {
+  if (Left.is(tok::exclaim) && Left.TokenText.equals("not"))
+return true;

For consistency, `Left.TokenText == "not"` may be better. I cannot find another 
instance of `.equals()`.



Comment at: clang/unittests/Format/FormatTest.cpp:14445
+TEST_F(FormatTest, AlternativeOperators) {
+  // Test case for bug https://bugs.llvm.org/show_bug.cgi?id=43531
+  verifyFormat("int a and b;");

Instead of using a URL link, perhaps describe in words what this test is 
supposed to do?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68332/new/

https://reviews.llvm.org/D68332



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68415: [clang-format] C++11 braced lists should respect the SpacesInParentheses setting

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

I don't think that's a good idea, considering the fact that braces can mean 
different things in different contexts, and it would cause trouble for existing 
clang-format settings.

If a hypothetical `SpacesInBraces` were `false` by default, then you could have 
clang-format output something like:

`[](int foo) {bar(foo);}`

Currently, it is `[](int foo) { bar(foo); }`.

If a hypothetical `SpacesInBraces` were `true` by default, then you could have 
clang-format output something like:

`vector x{ 1, 2, 3, 4 };`

Currently, it is `vector x{1, 2, 3, 4};`.

This patch is minimally invasive and adheres to the existing documentation.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68415/new/

https://reviews.llvm.org/D68415



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68415: [clang-format] C++11 braced lists should respect the SpacesInParentheses setting

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar updated this revision to Diff 223077.
mitchell-stellar added a comment.

Added additional unit tests to verify that the `SpaceInEmptyParentheses` 
setting is also correctly adhered to.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68415/new/

https://reviews.llvm.org/D68415

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8210,6 +8210,34 @@
   SpaceBeforeBrace.SpaceBeforeCpp11BracedList = true;
   verifyFormat("vector x {1, 2, 3, 4};", SpaceBeforeBrace);
   verifyFormat("f({}, {{}, {}}, MyMap[{k, v}]);", SpaceBeforeBrace);
+
+  FormatStyle SpaceBetweenBraces = getLLVMStyle();
+  SpaceBetweenBraces.SpacesInAngles = true;
+  SpaceBetweenBraces.SpacesInParentheses = true;
+  SpaceBetweenBraces.SpacesInSquareBrackets = true;
+  verifyFormat("vector< int > x{ 1, 2, 3, 4 };", SpaceBetweenBraces);
+  verifyFormat("f( {}, { {}, {} }, MyMap[ { k, v } ] );", SpaceBetweenBraces);
+  verifyFormat("vector< int > x{ // comment 1\n"
+   " 1, 2, 3, 4 };",
+   SpaceBetweenBraces);
+  SpaceBetweenBraces.ColumnLimit = 20;
+  EXPECT_EQ("vector< int > x{\n"
+"1, 2, 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  SpaceBetweenBraces.ColumnLimit = 24;
+  EXPECT_EQ("vector< int > x{ 1, 2,\n"
+" 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  EXPECT_EQ("vector< int > x{\n"
+"1,\n"
+"2,\n"
+"3,\n"
+"4,\n"
+"};",
+format("vectorx{1,2,3,4,};", SpaceBetweenBraces));
+  verifyFormat("vector< int > x{};", SpaceBetweenBraces);
+  SpaceBetweenBraces.SpaceInEmptyParentheses = true;
+  verifyFormat("vector< int > x{ };", SpaceBetweenBraces);
 }
 
 TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2188,7 +2188,8 @@
 if (Current->is(TT_LineComment)) {
   if (Current->Previous->BlockKind == BK_BracedInit &&
   Current->Previous->opensScope())
-Current->SpacesRequiredBefore = Style.Cpp11BracedListStyle ? 0 : 1;
+Current->SpacesRequiredBefore =
+(Style.Cpp11BracedListStyle && !Style.SpacesInParentheses) ? 0 : 1;
   else
 Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
 
@@ -2628,7 +2629,7 @@
   if ((Left.is(tok::l_brace) && Left.BlockKind != BK_Block) ||
   (Right.is(tok::r_brace) && Right.MatchingParen &&
Right.MatchingParen->BlockKind != BK_Block))
-return !Style.Cpp11BracedListStyle;
+return Style.Cpp11BracedListStyle ? Style.SpacesInParentheses : true;
   if (Left.is(TT_BlockComment))
 // No whitespace in x(/*foo=*/1), except for JavaScript.
 return Style.Language == FormatStyle::LK_JavaScript ||


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8210,6 +8210,34 @@
   SpaceBeforeBrace.SpaceBeforeCpp11BracedList = true;
   verifyFormat("vector x {1, 2, 3, 4};", SpaceBeforeBrace);
   verifyFormat("f({}, {{}, {}}, MyMap[{k, v}]);", SpaceBeforeBrace);
+
+  FormatStyle SpaceBetweenBraces = getLLVMStyle();
+  SpaceBetweenBraces.SpacesInAngles = true;
+  SpaceBetweenBraces.SpacesInParentheses = true;
+  SpaceBetweenBraces.SpacesInSquareBrackets = true;
+  verifyFormat("vector< int > x{ 1, 2, 3, 4 };", SpaceBetweenBraces);
+  verifyFormat("f( {}, { {}, {} }, MyMap[ { k, v } ] );", SpaceBetweenBraces);
+  verifyFormat("vector< int > x{ // comment 1\n"
+   " 1, 2, 3, 4 };",
+   SpaceBetweenBraces);
+  SpaceBetweenBraces.ColumnLimit = 20;
+  EXPECT_EQ("vector< int > x{\n"
+"1, 2, 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  SpaceBetweenBraces.ColumnLimit = 24;
+  EXPECT_EQ("vector< int > x{ 1, 2,\n"
+" 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  EXPECT_EQ("vector< int > x{\n"
+"1,\n"
+"2,\n"
+"3,\n"
+"4,\n"
+"};",
+format("vectorx{1,2,3,4,};", SpaceBetweenBraces));
+  verifyFormat("vector< int > x{};", SpaceBetweenBraces);
+  SpaceBetweenBraces.SpaceInEmptyParentheses = true;
+  verifyFormat("vector< int > x{ };", SpaceBetweenBraces);
 }
 
 TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
Index: clang/lib/Format/TokenAnnotator.cpp
==

[PATCH] D68346: [clang-format] Add new option to add spaces around conditions

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar requested changes to this revision.
mitchell-stellar added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:2290
 
+**SpacesAroundConditions** (``bool``)
+  If ``true``, spaces will be inserted around if/for/while (and similar) 
conditions.

`SpacesInConditionalStatement` is probably a better name, remaining consistent 
with existing `SpacesIn*` options, as well as indicating it only affects the 
entire conditional statement, and not individual conditions within the 
statement.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2511
+  return t->isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok::kw_while,
+tok::kw_switch, tok::kw_constexpr, TT_ForEachMacro);
+};

It seems that you are mixing statement tokens `if`, `while`, etc. with 
preprocessor and macro tokens. Going by the documentation, this is unexpected.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2919
+  if (Right.is(tok::coloncolon) &&
+  !Left.isOneOf(tok::l_brace, tok::comment, tok::l_paren))
 return (Left.is(TT_TemplateOpener) &&

MyDeveloperDay wrote:
> I'm not sure I understand this change so you added a `tok::l_paren` here? but 
> its not just for your style, so something else must have changed. Did you run 
> all FormatTests?
> 
> one of the tests you add need to test why you added this here
This is not covered by your tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68346/new/

https://reviews.llvm.org/D68346



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68415: [clang-format] C++11 braced lists should respect the SpacesInParentheses setting

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

If you want spaces in your C++11 initializers and nothing else, then you don't 
want the `Cpp11BracedListStyle` setting enabled. Turning it off gives you 
spaces inside without affecting anything else.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68415/new/

https://reviews.llvm.org/D68415



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68415: [clang-format] C++11 braced lists should respect the SpacesInParentheses setting

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

Thanks. Please commit on my behalf when you have a chance.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68415/new/

https://reviews.llvm.org/D68415



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68415: [clang-format] C++11 braced lists should respect the SpacesInParentheses setting

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

You are correct. I'll work on this.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68415/new/

https://reviews.llvm.org/D68415



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68415: [clang-format] C++11 braced lists should respect the SpacesInParentheses setting

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar updated this revision to Diff 223212.
mitchell-stellar added a comment.

Fixed empty parentheses unit test for C++11 braced initializer list.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68415/new/

https://reviews.llvm.org/D68415

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8210,6 +8210,34 @@
   SpaceBeforeBrace.SpaceBeforeCpp11BracedList = true;
   verifyFormat("vector x {1, 2, 3, 4};", SpaceBeforeBrace);
   verifyFormat("f({}, {{}, {}}, MyMap[{k, v}]);", SpaceBeforeBrace);
+
+  FormatStyle SpaceBetweenBraces = getLLVMStyle();
+  SpaceBetweenBraces.SpacesInAngles = true;
+  SpaceBetweenBraces.SpacesInParentheses = true;
+  SpaceBetweenBraces.SpacesInSquareBrackets = true;
+  verifyFormat("vector< int > x{ 1, 2, 3, 4 };", SpaceBetweenBraces);
+  verifyFormat("f( {}, { {}, {} }, MyMap[ { k, v } ] );", SpaceBetweenBraces);
+  verifyFormat("vector< int > x{ // comment 1\n"
+   " 1, 2, 3, 4 };",
+   SpaceBetweenBraces);
+  SpaceBetweenBraces.ColumnLimit = 20;
+  EXPECT_EQ("vector< int > x{\n"
+"1, 2, 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  SpaceBetweenBraces.ColumnLimit = 24;
+  EXPECT_EQ("vector< int > x{ 1, 2,\n"
+" 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  EXPECT_EQ("vector< int > x{\n"
+"1,\n"
+"2,\n"
+"3,\n"
+"4,\n"
+"};",
+format("vectorx{1,2,3,4,};", SpaceBetweenBraces));
+  verifyFormat("vector< int > x{};", SpaceBetweenBraces);
+  SpaceBetweenBraces.SpaceInEmptyParentheses = true;
+  verifyFormat("vector< int > x{ };", SpaceBetweenBraces);
 }
 
 TEST_F(FormatTest, FormatsBracedListsInColumnLayout) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2188,7 +2188,8 @@
 if (Current->is(TT_LineComment)) {
   if (Current->Previous->BlockKind == BK_BracedInit &&
   Current->Previous->opensScope())
-Current->SpacesRequiredBefore = Style.Cpp11BracedListStyle ? 0 : 1;
+Current->SpacesRequiredBefore =
+(Style.Cpp11BracedListStyle && !Style.SpacesInParentheses) ? 0 : 1;
   else
 Current->SpacesRequiredBefore = Style.SpacesBeforeTrailingComments;
 
@@ -2512,7 +2513,9 @@
 return Left.is(tok::hash);
   if (Left.isOneOf(tok::hashhash, tok::hash))
 return Right.is(tok::hash);
-  if (Left.is(tok::l_paren) && Right.is(tok::r_paren))
+  if ((Left.is(tok::l_paren) && Right.is(tok::r_paren)) ||
+  (Left.is(tok::l_brace) && Left.BlockKind != BK_Block &&
+   Right.is(tok::r_brace) && Right.BlockKind != BK_Block))
 return Style.SpaceInEmptyParentheses;
   if (Left.is(tok::l_paren) || Right.is(tok::r_paren))
 return (Right.is(TT_CastRParen) ||
@@ -2628,7 +2631,7 @@
   if ((Left.is(tok::l_brace) && Left.BlockKind != BK_Block) ||
   (Right.is(tok::r_brace) && Right.MatchingParen &&
Right.MatchingParen->BlockKind != BK_Block))
-return !Style.Cpp11BracedListStyle;
+return Style.Cpp11BracedListStyle ? Style.SpacesInParentheses : true;
   if (Left.is(TT_BlockComment))
 // No whitespace in x(/*foo=*/1), except for JavaScript.
 return Style.Language == FormatStyle::LK_JavaScript ||


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8210,6 +8210,34 @@
   SpaceBeforeBrace.SpaceBeforeCpp11BracedList = true;
   verifyFormat("vector x {1, 2, 3, 4};", SpaceBeforeBrace);
   verifyFormat("f({}, {{}, {}}, MyMap[{k, v}]);", SpaceBeforeBrace);
+
+  FormatStyle SpaceBetweenBraces = getLLVMStyle();
+  SpaceBetweenBraces.SpacesInAngles = true;
+  SpaceBetweenBraces.SpacesInParentheses = true;
+  SpaceBetweenBraces.SpacesInSquareBrackets = true;
+  verifyFormat("vector< int > x{ 1, 2, 3, 4 };", SpaceBetweenBraces);
+  verifyFormat("f( {}, { {}, {} }, MyMap[ { k, v } ] );", SpaceBetweenBraces);
+  verifyFormat("vector< int > x{ // comment 1\n"
+   " 1, 2, 3, 4 };",
+   SpaceBetweenBraces);
+  SpaceBetweenBraces.ColumnLimit = 20;
+  EXPECT_EQ("vector< int > x{\n"
+"1, 2, 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  SpaceBetweenBraces.ColumnLimit = 24;
+  EXPECT_EQ("vector< int > x{ 1, 2,\n"
+" 3, 4 };",
+format("vectorx{1,2,3,4};", SpaceBetweenBraces));
+  EXPECT_EQ("vector< int > x{\n"
+

[PATCH] D68332: [clang-format] [PR43531] clang-format damages "alternative representations" for operators

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68332/new/

https://reviews.llvm.org/D68332



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67773: [clang-format[PR43144] Add support for SpaceAroundBraces style

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar requested changes to this revision.
mitchell-stellar added a comment.
This revision now requires changes to proceed.

This needs to be more thought out. At the very least it needs to cover more 
keywords like `do`, `switch`, `try`, and `catch`. There may be more. 
Consideration also needs to be made for `class`, functions, `namespace`, etc. 
Otherwise this is an experimental feature at best. There is also confusion with 
how it may interact with `SpaceBeforeCpp11BracedList`. The name 
`SpacesAroundBraces` is just too generic in that regard considering the fact 
that braces play multiple roles in C++.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67773/new/

https://reviews.llvm.org/D67773



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68473: [clang-format] SpacesInSquareBrackets should affect lambdas with parameters too

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar created this revision.
mitchell-stellar added reviewers: MyDeveloperDay, reuk, owenpan.
mitchell-stellar added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch makes the `SpacesInSquareBrackets` setting also apply to C++ lambdas 
with parameters.

Looking through the revision history, it appears support for only array 
brackets was added, and lambda brackets were ignored. Therefore, I am inclined 
to think it was simply an omission, rather than a deliberate choice.

See https://bugs.llvm.org/show_bug.cgi?id=17887 and 
https://reviews.llvm.org/D4944.


Repository:
  rC Clang

https://reviews.llvm.org/D68473

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10511,10 +10511,6 @@
 
   FormatStyle Spaces = getLLVMStyle();
   Spaces.SpacesInSquareBrackets = true;
-  // Lambdas unchanged.
-  verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
-  verifyFormat("return [i, args...] {};", Spaces);
-
   // Not lambdas.
   verifyFormat("int a[ 5 ];", Spaces);
   verifyFormat("a[ 3 ] += 42;", Spaces);
@@ -10525,6 +10521,9 @@
   verifyFormat("std::unique_ptr foo() {}", Spaces);
   verifyFormat("int i = a[ a ][ a ]->f();", Spaces);
   verifyFormat("int i = (*b)[ a ]->f();", Spaces);
+  // Lambdas.
+  verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
+  verifyFormat("return [ i, args... ] {};", Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpaceBeforeAssignmentOperators) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2609,7 +2609,8 @@
 return (Left.is(TT_ArrayInitializerLSquare) && Right.isNot(tok::r_square) 
&&
 SpaceRequiredForArrayInitializerLSquare(Left, Style)) ||
(Left.isOneOf(TT_ArraySubscriptLSquare,
- TT_StructuredBindingLSquare) &&
+ TT_StructuredBindingLSquare,
+ TT_LambdaLSquare) &&
 Style.SpacesInSquareBrackets && Right.isNot(tok::r_square));
   if (Right.is(tok::r_square))
 return Right.MatchingParen &&
@@ -2618,7 +2619,8 @@
  Style)) ||
 (Style.SpacesInSquareBrackets &&
  Right.MatchingParen->isOneOf(TT_ArraySubscriptLSquare,
-  TT_StructuredBindingLSquare)) ||
+  TT_StructuredBindingLSquare,
+  TT_LambdaLSquare)) ||
 Right.MatchingParen->is(TT_AttributeParen));
   if (Right.is(tok::l_square) &&
   !Right.isOneOf(TT_ObjCMethodExpr, TT_LambdaLSquare,
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -2301,7 +2301,8 @@
 
 **SpacesInSquareBrackets** (``bool``)
   If ``true``, spaces will be inserted after ``[`` and before ``]``.
-  Lambdas or unspecified size array declarations will not be affected.
+  Lambdas without arguments or unspecified size array declarations will not be
+  affected.
 
   .. code-block:: c++
 


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10511,10 +10511,6 @@
 
   FormatStyle Spaces = getLLVMStyle();
   Spaces.SpacesInSquareBrackets = true;
-  // Lambdas unchanged.
-  verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
-  verifyFormat("return [i, args...] {};", Spaces);
-
   // Not lambdas.
   verifyFormat("int a[ 5 ];", Spaces);
   verifyFormat("a[ 3 ] += 42;", Spaces);
@@ -10525,6 +10521,9 @@
   verifyFormat("std::unique_ptr foo() {}", Spaces);
   verifyFormat("int i = a[ a ][ a ]->f();", Spaces);
   verifyFormat("int i = (*b)[ a ]->f();", Spaces);
+  // Lambdas.
+  verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
+  verifyFormat("return [ i, args... ] {};", Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpaceBeforeAssignmentOperators) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2609,7 +2609,8 @@
 return (Left.is(TT_ArrayInitializerLSquare) && Right.isNot(tok::r_square) &&
 SpaceRequiredForArrayInitializerLSquare(Left, Style)) ||
(Left.isOneOf(TT_ArraySubscriptLSquare,
- TT_StructuredBindingLSquare) &&
+

[PATCH] D68473: [clang-format] SpacesInSquareBrackets should affect lambdas with parameters too

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

Thanks, please commit on my behalf.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68473/new/

https://reviews.llvm.org/D68473



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68481: [clang-format] [PR27004] omits leading space for noexcept when formatting operator delete()

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar requested changes to this revision.
mitchell-stellar added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1617
+if (Tok.Next->isOneOf(tok::kw_noexcept, tok::kw_volatile, tok::kw_const,
+  tok::kw_throw, tok::l_square, tok::arrow))
+  return false;

What about the "override" decorator? Would that apply here too?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68481/new/

https://reviews.llvm.org/D68481



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68481: [clang-format] [PR27004] omits leading space for noexcept when formatting operator delete()

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68481/new/

https://reviews.llvm.org/D68481



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68551: [clang-format] [NFC] Ensure clang-format is itself clang-formatted.

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

LGTM.

I agree that a system in place that either enforces clang-formatting on commit 
or after the fact would be ideal. Otherwise, I don't see a need to have to 
approve these NFC commits.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68551/new/

https://reviews.llvm.org/D68551



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29039: [clang-format] Proposal for clang-format -r option

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a subscriber: djasper.
mitchell-stellar added a comment.

I agree with @djasper that this is outside the scope of clang-format. git for 
Windows gives you a full set of bash utilities to utilize, so doing stuff like 
this on Windows is much easier now.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D29039/new/

https://reviews.llvm.org/D29039



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

I don't care for the command line switches that try to emulate compiler flags. 
I am also of the opinion that external scripts should be used for this 
functionality. git for Windows gives you a full set of bash utilities to 
utilize, so doing stuff like this on Windows is much easier now.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68554/new/

https://reviews.llvm.org/D68554



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:847
 case tok::kw_while:
-  assert(!Line.startsWith(tok::hash));
-  if (Tok->is(tok::kw_if) && CurrentToken &&
-  CurrentToken->isOneOf(tok::kw_constexpr, tok::identifier))
-next();
-  if (CurrentToken && CurrentToken->is(tok::l_paren)) {
-next();
-if (!parseParens(/*LookForDecls=*/true))
-  return false;
+  if (!Line.startsWith(tok::hash)) {
+if (Tok->is(tok::kw_if) && CurrentToken &&

It's not clear to me whether or not the token should be consumed. The previous 
assertion leads me to think no, and in that case, I think this should be
```
if (Line.startsWith(tok::hash))
return false;
```
A comment on this would also be helpful.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68707/new/

https://reviews.llvm.org/D68707



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

To me, whether or not the assertion was valid is irrelevant. That assertion was 
intentionally added, and its replacement with an `if()` statement materially 
changes the inputs and outputs of the function. I'm questioning whether the 
input of a "while" token within a preprocessor statement should output `true` 
or `false`. (Before, it didn't output anything; it errored.) Does this make 
sense? I'm just asking for clarification on this change.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68707/new/

https://reviews.llvm.org/D68707



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

Okay.

LGTM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68707/new/

https://reviews.llvm.org/D68707



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68346: [clang-format] Add new option to add spaces around conditions

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar requested changes to this revision.
mitchell-stellar added inline comments.
This revision now requires changes to proceed.



Comment at: clang/include/clang/Format/Format.h:1955
+  /// \endcode
+  bool SpacesInConditionalStatement;
+

Nitpick: this option is still not in alphabetical order. Please re-order it. 
Otherwise, this looks good to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68346/new/

https://reviews.llvm.org/D68346



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68346: [clang-format] Add new option to add spaces around conditions

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68346/new/

https://reviews.llvm.org/D68346



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69404: [clang-format] [NFC] update the documentation in Format.h to allow dump_format_style.py to get a little closer to being correct.

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69404/new/

https://reviews.llvm.org/D69404



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69433: [clang-format] [NFC] update the documentation in Format.h to allow dump_format_style.py to get a little closer to being correct. (part 2)

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:175
+val = line.replace(',', '')
+pos = val.find(" // ")
+if (pos != -1):

This seems quite flimsy to me, as it depends on an undocumented comment style. 
It is true that if the file(s) in question are properly clang-formatted, then 
this would probably not fail, but it does not appear to be a very robust 
solution.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69433/new/

https://reviews.llvm.org/D69433



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69433: [clang-format] [NFC] update the documentation in Format.h to allow dump_format_style.py to get a little closer to being correct. (part 2)

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added inline comments.



Comment at: clang/docs/tools/dump_format_style.py:175
+val = line.replace(',', '')
+pos = val.find(" // ")
+if (pos != -1):

MyDeveloperDay wrote:
> mitchell-stellar wrote:
> > This seems quite flimsy to me, as it depends on an undocumented comment 
> > style. It is true that if the file(s) in question are properly 
> > clang-formatted, then this would probably not fail, but it does not appear 
> > to be a very robust solution.
> I'd tend to agree, but this whole dump_format_style.py is flimsy.. take a 
> look at this review {D31574} 
> 
> When you added this line, you forgot the third /
> 
> ```// Different ways to wrap braces after control statements.```
> 
> Also, the extra empty line in the LanguageStandard both caused the whole 
> python file to fail with an exception.
> 
> Do you have a suggestion for something better? (which doesn't leave the 
> Format.h looking too odd)
I would go back to the `/// c++03: Parse and format as C++03.` style. `///` is 
a Doxygen comment, and I think documentation should be generated solely from 
Doxygen comments, even if it requires a bit of post-processing. (The extra `/` 
needed after `//` in the ticket you mentioned is justified.)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69433/new/

https://reviews.llvm.org/D69433



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69433: [clang-format] [NFC] update the documentation in Format.h to allow dump_format_style.py to get a little closer to being correct. (part 2)

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

Sounds like you know what you're doing.

LGTM


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69433/new/

https://reviews.llvm.org/D69433



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75364: [clang-format] Handle macros in function params and return value

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:7359
   verifyIndependentOfContext("bool a = f() && final.f();");
+  verifyFormat("int f(M(x) *p1 = nullptr, M(x) *p2, volatile M(x) *p3);");
+  verifyFormat("M(x) *foo();");

I would like to see the macro definitions in your tests. It's not clear from 
looking at just the test line that `M` is supposed to be a macro. It looks like 
a syntax error.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75364/new/

https://reviews.llvm.org/D75364



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85304: [clang-format] fix BreakBeforeBraces.MultiLine with for each macros

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85304/new/

https://reviews.llvm.org/D85304

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85304: [clang-format] fix BreakBeforeBraces.MultiLine with for each macros

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7ad60f6452ff: [clang-format] fix BreakBeforeBraces.MultiLine 
with for each macros (authored by mitchell-stellar).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85304/new/

https://reviews.llvm.org/D85304

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1663,6 +1663,20 @@
 "  foo();\n"
 "}",
 format("for(int i=0;i<10;++i){foo();}", Style));
+  EXPECT_EQ("foreach (int i,\n"
+" list)\n"
+"{\n"
+"  foo();\n"
+"}",
+format("foreach(int i, list){foo();}", Style));
+  Style.ColumnLimit =
+  40; // to concentrate at brace wrapping, not line wrap due to column 
limit
+  EXPECT_EQ("foreach (int i, list) {\n"
+"  foo();\n"
+"}",
+format("foreach(int i, list){foo();}", Style));
+  Style.ColumnLimit =
+  20; // to concentrate at brace wrapping, not line wrap due to column 
limit
   EXPECT_EQ("while (foo || bar ||\n"
 "   baz)\n"
 "{\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -309,7 +309,8 @@
 // Try to merge a control statement block with left brace wrapped
 if (I[1]->First->is(tok::l_brace) &&
 (TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
- tok::kw_switch, tok::kw_try, tok::kw_do) ||
+ tok::kw_switch, tok::kw_try, tok::kw_do,
+ TT_ForEachMacro) ||
  (TheLine->First->is(tok::r_brace) && TheLine->First->Next &&
   TheLine->First->Next->isOneOf(tok::kw_else, tok::kw_catch))) &&
 Style.BraceWrapping.AfterControlStatement ==


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1663,6 +1663,20 @@
 "  foo();\n"
 "}",
 format("for(int i=0;i<10;++i){foo();}", Style));
+  EXPECT_EQ("foreach (int i,\n"
+" list)\n"
+"{\n"
+"  foo();\n"
+"}",
+format("foreach(int i, list){foo();}", Style));
+  Style.ColumnLimit =
+  40; // to concentrate at brace wrapping, not line wrap due to column limit
+  EXPECT_EQ("foreach (int i, list) {\n"
+"  foo();\n"
+"}",
+format("foreach(int i, list){foo();}", Style));
+  Style.ColumnLimit =
+  20; // to concentrate at brace wrapping, not line wrap due to column limit
   EXPECT_EQ("while (foo || bar ||\n"
 "   baz)\n"
 "{\n"
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -309,7 +309,8 @@
 // Try to merge a control statement block with left brace wrapped
 if (I[1]->First->is(tok::l_brace) &&
 (TheLine->First->isOneOf(tok::kw_if, tok::kw_while, tok::kw_for,
- tok::kw_switch, tok::kw_try, tok::kw_do) ||
+ tok::kw_switch, tok::kw_try, tok::kw_do,
+ TT_ForEachMacro) ||
  (TheLine->First->is(tok::r_brace) && TheLine->First->Next &&
   TheLine->First->Next->isOneOf(tok::kw_else, tok::kw_catch))) &&
 Style.BraceWrapping.AfterControlStatement ==
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D78869: clang-format: Add ControlStatementsExceptForEachMacros option to SpaceBeforeParens

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

I often:

1. Commit the change to my local repo, following the style of previous commit 
messages for clang-format.
2. Build and run the clang-format tests and make sure they pass.
3. Pull, rebase, and push. (Linear history is enforced.)
4. Build and run the clang-format tests again just in case. (Optional)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78869/new/

https://reviews.llvm.org/D78869



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71939: clang-format: fix conflict between FormatStyle::BWACS_MultiLine and BeforeCatch/BeforeElse

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd45aafa2fbcf: [clang-format] fix conflict between 
FormatStyle::BWACS_MultiLine and… (authored by mitchell-stellar).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71939/new/

https://reviews.llvm.org/D71939

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1565,6 +1565,41 @@
 "  baz();\n"
 "}",
 format("try{foo();}catch(Exception&bar){baz();}", Style));
+  Style.ColumnLimit =
+  40; // to concentrate at brace wrapping, not line wrap due to column 
limit
+  EXPECT_EQ("try {\n"
+"  foo();\n"
+"} catch (Exception &bar) {\n"
+"  baz();\n"
+"}",
+format("try{foo();}catch(Exception&bar){baz();}", Style));
+  Style.ColumnLimit =
+  20; // to concentrate at brace wrapping, not line wrap due to column 
limit
+
+  Style.BraceWrapping.BeforeElse = true;
+  EXPECT_EQ(
+  "if (foo) {\n"
+  "  bar();\n"
+  "}\n"
+  "else if (baz ||\n"
+  " quux)\n"
+  "{\n"
+  "  foobar();\n"
+  "}\n"
+  "else {\n"
+  "  barbaz();\n"
+  "}",
+  format("if(foo){bar();}else if(baz||quux){foobar();}else{barbaz();}",
+ Style));
+
+  Style.BraceWrapping.BeforeCatch = true;
+  EXPECT_EQ("try {\n"
+"  foo();\n"
+"}\n"
+"catch (...) {\n"
+"  baz();\n"
+"}",
+format("try{foo();}catch(...){baz();}", Style));
 }
 
 
//===--===//
@@ -14881,7 +14916,7 @@
   verifyFormat("auto lambda = [&a = a]() { a = 2; };", AlignStyle);
 }
 
- TEST_F(FormatTest, SpacesInConditionalStatement) {
+TEST_F(FormatTest, SpacesInConditionalStatement) {
   FormatStyle Spaces = getLLVMStyle();
   Spaces.SpacesInConditionalStatement = true;
   verifyFormat("for ( int i = 0; i; i++ )\n  continue;", Spaces);
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -326,6 +326,19 @@
   FormatStyle::BWACS_Always)
  ? tryMergeSimpleBlock(I, E, Limit)
  : 0;
+} else if (I[1]->First->is(tok::l_brace) &&
+   TheLine->First->isOneOf(tok::kw_else, tok::kw_catch) &&
+   Style.BraceWrapping.AfterControlStatement ==
+   FormatStyle::BWACS_MultiLine) {
+  // This case if different from the upper BWACS_MultiLine processing
+  // in that a preceding r_brace is not on the same line as else/catch
+  // most likely because of BeforeElse/BeforeCatch set to true.
+  // If the line length doesn't fit ColumnLimit, leave l_brace on the
+  // next line to respect the BWACS_MultiLine.
+  return (Style.ColumnLimit == 0 ||
+  TheLine->Last->TotalLength <= Style.ColumnLimit)
+ ? 1
+ : 0;
 }
 // Try to merge either empty or one-line block if is precedeed by control
 // statement token


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1565,6 +1565,41 @@
 "  baz();\n"
 "}",
 format("try{foo();}catch(Exception&bar){baz();}", Style));
+  Style.ColumnLimit =
+  40; // to concentrate at brace wrapping, not line wrap due to column limit
+  EXPECT_EQ("try {\n"
+"  foo();\n"
+"} catch (Exception &bar) {\n"
+"  baz();\n"
+"}",
+format("try{foo();}catch(Exception&bar){baz();}", Style));
+  Style.ColumnLimit =
+  20; // to concentrate at brace wrapping, not line wrap due to column limit
+
+  Style.BraceWrapping.BeforeElse = true;
+  EXPECT_EQ(
+  "if (foo) {\n"
+  "  bar();\n"
+  "}\n"
+  "else if (baz ||\n"
+  " quux)\n"
+  "{\n"
+  "  foobar();\n"
+  "}\n"
+  "else {\n"
+  "  barbaz();\n"
+  "}",
+  format("if(foo){bar();}else if(baz||quux){foobar();}else{barbaz();}",
+ Style));
+
+  Style.BraceWrapping.BeforeCatch = true;
+  EXPECT_EQ("try {\n"
+"  foo();\n"
+"}\n"
+"catch (...) {\n"
+"  baz();\n"
+"}",
+format("try{foo();}catch(...){baz();}", Style));
 }
 
 //===--===//
@@ -14881,7 +14916,7 @@
   verifyFormat("auto lambda = [&a = a]() { a = 2; };"

[PATCH] D70270: clang-tidy: modernize-use-using uses AST and now supports struct defintions and multiple types in a typedef

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG73d93617d3ae: [clang-tidy] modernize-use-using uses AST and 
now supports struct defintions… (authored by mitchell-stellar).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70270/new/

https://reviews.llvm.org/D70270

Files:
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-using.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
  clang/include/clang/Basic/SourceLocation.h

Index: clang/include/clang/Basic/SourceLocation.h
===
--- clang/include/clang/Basic/SourceLocation.h
+++ clang/include/clang/Basic/SourceLocation.h
@@ -230,6 +230,11 @@
 return B != X.B || E != X.E;
   }
 
+  // Returns true iff other is wholly contained within this range.
+  bool fullyContains(const SourceRange &other) const {
+return B <= other.B && E >= other.E;
+  }
+
   void print(raw_ostream &OS, const SourceManager &SM) const;
   std::string printToString(const SourceManager &SM) const;
   void dump(const SourceManager &SM) const;
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
@@ -84,7 +84,11 @@
 
 typedef int bla1, bla2, bla3;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef int bla1, bla2, bla3;
+// CHECK-MESSAGES: :[[@LINE-2]]:17: warning: use 'using' instead of 'typedef'
+// CHECK-MESSAGES: :[[@LINE-3]]:23: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using bla1 = int;
+// CHECK-FIXES-NEXT: using bla2 = int;
+// CHECK-FIXES-NEXT: using bla3 = int;
 
 #define CODE typedef int INT
 
@@ -136,16 +140,16 @@
 
 typedef struct Q1 { int a; } S1;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef struct Q1 { int a; } S1;
+// CHECK-FIXES: using S1 = struct Q1 { int a; };
 typedef struct { int b; } S2;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef struct { int b; } S2;
+// CHECK-FIXES: using S2 = struct { int b; };
 struct Q2 { int c; } typedef S3;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: struct Q2 { int c; } typedef S3;
+// CHECK-FIXES: using S3 = struct Q2 { int c; };
 struct { int d; } typedef S4;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: struct { int d; } typedef S4;
+// CHECK-FIXES: using S4 = struct { int d; };
 
 namespace my_space {
   class my_cclass {};
@@ -196,11 +200,15 @@
 
 typedef S<(0 > 0), int> S_t, *S_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef S<(0 > 0), int> S_t, *S_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:28: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using S_t = S<(0 > 0), int>;
+// CHECK-FIXES-NEXT: using S_p = S_t*;
 
 typedef S<(0 < 0), int> S2_t, *S2_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef S<(0 < 0), int> S2_t, *S2_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using S2_t = S<(0 < 0), int>;
+// CHECK-FIXES-NEXT: using S2_p = S2_t*;
 
 typedef S<(0 > 0 && (3 > 1) && (1 < 1)), int> S3_t;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
@@ -213,7 +221,9 @@
 
 typedef Q Q_t, *Q_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef Q Q_t, *Q_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:24: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Q_t = Q;
+// CHECK-FIXES-NEXT: using Q_p = Q_t*;
 
 typedef Q Q2_t;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
@@ -227,7 +237,9 @@
 
 typedef Q Q3_t, *Q3_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef Q Q3_t, *Q3_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:27: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Q3_t = Q;
+// CHECK-FIXES-NEXT: using Q3_p = Q3_t*;
 
 typedef Q Q3_t;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
@@ -246,4 +258,12 @@
 
 typedef Variadic >, S<(0 < 0), Variadic > > > Variadic_t, *Variadic_p;
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
-// CHECK-FIXES: typedef Variadic >, S<(0 < 0), Variadic > > > Variadic_t, *Variadic_p;
+// CHECK-MESSAGES: :[[@LINE-2]]:103: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: u

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.

I am in favor of landing this and iterating.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79773/new/

https://reviews.llvm.org/D79773

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D96744: clang-format IncludeBlocks: Regroup determination of "main" for framework-style includes fix

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

I am in favor of a new option, `IncludeIsMainAllowBraces`. It keeps things 
simple and does not cause any backwards-compatibility issues. The `${}` 
variables are clever, but if "users unconcerned with formatting ... could get 
the current functionality by including a simple rule [that includes a 
variable]", I think it might be asking too much. Being able to write (and read) 
an include regex without referring to documentation seems ideal to me. You 
mentioned automatically adding variables in rules for backwards-compatibility, 
but I'd be a bit concerned about robustness and too much magic going on behind 
the scenes for users to understand if something goes wrong or has 
unexpected/unexplained results.

Admittedly I don't fully comprehend the proposed additions and all their 
ins-and-outs, but I do understand a new, simple option.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96744/new/

https://reviews.llvm.org/D96744

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134042: [clang-format] Fix alignment in #else preprocessor blocks

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar created this revision.
mitchell-stellar added reviewers: HazardyKnusperkeks, curdeius, MyDeveloperDay.
mitchell-stellar added a project: clang-format.
Herald added a project: All.
mitchell-stellar requested review of this revision.
Herald added a project: clang.

Fixes https://github.com/llvm/llvm-project/issues/36070

clang-format makes multiple passes when #if/#else preprocessor blocks are 
found. It will make one pass for normal code and code in the #if block, and 
then it will make another pass for just the code in #else blocks. This often 
results in invalid alignment inside the else blocks because they do not have 
any scope or indentAndNestingLevel context from their surrounding tokens/lines.

This patch remedies that by caching any initial indentAndNestingLevel from a 
second pass and not breaking/returning early when a scope change is detected.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134042

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5743,6 +5743,139 @@
Style);
 }
 
+TEST_F(FormatTest, FormatAlignInsidePreprocessorElseBlock) {
+  FormatStyle Style = getLLVMStyleWithColumns(40);
+  Style.AlignConsecutiveAssignments.Enabled = true;
+  Style.AlignConsecutiveDeclarations.Enabled = true;
+
+  // Test with just #if blocks.
+  {
+const char *Expected = "void f1() {\n"
+   "#if 1\n"
+   "  int foo= 1;\n"
+   "  int foobar = 2;\n"
+   "#endif\n"
+   "}\n"
+   "\n"
+   "#if 1\n"
+   "int baz = 3;\n"
+   "#endif\n"
+   "\n"
+   "void f2() {\n"
+   "#if 1\n"
+   "  char *foobarbaz = \"foobarbaz\";\n"
+   "  int   quux  = 4;\n"
+   "}";
+const char *ToFormat = "void f1() {\n"
+   "#if 1\n"
+   "  int foo = 1;\n"
+   "  int foobar = 2;\n"
+   "#endif\n"
+   "}\n"
+   "\n"
+   "#if 1\n"
+   "int baz = 3;\n"
+   "#endif\n"
+   "\n"
+   "void f2() {\n"
+   "#if 1\n"
+   "  char *foobarbaz = \"foobarbaz\";\n"
+   "  int quux = 4;\n"
+   "}";
+EXPECT_EQ(Expected, format(ToFormat, Style));
+EXPECT_EQ(Expected, format(Expected, Style));
+  }
+
+  // Test with just #else blocks.
+  {
+const char *Expected = "void f1() {\n"
+   "#if 1\n"
+   "#else\n"
+   "  int foo= 1;\n"
+   "  int foobar = 2;\n"
+   "#endif\n"
+   "}\n"
+   "\n"
+   "#if 1\n"
+   "#else\n"
+   "int baz = 3;\n"
+   "#endif\n"
+   "\n"
+   "void f2() {\n"
+   "#if 1\n"
+   "#else\n"
+   "  char *foobarbaz = \"foobarbaz\";\n"
+   "  int   quux  = 4;\n"
+   "}";
+const char *ToFormat = "void f1() {\n"
+   "#if 1\n"
+   "#else\n"
+   "  int foo = 1;\n"
+   "  int foobar = 2;\n"
+   "#endif\n"
+   "}\n"
+   "\n"
+   "#if 1\n"
+   "#else\n"
+   "int baz = 3;\n"
+   "#endif\n"
+   "\n"
+   "void f2() {\n"
+   "#if 1\n"
+   "#else\n"
+   "  char *foobarbaz = \"foobarbaz\";\n"
+   "  int quux = 4;\n"
+   "}";
+EXPECT_EQ(Expected, format(ToFormat, Style));
+EXPECT_EQ(Expected, format(Expected, Style));
+  }
+
+  // Test with a mix of #if and #else blocks.
+  {
+const char *Expected = "void f1() {\n"
+   "#if 1\n"
+   "#else\n"
+   "  int foo= 1;\n"
+   "  in

[PATCH] D134042: [clang-format] Fix alignment in #else preprocessor blocks

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar updated this revision to Diff 460890.
mitchell-stellar added a comment.

Added context and use verifyFormat().


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134042/new/

https://reviews.llvm.org/D134042

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5743,6 +5743,69 @@
Style);
 }
 
+TEST_F(FormatTest, FormatAlignInsidePreprocessorElseBlock) {
+  FormatStyle Style = getLLVMStyleWithColumns(40);
+  Style.AlignConsecutiveAssignments.Enabled = true;
+  Style.AlignConsecutiveDeclarations.Enabled = true;
+
+  // Test with just #if blocks.
+  verifyFormat("void f1() {\n"
+   "#if 1\n"
+   "  int foo= 1;\n"
+   "  int foobar = 2;\n"
+   "#endif\n"
+   "}\n"
+   "#if 1\n"
+   "int baz = 3;\n"
+   "#endif\n"
+   "void f2() {\n"
+   "#if 1\n"
+   "  char *foobarbaz = \"foobarbaz\";\n"
+   "  int   quux  = 4;\n"
+   "}",
+   Style);
+
+  // Test with just #else blocks.
+  verifyFormat("void f1() {\n"
+   "#if 1\n"
+   "#else\n"
+   "  int foo= 1;\n"
+   "  int foobar = 2;\n"
+   "#endif\n"
+   "}\n"
+   "#if 1\n"
+   "#else\n"
+   "int baz = 3;\n"
+   "#endif\n"
+   "void f2() {\n"
+   "#if 1\n"
+   "#else\n"
+   "  char *foobarbaz = \"foobarbaz\";\n"
+   "  int   quux  = 4;\n"
+   "}",
+   Style);
+
+  // Test with a mix of #if and #else blocks.
+  verifyFormat("void f1() {\n"
+   "#if 1\n"
+   "#else\n"
+   "  int foo= 1;\n"
+   "  int foobar = 2;\n"
+   "#endif\n"
+   "}\n"
+   "#if 1\n"
+   "int baz = 3;\n"
+   "#endif\n"
+   "void f2() {\n"
+   "#if 1\n"
+   "#else\n"
+   "  // prevent alignment with #else in f1\n"
+   "  char *foobarbaz = \"foobarbaz\";\n"
+   "  int   quux  = 4;\n"
+   "}",
+   Style);
+}
+
 TEST_F(FormatTest, FormatHashIfNotAtStartOfLine) {
   verifyFormat("{\n  { a #c; }\n}");
 }
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -522,6 +522,13 @@
? Changes[StartAt].indentAndNestingLevel()
: std::tuple();
 
+  // Keep track if the first token has a non-zero indent and nesting level.
+  // This can happen when aligning the contents of "#else" preprocessor blocks,
+  // which is done separately.
+  bool HasInitialIndentAndNesting =
+  StartAt == 0 &&
+  IndentAndNestingLevel > std::tuple();
+
   // Keep track of the number of commas before the matching tokens, we will only
   // align a sequence of matching tokens if they are preceded by the same number
   // of commas.
@@ -556,8 +563,19 @@
 
   unsigned i = StartAt;
   for (unsigned e = Changes.size(); i != e; ++i) {
-if (Changes[i].indentAndNestingLevel() < IndentAndNestingLevel)
-  break;
+if (Changes[i].indentAndNestingLevel() < IndentAndNestingLevel) {
+  if (!HasInitialIndentAndNesting)
+break;
+  // The contents of preprocessor blocks are aligned separately.
+  // If the initial preprocessor block is indented or nested (e.g. it's in
+  // a function), do not align and exit after finishing this scope block.
+  // Instead, align, and then lower the baseline indent and nesting level
+  // in order to continue aligning subsequent blocks.
+  EndOfSequence = i;
+  AlignCurrentSequence();
+  IndentAndNestingLevel =
+  Changes[i].indentAndNestingLevel(); // new baseline
+}
 
 if (Changes[i].NewlinesBefore != 0) {
   CommasBeforeMatch = 0;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134042: [clang-format] Fix alignment in #else preprocessor blocks

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar updated this revision to Diff 461979.
mitchell-stellar added a comment.

Do not limit columns and added nested test case.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134042/new/

https://reviews.llvm.org/D134042

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -5743,6 +5743,98 @@
Style);
 }
 
+TEST_F(FormatTest, FormatAlignInsidePreprocessorElseBlock) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveAssignments.Enabled = true;
+  Style.AlignConsecutiveDeclarations.Enabled = true;
+
+  // Test with just #if blocks.
+  verifyFormat("void f1() {\n"
+   "#if 1\n"
+   "  int foo= 1;\n"
+   "  int foobar = 2;\n"
+   "#endif\n"
+   "}\n"
+   "#if 1\n"
+   "int baz = 3;\n"
+   "#endif\n"
+   "void f2() {\n"
+   "#if 1\n"
+   "  char *foobarbaz = \"foobarbaz\";\n"
+   "  int   quux  = 4;\n"
+   "}",
+   Style);
+
+  // Test with just #else blocks.
+  verifyFormat("void f1() {\n"
+   "#if 1\n"
+   "#else\n"
+   "  int foo= 1;\n"
+   "  int foobar = 2;\n"
+   "#endif\n"
+   "}\n"
+   "#if 1\n"
+   "#else\n"
+   "int baz = 3;\n"
+   "#endif\n"
+   "void f2() {\n"
+   "#if 1\n"
+   "#else\n"
+   "  char *foobarbaz = \"foobarbaz\";\n"
+   "  int   quux  = 4;\n"
+   "}",
+   Style);
+
+  // Test with a mix of #if and #else blocks.
+  verifyFormat("void f1() {\n"
+   "#if 1\n"
+   "#else\n"
+   "  int foo= 1;\n"
+   "  int foobar = 2;\n"
+   "#endif\n"
+   "}\n"
+   "#if 1\n"
+   "int baz = 3;\n"
+   "#endif\n"
+   "void f2() {\n"
+   "#if 1\n"
+   "#else\n"
+   "  // prevent alignment with #else in f1\n"
+   "  char *foobarbaz = \"foobarbaz\";\n"
+   "  int   quux  = 4;\n"
+   "}",
+   Style);
+
+  // Test with nested #if and #else blocks.
+  verifyFormat("void f1() {\n"
+   "#if 1\n"
+   "#else\n"
+   "#if 2\n"
+   "#else\n"
+   "  int foo= 1;\n"
+   "  int foobar = 2;\n"
+   "#endif\n"
+   "#endif\n"
+   "}\n"
+   "#if 1\n"
+   "#else\n"
+   "#if 2\n"
+   "int baz = 3;\n"
+   "#endif\n"
+   "#endif\n"
+   "void f2() {\n"
+   "#if 1\n"
+   "#if 2\n"
+   "#else\n"
+   "  // prevent alignment with #else in f1\n"
+   "  char *foobarbaz = \"foobarbaz\";\n"
+   "  int   quux  = 4;\n"
+   "#endif\n"
+   "#endif\n"
+   "}",
+   Style);
+}
+
 TEST_F(FormatTest, FormatHashIfNotAtStartOfLine) {
   verifyFormat("{\n  { a #c; }\n}");
 }
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -522,6 +522,13 @@
? Changes[StartAt].indentAndNestingLevel()
: std::tuple();
 
+  // Keep track if the first token has a non-zero indent and nesting level.
+  // This can happen when aligning the contents of "#else" preprocessor blocks,
+  // which is done separately.
+  bool HasInitialIndentAndNesting =
+  StartAt == 0 &&
+  IndentAndNestingLevel > std::tuple();
+
   // Keep track of the number of commas before the matching tokens, we will only
   // align a sequence of matching tokens if they are preceded by the same number
   // of commas.
@@ -556,8 +563,19 @@
 
   unsigned i = StartAt;
   for (unsigned e = Changes.size(); i != e; ++i) {
-if (Changes[i].indentAndNestingLevel() < IndentAndNestingLevel)
-  break;
+if (Changes[i].indentAndNestingLevel() < IndentAndNestingLevel) {
+  if (!HasInitialIndentAndNesting)
+break;
+  // The contents of preprocessor blocks are aligned separately.
+  // If the initial preprocessor block is indented or nested (e.g. it's in
+  // a function), do not align and exit after finishing this scope block.
+  // Instead, align, and then lower the baseline indent and nesting level
+  // in order to continue aligning subsequent blocks.
+  EndOfSequ

[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.

Assuming this passes all existing tests, LGTM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75022/new/

https://reviews.llvm.org/D75022



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75034: [clang-format] use spaces for alignment with UT_ForContinuationAndIndentation

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a subscriber: mxbOctasic.
mitchell-stellar added a comment.

Digging through the history, it seems this behavior was explicitly desired: 
https://reviews.llvm.org/D19028. @mxbOctasic was the original author and 
perhaps could comment. Regardless, I think there should be a new option.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75034/new/

https://reviews.llvm.org/D75034



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

Not that I am aware of. Whoever ends up doing the merge will likely run the 
necessary tests before committing. If you've run as many as you can, then 
hopefully all will be fine.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75022/new/

https://reviews.llvm.org/D75022



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69649: [clang-format] Fix SpacesInSquareBrackets for Lambdas with Initial "&ref" Parameter

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar created this revision.
mitchell-stellar added reviewers: MyDeveloperDay, klimek, sammccall.
mitchell-stellar added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch fixes an edge case in the `SpacesInSquareBrackets` option where an 
initial `&ref` lambda parameter is not padded with an initial space.

`int foo = [&bar ]() {}` is fixed to give `int foo = [ &bar ]() {}`


Repository:
  rC Clang

https://reviews.llvm.org/D69649

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10572,6 +10572,10 @@
   // Lambdas.
   verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
   verifyFormat("return [ i, args... ] {};", Spaces);
+  verifyFormat("int foo = [ &bar ]() {};", Spaces);
+  verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ =, &bar ]() {};", Spaces);
+  verifyFormat("int foo = [ &bar, = ]() {};", Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpaceBeforeAssignmentOperators) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2567,7 +2567,7 @@
 return Left.Tok.isLiteral() || (Left.is(tok::identifier) && Left.Previous 
&&
 Left.Previous->is(tok::kw_case));
   if (Left.is(tok::l_square) && Right.is(tok::amp))
-return false;
+return Style.SpacesInSquareBrackets;
   if (Right.is(TT_PointerOrReference)) {
 if (Left.is(tok::r_paren) && Line.MightBeFunctionDecl) {
   if (!Left.MatchingParen)


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10572,6 +10572,10 @@
   // Lambdas.
   verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
   verifyFormat("return [ i, args... ] {};", Spaces);
+  verifyFormat("int foo = [ &bar ]() {};", Spaces);
+  verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ =, &bar ]() {};", Spaces);
+  verifyFormat("int foo = [ &bar, = ]() {};", Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpaceBeforeAssignmentOperators) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2567,7 +2567,7 @@
 return Left.Tok.isLiteral() || (Left.is(tok::identifier) && Left.Previous &&
 Left.Previous->is(tok::kw_case));
   if (Left.is(tok::l_square) && Right.is(tok::amp))
-return false;
+return Style.SpacesInSquareBrackets;
   if (Right.is(TT_PointerOrReference)) {
 if (Left.is(tok::r_paren) && Line.MightBeFunctionDecl) {
   if (!Left.MatchingParen)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69649: [clang-format] Fix SpacesInSquareBrackets for Lambdas with Initial "&ref" Parameter

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

Thanks. Would you mind committing this on my behalf? It seems I wasn't migrated 
from SVN access to git access. It may take some time to sort out.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69649/new/

https://reviews.llvm.org/D69649



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69649: [clang-format] Fix SpacesInSquareBrackets for Lambdas with Initial "&ref" Parameter

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8d7bd5752648: [clang-format] Fix SpacesInSquareBrackets for 
Lambdas with Initial "&ref"… (authored by mitchell-stellar).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69649/new/

https://reviews.llvm.org/D69649

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10572,6 +10572,10 @@
   // Lambdas.
   verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
   verifyFormat("return [ i, args... ] {};", Spaces);
+  verifyFormat("int foo = [ &bar ]() {};", Spaces);
+  verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ =, &bar ]() {};", Spaces);
+  verifyFormat("int foo = [ &bar, = ]() {};", Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpaceBeforeAssignmentOperators) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2567,7 +2567,7 @@
 return Left.Tok.isLiteral() || (Left.is(tok::identifier) && Left.Previous 
&&
 Left.Previous->is(tok::kw_case));
   if (Left.is(tok::l_square) && Right.is(tok::amp))
-return false;
+return Style.SpacesInSquareBrackets;
   if (Right.is(TT_PointerOrReference)) {
 if (Left.is(tok::r_paren) && Line.MightBeFunctionDecl) {
   if (!Left.MatchingParen)


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10572,6 +10572,10 @@
   // Lambdas.
   verifyFormat("int c = []() -> int { return 2; }();\n", Spaces);
   verifyFormat("return [ i, args... ] {};", Spaces);
+  verifyFormat("int foo = [ &bar ]() {};", Spaces);
+  verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ =, &bar ]() {};", Spaces);
+  verifyFormat("int foo = [ &bar, = ]() {};", Spaces);
 }
 
 TEST_F(FormatTest, ConfigurableSpaceBeforeAssignmentOperators) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2567,7 +2567,7 @@
 return Left.Tok.isLiteral() || (Left.is(tok::identifier) && Left.Previous &&
 Left.Previous->is(tok::kw_case));
   if (Left.is(tok::l_square) && Right.is(tok::amp))
-return false;
+return Style.SpacesInSquareBrackets;
   if (Right.is(TT_PointerOrReference)) {
 if (Left.is(tok::r_paren) && Line.MightBeFunctionDecl) {
   if (!Left.MatchingParen)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69649: [clang-format] Fix SpacesInSquareBrackets for Lambdas with Initial "&ref" Parameter

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

Never mind, I got it resolved and pushed. Sorry for the noise.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69649/new/

https://reviews.llvm.org/D69649



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56345: [clang-format] Assert that filenames are not empty

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56345/new/

https://reviews.llvm.org/D56345



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70003: [clang-format] Ensure dump_format_style.py can generate ClangFormatStyleOptions.rst without manual intervention

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70003/new/

https://reviews.llvm.org/D70003



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69951: [clang-format] NFC allow Format.h to be clang-formatted but still maintain the same doc layout in ClangFormatStyleOptions.rst

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added inline comments.



Comment at: clang/include/clang/Format/Format.h:1309
 
+  // clang-format off
   /// Indent case labels one level from the switch statement.

Can this documentation be formatted in a way that avoids clang-format 
reformatting it? It doesn't look like it depends on long lines like the other 
bits in this change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69951/new/

https://reviews.llvm.org/D69951



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69951: [clang-format] NFC allow Format.h to be clang-formatted but still maintain the same doc layout in ClangFormatStyleOptions.rst

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added inline comments.



Comment at: clang/include/clang/Format/Format.h:1309
 
+  // clang-format off
   /// Indent case labels one level from the switch statement.

MyDeveloperDay wrote:
> mitchell-stellar wrote:
> > Can this documentation be formatted in a way that avoids clang-format 
> > reformatting it? It doesn't look like it depends on long lines like the 
> > other bits in this change.
> It's this line..
> 
> it turns:
> 
> ```
> When ``false``, use the same indentation level as for the switch statement.
> Switch statement body is always indented one level more than case labels.
> ```
> 
> into:
> 
> ```
> When ``false``, use the same indentation level as for the switch
> statement. Switch statement body is always indented one level more than
> case labels.
> ```
> 
> My assumption was that the author wanted the "Switch statement.." to be a new 
> paragraph, but if we don't mind then we could lose this one
The HTML is no different. If the author wanted a separate paragraph, I suspect 
that person would have used two line breaks. I would reformat this one and 
remove the clang-format switches.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69951/new/

https://reviews.llvm.org/D69951



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69951: [clang-format] NFC allow Format.h to be clang-formatted but still maintain the same doc layout in ClangFormatStyleOptions.rst

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:1589
+  statement. Switch statement body is always indented one level more than
+  case labels. \code
  false: true:

This generated rst is not correct. See below.



Comment at: clang/include/clang/Format/Format.h:1313
+  /// statement. Switch statement body is always indented one level more than
+  /// case labels. \code
   ///false: true:

`\code` needs to be on its own line. Otherwise you get the incorrectly 
generated .rst I mentioned earlier.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69951/new/

https://reviews.llvm.org/D69951



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69951: [clang-format] NFC allow Format.h to be clang-formatted but still maintain the same doc layout in ClangFormatStyleOptions.rst

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69951/new/

https://reviews.llvm.org/D69951



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54628: Extend format with AllowShortEnumsOnASingleLine option

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3109
 return Right.HasUnescapedNewline;
+  if (isAllmanBrace(Left) && !Style.AllowShortEnumsOnASingleLine &&
+  (Line.startsWith(tok::kw_enum) ||

MyDeveloperDay wrote:
> would you be happy to consider this being something like this instead?
> 
> `if (isAllmanBrace(Left) && Style.BraceWrapping.AfterEnum == 
> FormatStyle::BWAE_Attach &...`
> 
> 
> There is now precedent for the BraceWrapping styles to be an enum (see work 
> by @mitchell-stellar who did something similar for `AfterControlStylement`)
I would not recommend making this option an enum. 
`Style.BraceWrapping.AfterControlStatement` is a special case since there was a 
potential readability issue for multi-line control statement conditions. Enums 
do not suffer from this. I think the option is in line with the other 
`AllowShort*OnASingleLine` options.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54628/new/

https://reviews.llvm.org/D54628



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70249: [clang-format] Fixed edge-case with `Style.SpacesInSquareBrackets` with trailing bare `&` lambda capture

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar created this revision.
mitchell-stellar added a reviewer: MyDeveloperDay.
mitchell-stellar added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Lambda captures allow for a lone `&` capture, so `&]` needs to be properly 
handled.


Repository:
  rC Clang

https://reviews.llvm.org/D70249

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10638,6 +10638,7 @@
   verifyFormat("return [ i, args... ] {};", Spaces);
   verifyFormat("int foo = [ &bar ]() {};", Spaces);
   verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ & ]() {};", Spaces);
   verifyFormat("int foo = [ =, &bar ]() {};", Spaces);
   verifyFormat("int foo = [ &bar, = ]() {};", Spaces);
 }
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2996,14 +2996,18 @@
 return true;
   }
   if (Left.is(TT_UnaryOperator)) {
-// The alternative operators for ~ and ! are "compl" and "not".
-// If they are used instead, we do not want to combine them with
-// the token to the right, unless that is a left paren.
 if (!Right.is(tok::l_paren)) {
+  // The alternative operators for ~ and ! are "compl" and "not".
+  // If they are used instead, we do not want to combine them with
+  // the token to the right, unless that is a left paren.
   if (Left.is(tok::exclaim) && Left.TokenText == "not")
 return true;
   if (Left.is(tok::tilde) && Left.TokenText == "compl")
 return true;
+  // Lambda captures allow for a lone &, so "&]" needs to be properly
+  // handled.
+  if (Left.is(tok::amp) && Right.is(tok::r_square))
+return Style.SpacesInSquareBrackets;
 }
 return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
Right.is(TT_BinaryOperator);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10638,6 +10638,7 @@
   verifyFormat("return [ i, args... ] {};", Spaces);
   verifyFormat("int foo = [ &bar ]() {};", Spaces);
   verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ & ]() {};", Spaces);
   verifyFormat("int foo = [ =, &bar ]() {};", Spaces);
   verifyFormat("int foo = [ &bar, = ]() {};", Spaces);
 }
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2996,14 +2996,18 @@
 return true;
   }
   if (Left.is(TT_UnaryOperator)) {
-// The alternative operators for ~ and ! are "compl" and "not".
-// If they are used instead, we do not want to combine them with
-// the token to the right, unless that is a left paren.
 if (!Right.is(tok::l_paren)) {
+  // The alternative operators for ~ and ! are "compl" and "not".
+  // If they are used instead, we do not want to combine them with
+  // the token to the right, unless that is a left paren.
   if (Left.is(tok::exclaim) && Left.TokenText == "not")
 return true;
   if (Left.is(tok::tilde) && Left.TokenText == "compl")
 return true;
+  // Lambda captures allow for a lone &, so "&]" needs to be properly
+  // handled.
+  if (Left.is(tok::amp) && Right.is(tok::r_square))
+return Style.SpacesInSquareBrackets;
 }
 return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
Right.is(TT_BinaryOperator);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70249: [clang-format] Fixed edge-case with `Style.SpacesInSquareBrackets` with trailing bare `&` lambda capture

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4ee70e00b509: [clang-format] Fixed edge-case with 
SpacesInSquareBrackets with trailing bare… (authored by mitchell-stellar).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70249/new/

https://reviews.llvm.org/D70249

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10638,6 +10638,7 @@
   verifyFormat("return [ i, args... ] {};", Spaces);
   verifyFormat("int foo = [ &bar ]() {};", Spaces);
   verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ & ]() {};", Spaces);
   verifyFormat("int foo = [ =, &bar ]() {};", Spaces);
   verifyFormat("int foo = [ &bar, = ]() {};", Spaces);
 }
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2996,14 +2996,18 @@
 return true;
   }
   if (Left.is(TT_UnaryOperator)) {
-// The alternative operators for ~ and ! are "compl" and "not".
-// If they are used instead, we do not want to combine them with
-// the token to the right, unless that is a left paren.
 if (!Right.is(tok::l_paren)) {
+  // The alternative operators for ~ and ! are "compl" and "not".
+  // If they are used instead, we do not want to combine them with
+  // the token to the right, unless that is a left paren.
   if (Left.is(tok::exclaim) && Left.TokenText == "not")
 return true;
   if (Left.is(tok::tilde) && Left.TokenText == "compl")
 return true;
+  // Lambda captures allow for a lone &, so "&]" needs to be properly
+  // handled.
+  if (Left.is(tok::amp) && Right.is(tok::r_square))
+return Style.SpacesInSquareBrackets;
 }
 return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
Right.is(TT_BinaryOperator);


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10638,6 +10638,7 @@
   verifyFormat("return [ i, args... ] {};", Spaces);
   verifyFormat("int foo = [ &bar ]() {};", Spaces);
   verifyFormat("int foo = [ = ]() {};", Spaces);
+  verifyFormat("int foo = [ & ]() {};", Spaces);
   verifyFormat("int foo = [ =, &bar ]() {};", Spaces);
   verifyFormat("int foo = [ &bar, = ]() {};", Spaces);
 }
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2996,14 +2996,18 @@
 return true;
   }
   if (Left.is(TT_UnaryOperator)) {
-// The alternative operators for ~ and ! are "compl" and "not".
-// If they are used instead, we do not want to combine them with
-// the token to the right, unless that is a left paren.
 if (!Right.is(tok::l_paren)) {
+  // The alternative operators for ~ and ! are "compl" and "not".
+  // If they are used instead, we do not want to combine them with
+  // the token to the right, unless that is a left paren.
   if (Left.is(tok::exclaim) && Left.TokenText == "not")
 return true;
   if (Left.is(tok::tilde) && Left.TokenText == "compl")
 return true;
+  // Lambda captures allow for a lone &, so "&]" needs to be properly
+  // handled.
+  if (Left.is(tok::amp) && Right.is(tok::r_square))
+return Style.SpacesInSquareBrackets;
 }
 return (Style.SpaceAfterLogicalNot && Left.is(tok::exclaim)) ||
Right.is(TT_BinaryOperator);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67460: clang-tidy: modernize-use-using work with multi-argument templates

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG782392db8122: [clang-tidy] modernize-use-using work with 
multi-argument templates (authored by mitchell-stellar).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67460/new/

https://reviews.llvm.org/D67460

Files:
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
@@ -183,3 +183,67 @@
   void f() override { super::f(); }
 };
 }
+
+template 
+class TwoArgTemplate {
+  typedef TwoArgTemplate self;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef'
+  // CHECK-FIXES: using self = TwoArgTemplate;
+};
+
+template 
+struct S {};
+
+typedef S<(0 > 0), int> S_t, *S_p;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: typedef S<(0 > 0), int> S_t, *S_p;
+
+typedef S<(0 < 0), int> S2_t, *S2_p;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: typedef S<(0 < 0), int> S2_t, *S2_p;
+
+typedef S<(0 > 0 && (3 > 1) && (1 < 1)), int> S3_t;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using S3_t = S<(0 > 0 && (3 > 1) && (1 < 1)), int>;
+
+template 
+struct Q {};
+
+constexpr bool b[1] = {true};
+
+typedef Q Q_t, *Q_p;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: typedef Q Q_t, *Q_p;
+
+typedef Q Q2_t;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Q2_t = Q;
+
+struct T {
+  constexpr T(bool) {}
+
+  static constexpr bool b = true;
+};
+
+typedef Q Q3_t, *Q3_p;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: typedef Q Q3_t, *Q3_p;
+
+typedef Q Q3_t;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Q3_t = Q;
+
+typedef TwoArgTemplate >, S<(0 < 0), Q > > Nested_t;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Nested_t = TwoArgTemplate >, S<(0 < 0), Q > >;
+
+template 
+class Variadic {};
+
+typedef Variadic >, S<(0 < 0), Variadic > > > Variadic_t;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: using Variadic_t = Variadic >, S<(0 < 0), Variadic > > >
+
+typedef Variadic >, S<(0 < 0), Variadic > > > Variadic_t, *Variadic_p;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
+// CHECK-FIXES: typedef Variadic >, S<(0 < 0), Variadic > > > Variadic_t, *Variadic_p;
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -39,25 +39,46 @@
   File.begin(), TokenBegin, File.end());
 
   Token Tok;
-  int ParenLevel = 0;
+  int NestingLevel = 0; // Parens, braces, and square brackets
+  int AngleBracketLevel = 0;
   bool FoundTypedef = false;
 
   while (!DeclLexer.LexFromRawLexer(Tok) && !Tok.is(tok::semi)) {
 switch (Tok.getKind()) {
 case tok::l_brace:
-case tok::r_brace:
-  // This might be the `typedef struct {...} T;` case.
-  return false;
+  if (NestingLevel == 0 && AngleBracketLevel == 0) {
+// At top level, this might be the `typedef struct {...} T;` case.
+// Inside parens, square brackets, or angle brackets it's not.
+return false;
+  }
+  ++NestingLevel;
+  break;
 case tok::l_paren:
-  ParenLevel++;
+case tok::l_square:
+  ++NestingLevel;
   break;
+case tok::r_brace:
 case tok::r_paren:
-  ParenLevel--;
+case tok::r_square:
+  --NestingLevel;
+  break;
+case tok::less:
+  // If not nested in paren/brace/square bracket, treat as opening angle bracket.
+  if (NestingLevel == 0)
+++AngleBracketLevel;
+  break;
+case tok::greater:
+  // Per C++ 17 Draft N4659, Section 17.2/3
+  //   https://timsong-cpp.github.io/cppwp/n4659/temp.names#3:
+  // "When parsing a template-argument-list, the first non-nested > is
+  // taken as the ending delimiter rather than a greater-than operator."
+  // If not nested in paren/brace/square bracket, treat as closing angle bracket.
+  if (NestingLevel == 0)
+--AngleBracketLevel;
   break;
 case tok::comma:
-  if (ParenLevel == 0) {
-// If there is comma and we are not between open parenthesis then it is
-// two o

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG50e99563fb04: [clang-tidy] modernize-use-override new option 
AllowOverrideAndFinal (authored by mitchell-stellar).

Changed prior to commit:
  https://reviews.llvm.org/D70165?vs=229012&id=229632#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70165/new/

https://reviews.llvm.org/D70165

Files:
  clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init-msvc.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
@@ -1,5 +1,4 @@
-// RUN: %check_clang_tidy -std=c++11,c++14 %s 
readability-redundant-string-init %t
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy %s readability-redundant-string-init %t
 
 namespace std {
 template 
@@ -131,6 +130,11 @@
   // CHECK-FIXES: std::string a, b, c;
 
   std::string d = "u", e = "u", f = "u";
+
+  std::string g = "u", h = "", i = "uuu", j = "", k;
+  // CHECK-MESSAGES: [[@LINE-1]]:24: warning: redundant string initialization
+  // CHECK-MESSAGES: [[@LINE-2]]:43: warning: redundant string initialization
+  // CHECK-FIXES: std::string g = "u", h, i = "uuu", j, k;
 }
 
 // These cases should not generate warnings.
Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init-msvc.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init-msvc.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init-msvc.cpp
@@ -1,5 +1,4 @@
-// RUN: %check_clang_tidy -std=c++11,c++14 %s 
readability-redundant-string-init %t
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy %s readability-redundant-string-init %t
 
 namespace std {
 template 
Index: clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
@@ -46,23 +46,23 @@
   // string bar("");
   Finder->addMatcher(
   namedDecl(
-  varDecl(hasType(hasUnqualifiedDesugaredType(recordType(
-  
hasDeclaration(cxxRecordDecl(hasName("basic_string")),
-  hasInitializer(expr(ignoringImplicit(anyOf(
-  EmptyStringCtorExpr,
-  EmptyStringCtorExprWithTemporaries)))
- .bind("expr"))),
-  unless(parmVarDecl()))
-  .bind("decl"),
+  varDecl(
+  hasType(hasUnqualifiedDesugaredType(recordType(
+  hasDeclaration(cxxRecordDecl(hasName("basic_string")),
+  hasInitializer(expr(ignoringImplicit(anyOf(
+  EmptyStringCtorExpr, EmptyStringCtorExprWithTemporaries)
+  .bind("vardecl"),
+  unless(parmVarDecl())),
   this);
 }
 
 void RedundantStringInitCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *CtorExpr = Result.Nodes.getNodeAs("expr");
-  const auto *Decl = Result.Nodes.getNodeAs("decl");
-  diag(CtorExpr->getExprLoc(), "redundant string initialization")
-  << FixItHint::CreateReplacement(CtorExpr->getSourceRange(),
-  Decl->getName());
+  const auto *VDecl = Result.Nodes.getNodeAs("vardecl");
+  // VarDecl's getSourceRange() spans 'string foo = ""' or 'string bar("")'.
+  // So start at getLocation() to span just 'foo = ""' or 'bar("")'.
+  SourceRange ReplaceRange(VDecl->getLocation(), VDecl->getEndLoc());
+  diag(VDecl->getLocation(), "redundant string initialization")
+  << FixItHint::CreateReplacement(ReplaceRange, VDecl->getName());
 }
 
 } // namespace readability


Index: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
@@ -1,5 +1,4 @@
-// RUN: %check_clang_tidy -std=c++11,c++14 %s readability-redundant-string-init %t
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy %s readability-redundant-string-init %t
 
 namespace std {
 template 
@@ -131,6 +130,11 @@
   // CHECK-FIXES: std::string a, b, c;
 
   std

[PATCH] D69548: Give clang-tidy readability-redundant-string-init a customizable list of string types to fix

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG96fbc32cb9ea: [clang-tidy] Give 
readability-redundant-string-init a customizable list of… (authored by 
mitchell-stellar).

Changed prior to commit:
  https://reviews.llvm.org/D69548?vs=228769&id=229637#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69548/new/

https://reviews.llvm.org/D69548

Files:
  clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
  clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-redundant-string-init.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
@@ -1,4 +1,9 @@
-// RUN: %check_clang_tidy %s readability-redundant-string-init %t
+// RUN: %check_clang_tidy -std=c++11,c++14 %s readability-redundant-string-init %t \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-string-init.StringNames, \
+// RUN:   value: "::std::basic_string;our::TestString"}] \
+// RUN: }"
+// FIXME: Fix the checker to work in C++17 mode.
 
 namespace std {
 template 
@@ -143,3 +148,82 @@
 void Param3(std::string param = "") {}
 void Param4(STRING param = "") {}
 
+namespace our {
+struct TestString {
+  TestString();
+  TestString(const TestString &);
+  TestString(const char *);
+  ~TestString();
+};
+}
+
+void ourTestStringTests() {
+  our::TestString a = "";
+  // CHECK-MESSAGES: [[@LINE-1]]:19: warning: redundant string initialization
+  // CHECK-FIXES: our::TestString a;
+  our::TestString b("");
+  // CHECK-MESSAGES: [[@LINE-1]]:19: warning: redundant string initialization
+  // CHECK-FIXES: our::TestString b;
+  our::TestString c = R"()";
+  // CHECK-MESSAGES: [[@LINE-1]]:19: warning: redundant string initialization
+  // CHECK-FIXES: our::TestString c;
+  our::TestString d(R"()");
+  // CHECK-MESSAGES: [[@LINE-1]]:19: warning: redundant string initialization
+  // CHECK-FIXES: our::TestString d;
+
+  our::TestString u = "u";
+  our::TestString w("w");
+  our::TestString x = R"(x)";
+  our::TestString y(R"(y)");
+  our::TestString z;
+}
+
+namespace their {
+using TestString = our::TestString;
+}
+
+// their::TestString is the same type so should warn / fix
+void theirTestStringTests() {
+  their::TestString a = "";
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: redundant string initialization
+  // CHECK-FIXES: their::TestString a;
+  their::TestString b("");
+  // CHECK-MESSAGES: [[@LINE-1]]:21: warning: redundant string initialization
+  // CHECK-FIXES: their::TestString b;
+}
+
+namespace other {
+// Identical declarations to above but different type
+struct TestString {
+  TestString();
+  TestString(const TestString &);
+  TestString(const char *);
+  ~TestString();
+};
+
+// Identical declarations to above but different type
+template 
+class allocator {};
+template 
+class char_traits {};
+template , typename A = std::allocator>
+struct basic_string {
+  basic_string();
+  basic_string(const basic_string &);
+  basic_string(const C *, const A &a = A());
+  ~basic_string();
+};
+typedef basic_string string;
+typedef basic_string wstring;
+}
+
+// other::TestString, other::string, other::wstring are unrelated to the types
+// being checked. No warnings / fixes should be produced for these types.
+void otherTestStringTests() {
+  other::TestString a = "";
+  other::TestString b("");
+  other::string c = "";
+  other::string d("");
+  other::wstring e = L"";
+  other::wstring f(L"");
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-redundant-string-init.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-redundant-string-init.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-redundant-string-init.rst
@@ -5,7 +5,8 @@
 
 Finds unnecessary string initializations.
 
-Examples:
+Examples
+
 
 .. code-block:: c++
 
@@ -17,3 +18,15 @@
 
   std::string a;
   std::string b;
+
+Options
+---
+
+.. option:: StringNames
+
+Default is `::std::basic_string`.
+
+Semicolon-delimited list of class names to apply this check to.
+By default `::std::basic_string` applies to ``std::string`` and
+``std::wstring``. Set to e.g. `::std::basic_string;llvm::StringRef;QString`
+to perform this check on custom classes.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/Rel

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG06f3dabe4a2e: [clang-tidy] Fix 
readability-redundant-string-init for c++17/c++2a (authored by 
mitchell-stellar).

Changed prior to commit:
  https://reviews.llvm.org/D69238?vs=228556&id=229651#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69238/new/

https://reviews.llvm.org/D69238

Files:
  clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst


Index: clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
@@ -27,6 +27,14 @@
 
If set to non-zero, this check will not diagnose destructors. Default is 
`0`.
 
+.. option:: AllowOverrideAndFinal
+
+   If set to non-zero, this check will not diagnose ``override`` as redundant
+   with ``final``. This is useful when code will be compiled by a compiler with
+   warning/error checking flags requiring ``override`` explicitly on overriden
+   members, such as ``gcc -Wsuggest-override``/``gcc 
-Werror=suggest-override``.
+   Default is `0`.
+
 .. option:: OverrideSpelling
 
Specifies a macro to use instead of ``override``. This is useful when
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -164,6 +164,12 @@
   Finds non-static member functions that can be made ``const``
   because the functions don't use ``this`` in a non-const way.
 
+- Improved :doc:`modernize-use-override
+  ` check.
+
+  The check now supports the ``AllowOverrideAndFinal`` option to eliminate
+  conflicts with ``gcc -Wsuggest-override`` or ``gcc 
-Werror=suggest-override``.
+
 Improvements to include-fixer
 -
 
Index: clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
+++ clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
@@ -26,6 +26,7 @@
 
 private:
   const bool IgnoreDestructors;
+  const bool AllowOverrideAndFinal;
   const std::string OverrideSpelling;
   const std::string FinalSpelling;
 };
Index: clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
@@ -20,11 +20,13 @@
 UseOverrideCheck::UseOverrideCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
   IgnoreDestructors(Options.get("IgnoreDestructors", false)),
+  AllowOverrideAndFinal(Options.get("AllowOverrideAndFinal", false)),
   OverrideSpelling(Options.get("OverrideSpelling", "override")),
   FinalSpelling(Options.get("FinalSpelling", "final")) {}
 
 void UseOverrideCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "IgnoreDestructors", IgnoreDestructors);
+  Options.store(Opts, "AllowOverrideAndFinal", AllowOverrideAndFinal);
   Options.store(Opts, "OverrideSpelling", OverrideSpelling);
   Options.store(Opts, "FinalSpelling", FinalSpelling);
 }
@@ -103,7 +105,8 @@
   bool OnlyVirtualSpecified = HasVirtual && !HasOverride && !HasFinal;
   unsigned KeywordCount = HasVirtual + HasOverride + HasFinal;
 
-  if (!OnlyVirtualSpecified && KeywordCount == 1)
+  if ((!OnlyVirtualSpecified && KeywordCount == 1) ||
+  (!HasVirtual && HasOverride && HasFinal && AllowOverrideAndFinal))
 return; // Nothing to do.
 
   std::string Message;
@@ -113,8 +116,9 @@
 Message = "annotate this function with '%0' or (rarely) '%1'";
   } else {
 StringRef Redundant =
-HasVirtual ? (HasOverride && HasFinal ? "'virtual' and '%0' are"
-  : "'virtual' is")
+HasVirtual ? (HasOverride && HasFinal && !AllowOverrideAndFinal
+  ? "'virtual' and '%0' are"
+  : "'virtual' is")
: "'%0' is";
 StringRef Correct = HasFinal ? "'%1'" : "'%0'";
 
@@ -211,7 +215,7 @@
 Diag << FixItHint::CreateInsertion(InsertLoc, ReplacementText);
   }
 
-  if (HasFinal && HasOverride) {
+  if (HasFinal && HasOverride && !AllowOverrideAndFinal) {
 SourceLocation OverrideLoc = 
Method->getAttr()->getLocation();
 Diag << FixItHint::CreateRemoval(
 CharSourceRange::getTokenRange(OverrideLoc, OverrideLoc));


Index: clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
==

[PATCH] D70355: [clang-format] [NFC] add recent changes to release notes

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70355/new/

https://reviews.llvm.org/D70355



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

Yes, there was a failing unit test that had to be fixed. I reverted the first 
commit and then committed a version that fixed the failing test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70165/new/

https://reviews.llvm.org/D70165



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

Oops, it looks like I mixed up this ticket with 
https://reviews.llvm.org/D69238. Sorry about that. Would you like me to revert 
both commits and then re-commit with the correct links, etc.?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70165/new/

https://reviews.llvm.org/D70165



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar reopened this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

Reopening in order to correct the accidentally swapped commits between this 
ticket and https://reviews.llvm.org/D69238.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70165/new/

https://reviews.llvm.org/D70165



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar reopened this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

Reopening in order to correct the accidentally swapped commits between this 
ticket and https://reviews.llvm.org/D70165.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69238/new/

https://reviews.llvm.org/D69238



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70165: clang-tidy: modernize-use-override new option AllowOverrideAndFinal

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdf7086fe: [clang-tidy] modernize-use-override new option 
AllowOverrideAndFinal (authored by mitchell-stellar).

Changed prior to commit:
  https://reviews.llvm.org/D70165?vs=229632&id=230040#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70165/new/

https://reviews.llvm.org/D70165

Files:
  clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-override-allow-override-and-final.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-override-allow-override-and-final.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-override-allow-override-and-final.cpp
@@ -0,0 +1,40 @@
+// RUN: %check_clang_tidy %s modernize-use-override %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-override.AllowOverrideAndFinal, value: 1}]}"
+
+struct Base {
+  virtual ~Base();
+  virtual void a();
+  virtual void b();
+  virtual void c();
+  virtual void d();
+  virtual void e();
+  virtual void f();
+  virtual void g();
+  virtual void h();
+  virtual void i();
+};
+
+struct Simple : public Base {
+  virtual ~Simple();
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  ~Simple() override;
+  virtual void a() override;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant since the function is already declared 'override' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void a() override;
+  virtual void b() final;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant since the function is already declared 'final' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void b() final;
+  virtual void c() final override;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant since the function is already declared 'final' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void c() final override;
+  virtual void d() override final;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: 'virtual' is redundant since the function is already declared 'final' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void d() override final;
+  void e() final override;
+  void f() override final;
+  void g() final;
+  void h() override;
+  void i();
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: annotate this function with 'override' or (rarely) 'final' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void i() override;
+};
Index: clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize-use-override.rst
@@ -27,6 +27,14 @@
 
If set to non-zero, this check will not diagnose destructors. Default is `0`.
 
+.. option:: AllowOverrideAndFinal
+
+   If set to non-zero, this check will not diagnose ``override`` as redundant
+   with ``final``. This is useful when code will be compiled by a compiler with
+   warning/error checking flags requiring ``override`` explicitly on overriden
+   members, such as ``gcc -Wsuggest-override``/``gcc -Werror=suggest-override``.
+   Default is `0`.
+
 .. option:: OverrideSpelling
 
Specifies a macro to use instead of ``override``. This is useful when
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -170,6 +170,12 @@
   Finds non-static member functions that can be made ``const``
   because the functions don't use ``this`` in a non-const way.
 
+- Improved :doc:`modernize-use-override
+  ` check.
+
+  The check now supports the ``AllowOverrideAndFinal`` option to eliminate
+  conflicts with ``gcc -Wsuggest-override`` or ``gcc -Werror=suggest-override``.
+
 - The :doc:`readability-redundant-string-init
   ` check now supports a
   `StringNames` option enabling its application to custom string classes.
Index: clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
===
--- clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
+++ clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.h
@@ -26,6 +26,7 @@
 
 private:
   const bool IgnoreDestructors;
+  const bool AllowOverrideAndFinal;
   const std::string OverrideSpelling;
   const std::string FinalSpelling;
 };
Index: clang-tools-extra/clang-tidy/modernize/UseOverrideCheck.cpp
==

[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1315f4e009b0: [clang-tidy] Fix 
readability-redundant-string-init for c++17/c++2a (authored by 
mitchell-stellar).

Changed prior to commit:
  https://reviews.llvm.org/D69238?vs=229651&id=230039#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69238/new/

https://reviews.llvm.org/D69238

Files:
  clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init-msvc.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
@@ -1,4 +1,8 @@
-// RUN: %check_clang_tidy -std=c++11,c++14 %s 
readability-redundant-string-init %t
+// RUN: %check_clang_tidy -std=c++11,c++14 %s 
readability-redundant-string-init %t \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-string-init.StringNames, \
+// RUN:   value: '::std::basic_string;our::TestString'}] \
+// RUN: }"
 // FIXME: Fix the checker to work in C++17 mode.
 
 namespace std {
@@ -131,6 +135,11 @@
   // CHECK-FIXES: std::string a, b, c;
 
   std::string d = "u", e = "u", f = "u";
+
+  std::string g = "u", h = "", i = "uuu", j = "", k;
+  // CHECK-MESSAGES: [[@LINE-1]]:24: warning: redundant string initialization
+  // CHECK-MESSAGES: [[@LINE-2]]:43: warning: redundant string initialization
+  // CHECK-FIXES: std::string g = "u", h, i = "uuu", j, k;
 }
 
 // These cases should not generate warnings.
Index: 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init-msvc.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init-msvc.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init-msvc.cpp
@@ -1,5 +1,4 @@
-// RUN: %check_clang_tidy -std=c++11,c++14 %s 
readability-redundant-string-init %t
-// FIXME: Fix the checker to work in C++17 mode.
+// RUN: %check_clang_tidy %s readability-redundant-string-init %t
 
 namespace std {
 template 
Index: clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp
@@ -73,7 +73,7 @@
   namedDecl(
   varDecl(
   hasType(hasUnqualifiedDesugaredType(recordType(
-  hasDeclaration(cxxRecordDecl(hasName("basic_string")),
+  hasDeclaration(cxxRecordDecl(hasStringTypeName),
   hasInitializer(expr(ignoringImplicit(anyOf(
   EmptyStringCtorExpr, EmptyStringCtorExprWithTemporaries)
   .bind("vardecl"),
@@ -82,11 +82,12 @@
 }
 
 void RedundantStringInitCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *CtorExpr = Result.Nodes.getNodeAs("expr");
-  const auto *Decl = Result.Nodes.getNodeAs("decl");
-  diag(CtorExpr->getExprLoc(), "redundant string initialization")
-  << FixItHint::CreateReplacement(CtorExpr->getSourceRange(),
-  Decl->getName());
+  const auto *VDecl = Result.Nodes.getNodeAs("vardecl");
+  // VarDecl's getSourceRange() spans 'string foo = ""' or 'string bar("")'.
+  // So start at getLocation() to span just 'foo = ""' or 'bar("")'.
+  SourceRange ReplaceRange(VDecl->getLocation(), VDecl->getEndLoc());
+  diag(VDecl->getLocation(), "redundant string initialization")
+  << FixItHint::CreateReplacement(ReplaceRange, VDecl->getName());
 }
 
 } // namespace readability


Index: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-redundant-string-init.cpp
@@ -1,4 +1,8 @@
-// RUN: %check_clang_tidy -std=c++11,c++14 %s readability-redundant-string-init %t
+// RUN: %check_clang_tidy -std=c++11,c++14 %s readability-redundant-string-init %t \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-string-init.StringNames, \
+// RUN:   value: '::std::basic_string;our::TestString'}] \
+// RUN: }"
 // FIXME: Fix the checker to work in C++17 mode.
 
 namespace std {
@@ -131,6 +135,11 @@
   // CHECK-FIXES: std::string a, b, c;
 
   std::string d = "u", e = "u", f = "u";
+
+  std:

[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG980653621ef5: [clang-tidy] Give 
readability-redundant-member-init an option… (authored by mitchell-stellar).

Changed prior to commit:
  https://reviews.llvm.org/D69145?vs=229934&id=230077#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145

Files:
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-redundant-member-init.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-member-init.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-redundant-member-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-redundant-member-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-redundant-member-init.cpp
@@ -1,4 +1,8 @@
-// RUN: %check_clang_tidy %s readability-redundant-member-init %t
+// RUN: %check_clang_tidy %s readability-redundant-member-init %t \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: readability-redundant-member-init.IgnoreBaseInCopyConstructors, \
+// RUN:   value: 1}] \
+// RUN: }"
 
 struct S {
   S() = default;
@@ -116,6 +120,35 @@
   };
 };
 
+// struct whose inline copy constructor default-initializes its base class
+struct WithCopyConstructor1 : public T {
+  WithCopyConstructor1(const WithCopyConstructor1& other) : T(),
+f(),
+g()
+  {}
+  S f, g;
+};
+// No warning in copy constructor about T since IgnoreBaseInCopyConstructors=1
+// CHECK-MESSAGES: :[[@LINE-6]]:5: warning: initializer for member 'f' is redundant
+// CHECK-MESSAGES: :[[@LINE-6]]:5: warning: initializer for member 'g' is redundant
+// CHECK-FIXES: WithCopyConstructor1(const WithCopyConstructor1& other) : T()
+// CHECK-NEXT: 
+// CHECK-NEXT: 
+// CHECK-NEXT: {}
+
+// struct whose copy constructor default-initializes its base class
+struct WithCopyConstructor2 : public T {
+  WithCopyConstructor2(const WithCopyConstructor2& other);
+  S a;
+};
+WithCopyConstructor2::WithCopyConstructor2(const WithCopyConstructor2& other)
+  : T(), a()
+{}
+// No warning in copy constructor about T since IgnoreBaseInCopyConstructors=1
+// CHECK-MESSAGES: :[[@LINE-3]]:10: warning: initializer for member 'a' is redundant
+// CHECK-FIXES: {{^}}  : T() {{$}}
+// CHECK-NEXT: {}
+
 // Initializer not written
 struct NF1 {
   NF1() {}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-redundant-member-init.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-redundant-member-init.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-redundant-member-init.rst
@@ -6,7 +6,8 @@
 Finds member initializations that are unnecessary because the same default
 constructor would be called if they were not present.
 
-Example:
+Example
+---
 
 .. code-block:: c++
 
@@ -18,3 +19,27 @@
   private:
 std::string s;
   };
+
+Options
+---
+
+.. option:: IgnoreBaseInCopyConstructors
+
+Default is ``0``.
+
+When non-zero, the check will ignore unnecessary base class initializations
+within copy constructors, since some compilers issue warnings/errors when
+base classes are not explicitly intialized in copy constructors. For example,
+``gcc`` with ``-Wextra`` or ``-Werror=extra`` issues warning or error
+``base class ‘Bar’ should be explicitly initialized in the copy constructor``
+if ``Bar()`` were removed in the following example:
+
+.. code-block:: c++
+
+  // Explicitly initializing member s and base class Bar is unnecessary.
+  struct Foo : public Bar {
+// Remove s() below. If IgnoreBaseInCopyConstructors!=0, keep Bar().
+Foo(const Foo& foo) : Bar(), s() {}
+std::string s;
+  };
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -176,6 +176,13 @@
   The check now supports the ``AllowOverrideAndFinal`` option to eliminate
   conflicts with ``gcc -Wsuggest-override`` or ``gcc -Werror=suggest-override``.
 
+- Improved :doc:`readability-redundant-member-init
+  ` check.
+
+  The check  now supports the ``IgnoreBaseInCopyConstructors`` option to avoid
+  `"base class \‘Foo\’ should be explicitly initialized in the copy constructor"`
+  warnings or errors with ``gcc -Wextra`` or ``gcc -Werror=extra``.
+
 - The :doc:`readability-redundant-string-init
   ` check now supports a
   `StringNames` option enabling its application to custom string classes.
Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
===

[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG24aafcadff38: [clang-tidy] modernize-use-equals-default 
avoid adding redundant semicolons (authored by mitchell-stellar).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70144/new/

https://reviews.llvm.org/D70144

Files:
  clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
  clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
  clang-tools-extra/clang-tidy/utils/LexerUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default.cpp
@@ -7,7 +7,7 @@
   ~OL();
 };
 
-OL::OL() {}
+OL::OL() {};
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use '= default' to define a trivial default constructor [modernize-use-equals-default]
 // CHECK-FIXES: OL::OL() = default;
 OL::~OL() {}
@@ -17,9 +17,9 @@
 // Inline definitions.
 class IL {
 public:
-  IL() {}
+  IL() {} 	 ; // Note embedded tab on this line
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
-  // CHECK-FIXES: IL() = default;
+  // CHECK-FIXES: IL() = default 	 ; // Note embedded tab on this line
   ~IL() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: ~IL() = default;
@@ -46,18 +46,20 @@
 // Default member initializer
 class DMI {
 public:
-  DMI() {}
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
-  // CHECK-FIXES: DMI() = default;
+  DMI() {} // Comment before semi-colon on next line
+  ;
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use '= default'
+  // CHECK-FIXES: DMI() = default // Comment before semi-colon on next line
+  // CHECK-FIXES-NEXT:   ;
   int Field = 5;
 };
 
 // Class member
 class CM {
 public:
-  CM() {}
+  CM() {} /* Comments */ /* before */ /* semicolon */;
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
-  // CHECK-FIXES: CM() = default;
+  // CHECK-FIXES: CM() = default /* Comments */ /* before */ /* semicolon */;
   OL o;
 };
 
@@ -66,7 +68,7 @@
   Priv() {}
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: Priv() = default;
-  ~Priv() {}
+  ~Priv() {};
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use '= default'
   // CHECK-FIXES: ~Priv() = default;
 };
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-equals-default-copy.cpp
@@ -119,7 +119,7 @@
 struct BF {
   BF() = default;
   BF(const BF &Other) : Field1(Other.Field1), Field2(Other.Field2), Field3(Other.Field3),
-Field4(Other.Field4) {}
+Field4(Other.Field4) {};
   // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: use '= default'
   // CHECK-FIXES: BF(const BF &Other) {{$}}
   // CHECK-FIXES: = default;
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -158,6 +158,10 @@
 - The 'objc-avoid-spinlock' check was renamed to :doc:`darwin-avoid-spinlock
   `
 
+- The :doc:`modernize-use-equals-default
+  ` fix no longer adds
+  semicolons where they would be redundant.
+
 - New :doc:`readability-redundant-access-specifiers
   ` check.
 
Index: clang-tools-extra/clang-tidy/utils/LexerUtils.h
===
--- clang-tools-extra/clang-tidy/utils/LexerUtils.h
+++ clang-tools-extra/clang-tidy/utils/LexerUtils.h
@@ -80,6 +80,11 @@
   }
 }
 
+// Finds next token that's not a comment.
+Optional findNextTokenSkippingComments(SourceLocation Start,
+  const SourceManager &SM,
+  const LangOptions &LangOpts);
+
 /// Re-lex the provide \p Range and return \c false if either a macro spans
 /// multiple tokens, a pre-processor directive or failure to retrieve the
 /// next token is found, otherwise \c true.
Index: clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
@@ -68,6 +68,16 @@
   return findNextAnyTokenKind(Start, SM, LangOpts, tok::comma, tok::semi);
 }
 
+Optional findNextTokenSkippingComments(SourceLocati

[PATCH] D68346: [clang-format] Add new option to add spaces around conditions

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG26748a321e20: [clang-format] Add new option to add spaces 
around conditions Summary: This… (authored by mitchell-stellar).

Changed prior to commit:
  https://reviews.llvm.org/D68346?vs=225345&id=231930#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68346/new/

https://reviews.llvm.org/D68346

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12555,6 +12555,7 @@
   CHECK_PARSE_BOOL(SpacesInParentheses);
   CHECK_PARSE_BOOL(SpacesInSquareBrackets);
   CHECK_PARSE_BOOL(SpacesInAngles);
+  CHECK_PARSE_BOOL(SpacesInConditionalStatement);
   CHECK_PARSE_BOOL(SpaceInEmptyBlock);
   CHECK_PARSE_BOOL(SpaceInEmptyParentheses);
   CHECK_PARSE_BOOL(SpacesInContainerLiterals);
@@ -14880,6 +14881,22 @@
   verifyFormat("auto lambda = [&a = a]() { a = 2; };", AlignStyle);
 }
 
+ TEST_F(FormatTest, SpacesInConditionalStatement) {
+  FormatStyle Spaces = getLLVMStyle();
+  Spaces.SpacesInConditionalStatement = true;
+  verifyFormat("for ( int i = 0; i; i++ )\n  continue;", Spaces);
+  verifyFormat("if ( !a )\n  return;", Spaces);
+  verifyFormat("if ( a )\n  return;", Spaces);
+  verifyFormat("if constexpr ( a )\n  return;", Spaces);
+  verifyFormat("switch ( a )\ncase 1:\n  return;", Spaces);
+  verifyFormat("while ( a )\n  return;", Spaces);
+  verifyFormat("while ( (a && b) )\n  return;", Spaces);
+  verifyFormat("do {\n} while ( 1 != 0 );", Spaces);
+  // Check that space on the left of "::" is inserted as expected at beginning
+  // of condition.
+  verifyFormat("while ( ::func() )\n  return;", Spaces);
+}
+
 TEST_F(FormatTest, AlternativeOperators) {
   // Test case for ensuring alternate operators are not
   // combined with their right most neighbour.
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2592,6 +2592,13 @@
   Right.ParameterCount > 0);
 }
 
+/// Returns \c true if the token is followed by a boolean condition, \c false
+/// otherwise.
+static bool isKeywordWithCondition(const FormatToken &Tok) {
+  return Tok.isOneOf(tok::kw_if, tok::kw_for, tok::kw_while, tok::kw_switch,
+ tok::kw_constexpr);
+};
+
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
   const FormatToken &Left,
   const FormatToken &Right) {
@@ -2610,6 +2617,15 @@
   (Left.is(tok::l_brace) && Left.BlockKind != BK_Block &&
Right.is(tok::r_brace) && Right.BlockKind != BK_Block))
 return Style.SpaceInEmptyParentheses;
+  if (Style.SpacesInConditionalStatement) {
+if (Left.is(tok::l_paren) && Left.Previous &&
+isKeywordWithCondition(*Left.Previous))
+  return true;
+if (Right.is(tok::r_paren) && Right.MatchingParen &&
+Right.MatchingParen->Previous &&
+isKeywordWithCondition(*Right.MatchingParen->Previous))
+  return true;
+  }
   if (Left.is(tok::l_paren) || Right.is(tok::r_paren))
 return (Right.is(TT_CastRParen) ||
 (Left.MatchingParen && Left.MatchingParen->is(TT_CastRParen)))
@@ -3044,7 +3060,8 @@
 // The identifier might actually be a macro name such as ALWAYS_INLINE. If
 // this turns out to be too lenient, add analysis of the identifier itself.
 return Right.WhitespaceRange.getBegin() != Right.WhitespaceRange.getEnd();
-  if (Right.is(tok::coloncolon) && !Left.isOneOf(tok::l_brace, tok::comment))
+  if (Right.is(tok::coloncolon) &&
+  !Left.isOneOf(tok::l_brace, tok::comment, tok::l_paren))
 return (Left.is(TT_TemplateOpener) &&
 Style.Standard < FormatStyle::LS_Cpp11) ||
!(Left.isOneOf(tok::l_paren, tok::r_paren, tok::l_square,
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -537,6 +537,8 @@
 IO.mapOptional("SpacesBeforeTrailingComments",
Style.SpacesBeforeTrailingComments);
 IO.mapOptional("SpacesInAngles", Style.SpacesInAngles);
+IO.mapOptional("SpacesInConditionalStatement",
+   Style.SpacesInConditionalStatement);
 IO.mapOptional("SpacesInContainerLiterals",
Style.SpacesInContainerLiterals);
 IO.mapOptional("SpacesInCStyleCastParentheses",
@@ -817,6 +819,7 @@
   LLVMStyle.SpaceBeforeCpp11BracedList = false;
   LLVMStyle.SpaceBeforeSquareBrackets = false;
   LLVMStyle.Space

[PATCH] D71659: [clang-format] Added new option to allow setting spaces before and after the operator

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar requested changes to this revision.
mitchell-stellar added a comment.
This revision now requires changes to proceed.

This feature is missing unit tests. Also, what is the reason for all the 
comment changes? I don't think the changed comment lines originally exceeded 80 
characters.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71659/new/

https://reviews.llvm.org/D71659



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71769: [clang-format] C# formatting a class with inheritance followed by an attribute specifier assume its a braces initializer

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71769/new/

https://reviews.llvm.org/D71769



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

It's not more approval that is needed, it's just that someone with commit 
access (assuming you do not) needs to find the time to commit this. For what 
it's worth, I'm getting a patch rejection for the 4th block in 
lib/Format/Format.cpp. It seems the contents of `LoadConfigFile` need to be 
updated to reflect the most recent changes, so please rebase against master 
when you can.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72326/new/

https://reviews.llvm.org/D72326



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2eff1c3ce48e: [clang-format] Extend 
AllowShortLoopsOnASingleLine to do ... while loops. (authored by 
mitchell-stellar).

Changed prior to commit:
  https://reviews.llvm.org/D75022?vs=246512&id=248748#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75022/new/

https://reviews.llvm.org/D75022

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -556,6 +556,30 @@
   verifyFormat("for (;;) /* still don't merge */\n"
"  continue;",
AllowsMergedLoops);
+  verifyFormat("do a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do /* Don't merge */\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do // Don't merge\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do\n"
+   "  // Don't merge\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  // Without braces labels are interpreted differently.
+  verifyFormat("{\n"
+   "  do\n"
+   "  label:\n"
+   "a++;\n"
+   "  while (true);\n"
+   "}",
+   AllowsMergedLoops);
 }
 
 TEST_F(FormatTest, FormatShortBracedStatements) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -411,7 +411,7 @@
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
 }
-if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) {
+if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) {
   return Style.AllowShortLoopsOnASingleLine
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
@@ -514,7 +514,10 @@
   return 0;
 Limit = limitConsideringMacros(I + 1, E, Limit);
 AnnotatedLine &Line = **I;
-if (Line.Last->isNot(tok::r_paren))
+if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren))
+  return 0;
+// Only merge do while if do is the only statement on the line.
+if (Line.First->is(tok::kw_do) && !Line.Last->is(tok::kw_do))
   return 0;
 if (1 + I[1]->Last->TotalLength > Limit)
   return 0;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -556,6 +556,30 @@
   verifyFormat("for (;;) /* still don't merge */\n"
"  continue;",
AllowsMergedLoops);
+  verifyFormat("do a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do /* Don't merge */\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do // Don't merge\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  verifyFormat("do\n"
+   "  // Don't merge\n"
+   "  a++;\n"
+   "while (true);",
+   AllowsMergedLoops);
+  // Without braces labels are interpreted differently.
+  verifyFormat("{\n"
+   "  do\n"
+   "  label:\n"
+   "a++;\n"
+   "  while (true);\n"
+   "}",
+   AllowsMergedLoops);
 }
 
 TEST_F(FormatTest, FormatShortBracedStatements) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -411,7 +411,7 @@
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
 }
-if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) {
+if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) {
   return Style.AllowShortLoopsOnASingleLine
  ? tryMergeSimpleControlStatement(I, E, Limit)
  : 0;
@@ -514,7 +514,10 @@
   return 0;
 Limit = limitConsideringMacros(I + 1, E, Limit);
 AnnotatedLine &Line = **I;
-if (Line.Last->isNot(tok::r_paren))
+if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren))
+  return 0;
+// Only merge do while if do is the only statement on the line.
+if (Line.First->is(tok::kw_do) && !Line.Last->is(tok::kw_do))
   return 0;
 if (1 + I[1]->Last->TotalL

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

[Mitchell via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG10b1a87ba35d: [clang-format] Add option to specify explicit 
config file Summary: This diff… (authored by mitchell-stellar).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72326/new/

https://reviews.llvm.org/D72326

Files:
  clang/docs/ClangFormat.rst
  clang/docs/ClangFormatStyleOptions.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -14912,6 +14912,61 @@
   auto StyleTd = getStyle("file", "x.td", "llvm", "", &FS);
   ASSERT_TRUE((bool)StyleTd);
   ASSERT_EQ(*StyleTd, getLLVMStyle(FormatStyle::LK_TableGen));
+
+  // Test 9: explicit format file in parent directory.
+  ASSERT_TRUE(
+  FS.addFile("/e/.clang-format", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: LLVM")));
+  ASSERT_TRUE(
+  FS.addFile("/e/explicit.clang-format", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
+  ASSERT_TRUE(FS.addFile("/e/sub/sub/sub/test.cpp", 0,
+ llvm::MemoryBuffer::getMemBuffer("int i;")));
+  auto Style8 = getStyle("file:/e/explicit.clang-format",
+ "/e/sub/sub/sub/test.cpp", "LLVM", "", &FS);
+  ASSERT_TRUE((bool)Style8);
+  ASSERT_EQ(*Style8, getGoogleStyle());
+
+  // Test 10: relative pah to a format file
+  ASSERT_TRUE(
+  FS.addFile("../../e/explicit.clang-format", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
+  auto Style9 = getStyle("file:../../e/explicit.clang-format",
+ "/e/sub/sub/sub/test.cpp", "LLVM", "", &FS);
+  ASSERT_TRUE((bool)Style9);
+  ASSERT_EQ(*Style9, getGoogleStyle());
+
+  // Test 11: missing explicit format file
+  auto Style10 = getStyle("file:/e/missing.clang-format",
+  "/e/sub/sub/sub/test.cpp", "LLVM", "", &FS);
+  ASSERT_FALSE((bool)Style10);
+  llvm::consumeError(Style10.takeError());
+
+  // Test 12: format file from the filesystem
+  SmallString<128> FormatFilePath;
+  std::error_code ECF = llvm::sys::fs::createTemporaryFile(
+  "FormatFileTest", "tpl", FormatFilePath);
+  EXPECT_FALSE((bool)ECF);
+  llvm::raw_fd_ostream FormatFileTest(FormatFilePath, ECF);
+  EXPECT_FALSE((bool)ECF);
+  FormatFileTest << "BasedOnStyle: Google\n";
+  FormatFileTest.close();
+
+  SmallString<128> TestFilePath;
+  std::error_code ECT =
+  llvm::sys::fs::createTemporaryFile("CodeFileTest", "cc", TestFilePath);
+  EXPECT_FALSE((bool)ECT);
+  llvm::raw_fd_ostream CodeFileTest(TestFilePath, ECT);
+  CodeFileTest << "int i;\n";
+  CodeFileTest.close();
+
+  std::string format_file_arg = std::string("file:") + FormatFilePath.c_str();
+  auto Style11 = getStyle(format_file_arg, TestFilePath, "LLVM", "", nullptr);
+
+  llvm::sys::fs::remove(FormatFilePath.c_str());
+  llvm::sys::fs::remove(TestFilePath.c_str());
+  ASSERT_TRUE((bool)Style11);
+  ASSERT_EQ(*Style11, getGoogleStyle());
 }
 
 TEST_F(ReplacementTest, FormatCodeAfterReplacements) {
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2640,6 +2640,8 @@
 ".clang-format file located in one of the parent\n"
 "directories of the source file (or current\n"
 "directory for stdin).\n"
+"Use -style=file: to explicitly specify"
+"the configuration file.\n"
 "Use -style=\"{key: value, ...}\" to set specific\n"
 "parameters, e.g.:\n"
 "  -style=\"{BasedOnStyle: llvm, IndentWidth: 8}\"";
@@ -2685,6 +2687,29 @@
   return GuessedLanguage;
 }
 
+/// Attempts to load a format file
+llvm::Expected LoadConfigFile(StringRef ConfigFile,
+   llvm::vfs::FileSystem *FS,
+   bool *IsSuitable) {
+  FormatStyle Style = getLLVMStyle();
+  *IsSuitable = true;
+
+  llvm::ErrorOr> Text =
+  FS->getBufferForFile(ConfigFile.str());
+  if (std::error_code EC = Text.getError())
+return make_string_error(EC.message());
+  std::error_code ParserErrorCode =
+  parseConfiguration(Text.get()->getBuffer(), &Style);
+  if (ParserErrorCode == ParseError::Unsuitable) {
+*IsSuitable = false;
+return Style;
+  } else if (ParserErrorCode != ParseError::Success) {
+return make_string_error("Error reading " + ConfigFile + ": " +
+ ParserErrorCode.message());
+  }
+  return Style;
+}
+
 const char *DefaultFormatStyle = "file";
 
 const char *DefaultFallbackStyle = "LLVM";
@@ -2709,6 +2734,21 @@
 return Style;
   }
 
+  llvm::SmallVector FilesToLookFor;
+  // User provided clang-format f

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar reopened this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

Reverted this commit due to an unexpected test failure:

   TEST 'Clang :: Format/dump-config-objc.h' FAILED 

  Script:
  --
  : 'RUN: at line 1';   /b/1/clang-x86_64-debian-fast/llvm.obj/bin/clang-format 
-dump-config 
/b/1/clang-x86_64-debian-fast/llvm.src/clang/test/Format/dump-config-objc.h | 
/b/1/clang-x86_64-debian-fast/llvm.obj/bin/FileCheck 
/b/1/clang-x86_64-debian-fast/llvm.src/clang/test/Format/dump-config-objc.h
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  
/b/1/clang-x86_64-debian-fast/llvm.src/clang/test/Format/dump-config-objc.h:3:11:
 error: CHECK: expected string not found in input
  // CHECK: Language: ObjC
^
  :1:1: note: scanning from here
  ---
  ^
  :2:1: note: possible intended match here
  Language: Cpp
  ^
  
  --
  
  

I don't know enough about this patch in order to determine what the issue is, 
or how to proceed further. Perhaps @MyDeveloperDay will chime in.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72326/new/

https://reviews.llvm.org/D72326



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134042: [clang-format] Fix alignment in #else preprocessor blocks

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

After reviewing the GitHub issue, it looks like the IndentAndNestingLevel is 
sticky over the entire scope for `#else` blocks, so the regression only occurs 
within `#else` blocks in the same scope. The bug I fixed affected alignment in 
all `#else` blocks, regardless of scope.

I believe that if you choose to revert this @owenpan, you will see more bad 
alignments throughout `#else` blocks in an entire file. If you keep the fix, 
you will see bad alignment in `#else` blocks within the current scope. Pick 
your poison I guess. I personally prefer better alignment in separate `#else` 
blocks over the entire file, which is what prompted my bugfix in the first 
place. On those grounds I object to reverting the fix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134042/new/

https://reviews.llvm.org/D134042

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134042: [clang-format] Fix alignment in #else preprocessor blocks

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar added a comment.

I retract my objection. I forgot that from clang-format's perspective, `#else` 
blocks in separate functions have the same indent and nesting level, and thus 
the same "scope", so the alignment issues may persist there. You will see 
misalignment in `#else` blocks over an entire source file whether you keep or 
revert this fix (i.e. pick your poison.) I suppose based on precedent we keep 
the misalignments I wanted to fix since I introduced misalignments in other 
cases.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D134042/new/

https://reviews.llvm.org/D134042

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150057: [clang-format] Fix consecutive alignments in #else blocks

[Mitchell via Phabricator via cfe-commits]

mitchell-stellar accepted this revision.
mitchell-stellar added a comment.
This revision is now accepted and ready to land.

LGTM. Well done!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150057/new/

https://reviews.llvm.org/D150057

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123045: [clang][extract-api] Fix small issues with SymbolGraphSerializer

[Victoria Mitchell via Phabricator via cfe-commits]

QuietMisdreavus added a comment.

After a quick scan comparing the current output of these symbol graphs with the 
primary library used for reading them 
, the last thing i can spot 
that's "off" is that the "function signature" is currently being serialized 
under a `parameters` field instead of the required `functionSignature`.




Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:510
   Symbols.emplace_back(std::move(*Obj));
+  PathComponentContext.pop_back();
 }

zixuw wrote:
> What's the cost/would it worth it to wrap the `emplace_back`s and `pop_back`s 
> of `PathComponentContext` in meaningful APIs like `enterPathComponentContext` 
> and `exitPathComponentContext`?
> That way the code is more self-explanatory and easier to read. It took me 
> some time and mental resources to figure out why the `pop_back` is placed 
> here.
What's the use of having the `emplace_back` call inside `serializeAPIRecord` 
but to pop it outside? It seems like it's easier to mess up for new kinds of 
records.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123045/new/

https://reviews.llvm.org/D123045

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123045: [clang][extract-api] Fix small issues with SymbolGraphSerializer

[Victoria Mitchell via Phabricator via cfe-commits]

QuietMisdreavus added a comment.

In D123045#3427992 , @zixuw wrote:

> In D123045#3427699 , 
> @QuietMisdreavus wrote:
>
>> After a quick scan comparing the current output of these symbol graphs with 
>> the primary library used for reading them 
>> , the last thing i can spot 
>> that's "off" is that the "function signature" is currently being serialized 
>> under a `parameters` field instead of the required `functionSignature`.
>
> Hmm, I was looking at the format specification at 
> https://github.com/apple/swift-docc-symbolkit/blob/0a45209833f4a151212c1aa38e13cfc03b9462e4/openapi.yaml#L307,
>  and I searched the term `functionSignature` in the spec but found no 
> property with that name (except for the `FunctionSignature` schema that the 
> `parameters` property is referring to). But anyway this should be a easy fix.

It seems like the specification and implementation have diverged. The parser in 
swift-docc-symbolkit is looking for a `functionSignature` field by virtue of 
how it names its "coding key" 
.
 By comparison, here is the functionSignature emitter in the Swift symbol-graph 
generator 
.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123045/new/

https://reviews.llvm.org/D123045

STAMPS
actor(@QuietMisdreavus) application(Differential) author(@dang) herald(H423) 
herald(H576) herald(H864) herald(H875) herald(H876) mention(@QuietMisdreavus) 
mention(@zixuw) monogram(D123045) object-type(DREV) 
phid(PHID-DREV-d5goxxqici5xa3w2bgjy) reviewer(@QuietMisdreavus) 
reviewer(@ributzka) reviewer(@zixuw) revision-repository(rG) 
revision-status(needs-review) subscriber(@cfe-commits) tag(#all) tag(#clang) 
via(web)

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123045: [clang][extract-api] Fix small issues with SymbolGraphSerializer

[Victoria Mitchell via Phabricator via cfe-commits]

QuietMisdreavus added inline comments.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:510
   Symbols.emplace_back(std::move(*Obj));
+  PathComponentContext.pop_back();
 }

dang wrote:
> dang wrote:
> > zixuw wrote:
> > > zixuw wrote:
> > > > QuietMisdreavus wrote:
> > > > > zixuw wrote:
> > > > > > What's the cost/would it worth it to wrap the `emplace_back`s and 
> > > > > > `pop_back`s of `PathComponentContext` in meaningful APIs like 
> > > > > > `enterPathComponentContext` and `exitPathComponentContext`?
> > > > > > That way the code is more self-explanatory and easier to read. It 
> > > > > > took me some time and mental resources to figure out why the 
> > > > > > `pop_back` is placed here.
> > > > > What's the use of having the `emplace_back` call inside 
> > > > > `serializeAPIRecord` but to pop it outside? It seems like it's easier 
> > > > > to mess up for new kinds of records.
> > > > The reason to `emplace_back` the path component inside 
> > > > `serializeAPIRecord` is that the `pathComponent` field of the `Record` 
> > > > is serialized in there so you want to include the name of the symbol 
> > > > itself in the path component list.
> > > > The `pop_back` is delayed til all other additional serialization for a 
> > > > specific kind of record, for example `serializeEnumRecord` handles all 
> > > > the enum cases after processing the enum record itself using 
> > > > `serializeAPIRecord`. So in order to correctly include the enum name in 
> > > > the path components of all the enum cases, the enum name has to stay in 
> > > > `PathComponentContext` until all members are serialized.
> > > > 
> > > > This is exactly the reason why I wanted a clearer API to make it easier 
> > > > to see.
> > > Hmm now that I thought about this, it seems that it would be easier to 
> > > understand and avoid bugs if we lift 
> > > `PathComponentContext.emplace_back`/`enterPathComponentContext` out of 
> > > `serializeAPIRecord`, because we have access to the record name before 
> > > that anyway.
> > > 
> > > So we establish a pre-condition of `serializeAPIRecord` that the correct 
> > > path components would be set up in `PathComponentContext` before the call 
> > > so we could still serialize the field inside that method. And in specific 
> > > `serialize*Record` methods we push the record name, and pop out at the 
> > > end.
> > > 
> > > This way the push and pop operations would appear in pairs in a single 
> > > block, saving the confusion and mental work of jumping across functions 
> > > to see how `PathComponentContext` is evolving.
> > If you think we should have a specialized API I am happy to do this. I 
> > figured it was self-explanatory by the name of `PathComponentContext` but 
> > it clearly isn't so needs addressing. I put the `emplace_back` call in 
> > `serializeAPIRecord` since all the specific serialization methods call it. 
> > I thought it would make it impossible to forget to add them. @zixuw is 
> > correct in the reason why the pop is outside we don't want to pop before we 
> > have serialized the sub records by agree it is ugly and potentially error 
> > prone. I can see three ways forward for improving the ergonomics of this 
> > since this seems to be problematic:
> > - Provide `serializeAPIRecord` a continuation to serialize the sub records 
> > or additional fields, that way we can move the pop inside 
> > `serializeAPIRecord` I am not a fan personally because we get into JS style 
> > closure hell if we start doing this...
> > - Use a visitor pattern where the visitor would be responsible for managing 
> > `PathComponentContext` and do the appropriate push/pops in the `visit` 
> > methods. We would need to add a APIRecordVisitor type, and appropriate 
> > visit methods for each APIRecord. This would make sense because the records 
> > should really know how to visit themselves.
> > - Just add a specialized API although it seems it would really easy to 
> > forget to remove path components.
> > 
> > Let me know what you think is the way forward.
> Unless we go with the last option, I think this should be a follow up patch 
> since it would a structural change.
I'm most a fan of the `APIRecordVisitor` option, but you're right in that that 
would be a rather involved change. Now that you've said that the arrangement is 
for encoding sub-records, it makes sense to me. As an alternative, i think 
moving the `emplace_back` outside of `serializeAPIRecord` is actually my other 
preferred arrangement; without some other way of calculating the path 
components on-the-fly (e.g. walking up DeclContexts) simply having 
`serializeAPIRecord` look at what's there and have the wrapper calls deal with 
setting up its state sounds the most reasonable to me. That way, as @zixuw 
said, the `emplace_back` and `pop_back` calls are at least lexically close to 
each other.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  http

[PATCH] D123045: [clang][extract-api] Fix small issues with SymbolGraphSerializer

[Victoria Mitchell via Phabricator via cfe-commits]

QuietMisdreavus accepted this revision.
QuietMisdreavus added a comment.
This revision is now accepted and ready to land.

I like the idea of using the RAII context/guard to manage the path components 
stack. I have one non-blocking comment about the rest of the patch now.




Comment at: 
clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h:125
   /// containing common symbol information of \p Record.
-  Optional serializeAPIRecord(const APIRecord &Record) const;
+  Optional serializeAPIRecord(const APIRecord &Record);
 

Now that the path components are being manipulated outside of 
`serializeAPIRecord`, can this function become `const` again?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123045/new/

https://reviews.llvm.org/D123045

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123261: [clang][ExtractAPI] Fix declaration fragments for ObjC methods

[Victoria Mitchell via Phabricator via cfe-commits]

QuietMisdreavus accepted this revision.
QuietMisdreavus added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123261/new/

https://reviews.llvm.org/D123261

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123259: [clang][ExtractAPI] Fix appendSpace in DeclarationFragments

[Victoria Mitchell via Phabricator via cfe-commits]

QuietMisdreavus accepted this revision.
QuietMisdreavus added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123259/new/

https://reviews.llvm.org/D123259

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123391: [clang][extract-api] Emit "navigator" property of "name" in SymbolGraph

[Victoria Mitchell via Phabricator via cfe-commits]

QuietMisdreavus added a comment.

This looks good! Just one question.




Comment at: clang/test/ExtractAPI/objc_interface.m:198
+"kind": "identifier",
+"spelling": "getWithProperty:"
+  }

I'm curious: Does this properly handle Objective-C methods with multiple 
arguments? i.e. if there were a method `setProperty:andOtherProperty:`, would 
that be the navigator name?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123391/new/

https://reviews.llvm.org/D123391

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123391: [clang][extract-api] Emit "navigator" property of "name" in SymbolGraph

[Victoria Mitchell via Phabricator via cfe-commits]

QuietMisdreavus accepted this revision.
QuietMisdreavus added a comment.
This revision is now accepted and ready to land.

Looks great, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123391/new/

https://reviews.llvm.org/D123391

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125678: [clang][extract-api] Don't emit symbols prefixed with an underscore

[Victoria Mitchell via Phabricator via cfe-commits]

QuietMisdreavus accepted this revision.
QuietMisdreavus added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125678/new/

https://reviews.llvm.org/D125678

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D125678: [clang][extract-api] Don't emit symbols prefixed with an underscore

[Victoria Mitchell via Phabricator via cfe-commits]

QuietMisdreavus added a comment.

clang-format failed:

  ---  clang-format
  
  changed files:
  
  clang/test/ExtractAPI/underscored.c


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D125678/new/

https://reviews.llvm.org/D125678

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135804: [clang][ExtractAPI] Ignore fully anonymous RecordDecls

[Victoria Mitchell via Phabricator via cfe-commits]

QuietMisdreavus accepted this revision.
QuietMisdreavus added a comment.
This revision is now accepted and ready to land.

It would be ideal to be able to properly handle nested types like this, but for 
right now this is causing a crash in Swift-DocC, so this patch will at least 
get that working.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135804/new/

https://reviews.llvm.org/D135804

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D117809: [clang] Add an extract-api driver option

[Victoria Mitchell via Phabricator via cfe-commits]

QuietMisdreavus added inline comments.



Comment at: clang/include/clang/Driver/Types.def:103
 TYPE("hip-fatbin",   HIP_FATBIN,   INVALID, "hipfb",  
phases::Compile, phases::Backend, phases::Assemble, phases::Link)
+TYPE("api-information",  API_INFO, INVALID, "json",   
phases::Compile)
 TYPE("none", Nothing,  INVALID, nullptr,  
phases::Compile, phases::Backend, phases::Assemble, phases::Link)

Symbol graph files generally have the extension `.symbols.json` - is that 
something that should be reflected here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117809/new/

https://reviews.llvm.org/D117809

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits