[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

[Owen Pan via Phabricator via cfe-commits]

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

LGTM except the final nits.




Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:48
+const FormatToken *BeginTok, const FormatToken *EndTok) const {
+  // If there are zero or one tokens, nothing to do.
+  if (BeginTok == EndTok || BeginTok->Next == EndTok)





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:55
+  for (auto Tok = BeginTok; Tok != EndTok; Tok = Tok->Next) {
+if (Tok->is(tok::comma)) {
+  // Ignore the comma separators.





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:71
+// BinaryOperator or Identifier)
+if (Tok->Next->is(tok::equal)) {
+  Tok = Tok->Next;





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:73
+  Tok = Tok->Next;
+  if (Tok->Next->isNot(tok::identifier)) {
+// If we hit any other kind of token, just bail. It's unusual/illegal.





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:82-84
+  // There's no need to sort unless there's more than one thing.
+  if (PropertyAttributes.size() < 2)
+return;

Delete or assert instead.



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:103
+  // If there are no removals or shuffling, then don't suggest any fixup.
+  if (Indices.size() == PropertyAttributes.size() && llvm::is_sorted(Indices))
+return;

You can assert the equality of the sizes before the `if` if you like.



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:136
+tooling::Replacements , const FormatToken *Tok) const {
+  // Expect `property` to be the very next token or else just bail early.
+  const FormatToken *const PropertyTok = Tok->Next;





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:165
+  for (AnnotatedLine *Line : AnnotatedLines) {
+if (!Line->Affected || Line->Type != LT_ObjCProperty)
+  continue;





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:175
+for (const auto *Tok = First; Tok != Last; Tok = Tok->Next) {
+  // Skip until the `@` of a `@property` declaration.
+  if (Tok->isNot(TT_ObjCProperty))





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:97
+  }
+
+  // Create a "remapping index" on how to reorder the attributes.

jaredgrubb wrote:
> owenpan wrote:
> > Can we assert `PropertyAttributes.size() > 1` here?
> We shouldn't _assert_, as it is valid to have just one. But I can add an 
> early-return for that though. (I'll also early-return if it's zero, which is 
> also legal, eg `@property () int x`)
Now we can.  



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:184
+  for (AnnotatedLine *Line : AnnotatedLines) {
+if (!Line->Affected || Line->InPPDirective)
+  continue;

jaredgrubb wrote:
> owenpan wrote:
> > jaredgrubb wrote:
> > > owenpan wrote:
> > > > Why not `InPPDirective`?
> > > I copy-pasted this from `LeftRightQualifierAlignmentFixer::analyze`, 
> > > which I used as a template since I'm still getting used to the codebase. 
> > > I wasn't sure whether this was important, so I left it in. But I don't 
> > > think I have a good reason. 
> > > 
> > > I've added a new test case `SortsInPPDirective` that spot-checks some 
> > > macro definition examples (which did fail unless this 
> > > `Line->InPPDirective` check was removed, as expected.).
> > What about `Line->Type != LT_ObjCProperty` I suggested?
> Ah, I missed that suggestion. 
> 
> I don't have enough understanding of clang-format to say whether that would 
> be correct, but I'll trust you :) 
> 
> When I add this check it doesn't break unit tests or cause any 
> change-in-behavior on my ObjC test repo (~10K .h/.m files). So, it -seems- 
> like this is a valid check to add, so I'll do it!
> When I add this check it doesn't break unit tests or cause any 
> change-in-behavior on my ObjC test repo (~10K .h/.m files).

If it does, then we probably have a bug in setting `LT_ObjCProperty`.  


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

https://reviews.llvm.org/D150083

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

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

BTW, it may be simpler and more efficient to use a set (e.g. `llvm::SmallSet`) 
for `Indices`, especially if we don't need/want to handle duplicate attributes 
that have a value. (See D150083#inline-1551778 
.)


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

https://reviews.llvm.org/D150083

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

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

Thank you for your patience! There are still a few comments not done from the 
previous round.

In D150083#4655528 , @owenpan wrote:

> See also D153228 .

Would this patch have a similar performance issue?




Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:73
+  Tok = Tok->Next;
+  if (!Tok->Next->is(tok::identifier)) {
+// If we hit any other kind of token, just bail. It's unusual/illegal.





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:93
+  const auto SortOrderMax = Style.ObjCPropertyAttributeOrder.size();
+  const auto SortIndex = [&](const StringRef ) -> unsigned {
+auto I = SortOrderMap.find(Needle);





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:143
+const SourceManager , const AdditionalKeywords ,
+tooling::Replacements , const FormatToken *const Tok) const {
+  // Expect `property` to be the very next token or else just bail early.





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:185
+
+for (const auto *Tok = First; Tok && Tok != Last && Tok->Next;
+ Tok = Tok->Next) {





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:113
+
+  // Deduplicate the attributes.
+  Indices.erase(std::unique(Indices.begin(), Indices.end(),

jaredgrubb wrote:
> owenpan wrote:
> > Is it valid in Objective-C to have duplicate attributes?
> It's silly, but clang doesn't seem to care. I tried this, so duplicate was 
> ok, but contradiction was flagged:
> ```
> @property (strong, nonatomic, nonatomic) X *X;   // duplicate, but no error
> @property (strong, nonatomic, weak) Y *y;   // error: Property attributes 
> 'strong' and 'weak' are mutually exclusive
> ```
> 
> I wasn't sure whether to do this, but went that way since that's what "sort 
> include files" did. However, it seems like an odd corner case so I'd be ok 
> removing this uniquing if you prefer.
We should keep the duplicates if clang accepts them. What happens to e.g. 
`@property(x=a, y=b, x=a, y=c)`?



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:184
+  for (AnnotatedLine *Line : AnnotatedLines) {
+if (!Line->Affected || Line->InPPDirective)
+  continue;

jaredgrubb wrote:
> owenpan wrote:
> > Why not `InPPDirective`?
> I copy-pasted this from `LeftRightQualifierAlignmentFixer::analyze`, which I 
> used as a template since I'm still getting used to the codebase. I wasn't 
> sure whether this was important, so I left it in. But I don't think I have a 
> good reason. 
> 
> I've added a new test case `SortsInPPDirective` that spot-checks some macro 
> definition examples (which did fail unless this `Line->InPPDirective` check 
> was removed, as expected.).
What about `Line->Type != LT_ObjCProperty` I suggested?



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:190
+  continue;
+
+const auto *Last = Line->Last;

jaredgrubb wrote:
> owenpan wrote:
> > Must `@property` be the first non-comment tokens of an unwrapped line in 
> > Objective-C? And at most one `@property` per line?
> I can't think of any token that should precede a `@property`. Aka, I don't 
> think there are any `__attribute__` that can fill that slot. 
> 
> You could have `@optional` or `@required` floating around between 
> property/method declarations. I've added a test-case for a `@property` that 
> is preceded by these tokens and proved that the reordering is handled 
> correctly.
> 
> As for multiple properties in one line, I've never seen a codebase with that. 
> clang-format does split them already.
I asked the questions because if yes, we could move on to the next line before 
hitting the last token of the current line.


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

https://reviews.llvm.org/D150083

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

[PATCH] D156370: [clang-format] Fix bug with parsing of function/variable names.

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa852869398af: [clang-format] Fix a bug in parsing 
function/variable names (authored by gedare, committed by owenpan).

Changed prior to commit:
  https://reviews.llvm.org/D156370?vs=558061=558088#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156370

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


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2313,6 +2313,7 @@
 TEST_F(TokenAnnotatorTest, UnderstandsAttributes) {
   auto Tokens = annotate("bool foo __attribute__((unused));");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::identifier, TT_StartOfName);
   EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_AttributeLParen);
   EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_Unknown);
   EXPECT_TOKEN(Tokens[6], tok::r_paren, TT_Unknown);
@@ -2323,6 +2324,22 @@
   EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_AttributeLParen);
   EXPECT_TOKEN(Tokens[5], tok::r_paren, TT_AttributeRParen);
 
+  Tokens = annotate("bool __attribute__((unused)) foo;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_AttributeLParen);
+  EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_Unknown);
+  EXPECT_TOKEN(Tokens[5], tok::r_paren, TT_Unknown);
+  EXPECT_TOKEN(Tokens[6], tok::r_paren, TT_AttributeRParen);
+  EXPECT_TOKEN(Tokens[7], tok::identifier, TT_StartOfName);
+
+  Tokens = annotate("void __attribute__((x)) Foo();");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_AttributeLParen);
+  EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_Unknown);
+  EXPECT_TOKEN(Tokens[5], tok::r_paren, TT_Unknown);
+  EXPECT_TOKEN(Tokens[6], tok::r_paren, TT_AttributeRParen);
+  EXPECT_TOKEN(Tokens[7], tok::identifier, TT_FunctionDeclarationName);
+
   FormatStyle Style = getLLVMStyle();
   Style.AttributeMacros.push_back("FOO");
   Tokens = annotate("bool foo FOO(unused);", Style);
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16394,8 +16394,10 @@
 
   verifyFormat("int f ();", SpaceFuncDecl);
   verifyFormat("void f(int a, T b) {}", SpaceFuncDecl);
+  verifyFormat("void __attribute__((asdf)) f(int a, T b) {}", SpaceFuncDecl);
   verifyFormat("A::A() : a(1) {}", SpaceFuncDecl);
   verifyFormat("void f () __attribute__((asdf));", SpaceFuncDecl);
+  verifyFormat("void __attribute__((asdf)) f ();", SpaceFuncDecl);
   verifyFormat("#define A(x) x", SpaceFuncDecl);
   verifyFormat("#define A (x) x", SpaceFuncDecl);
   verifyFormat("#if defined(x)\n"
@@ -16429,8 +16431,10 @@
 
   verifyFormat("int f();", SpaceFuncDef);
   verifyFormat("void f (int a, T b) {}", SpaceFuncDef);
+  verifyFormat("void __attribute__((asdf)) f (int a, T b) {}", SpaceFuncDef);
   verifyFormat("A::A() : a(1) {}", SpaceFuncDef);
   verifyFormat("void f() __attribute__((asdf));", SpaceFuncDef);
+  verifyFormat("void __attribute__((asdf)) f();", SpaceFuncDef);
   verifyFormat("#define A(x) x", SpaceFuncDef);
   verifyFormat("#define A (x) x", SpaceFuncDef);
   verifyFormat("#if defined(x)\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2246,8 +2246,9 @@
  
PreviousNotConst->MatchingParen->Previous->isNot(tok::kw_template);
 }
 
-if (PreviousNotConst->is(tok::r_paren) &&
-PreviousNotConst->is(TT_TypeDeclarationParen)) {
+if ((PreviousNotConst->is(tok::r_paren) &&
+ PreviousNotConst->is(TT_TypeDeclarationParen)) ||
+PreviousNotConst->is(TT_AttributeRParen)) {
   return true;
 }
 


Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -2313,6 +2313,7 @@
 TEST_F(TokenAnnotatorTest, UnderstandsAttributes) {
   auto Tokens = annotate("bool foo __attribute__((unused));");
   ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::identifier, TT_StartOfName);
   EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_AttributeLParen);
   EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_Unknown);
   EXPECT_TOKEN(Tokens[6], tok::r_paren, TT_Unknown);
@@ -2323,6 +2324,22 @@
   EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_AttributeLParen);
   EXPECT_TOKEN(Tokens[5], tok::r_paren, TT_AttributeRParen);
 
+  Tokens = annotate("bool __attribute__((unused)) foo;");
+  

[PATCH] D155529: [clang-format] Add SpaceInParensOption for __attribute__ keyword

[Owen Pan via Phabricator via cfe-commits]

owenpan requested changes to this revision.
owenpan added a comment.
This revision now requires changes to proceed.

In D155529#4509686 , @MyDeveloperDay 
wrote:

> We should never make assumptions about what people do/don't use



> If you have this you have to honour it.. if 'SpacesInParentheses ' was true 
> then 'InAttributeSpecifiers' needs to be true by default, shouldn't there be 
> something in the expanding out of SpacesInParentheses

Because `SpacesInParentheses` was added 10+ years ago in rGb55acad91c37 
 and 
`__attribute__((foo))` was not in the unit tests, it's probably a bug that the 
double parens were not excluded. Not sure whether the users who didn't complain 
about it really wanted it or simply didn't bother. The only way to find out 
would be to fix the bug, assuming it indeed was a bug.

When fixing bugs, especially the very old ones, we often have to choose between 
just fixing the bugs and adding an option to avoid "regressions", and I usually 
prefer the former. Nevertheless, I would have no problem if this new option is 
extended to handle all double parens, e.g. `if (( i = j ))`, `decltype(( x ))`, 
etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155529

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

[PATCH] D156360: [clang-format] Support function and overloaded operator SpacesInParensOption

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3970-4009
   if (Left.is(tok::l_paren) || Right.is(tok::r_paren)) {
 if (Right.is(TT_CastRParen) ||
 (Left.MatchingParen && Left.MatchingParen->is(TT_CastRParen))) {
   return Style.SpacesInParensOptions.InCStyleCasts;
 }
 if (Left.isOneOf(TT_AttributeLParen, TT_AttributeRParen) ||
 Right.isOneOf(TT_AttributeLParen, TT_AttributeRParen) ||

Consider wrapping this in a function or lambda. Not sure if it would simply the 
logic with the parens being handled separately:
```
if (Left.is(tok::l_paren)) {
  ...
} else if (Right.is(tok::r_paren)) {
  ...
}



Comment at: clang/lib/Format/TokenAnnotator.cpp:3990
+  return Style.SpacesInParensOptions.InFunctionDefinitions;
+else
+  return Style.SpacesInParensOptions.InFunctionDeclarations;

No `else` after `return`.



Comment at: clang/unittests/Format/FormatTest.cpp:16786
   verifyFormat("SomeType *__attribute__( ( attr ) ) *a = NULL;", Spaces);
-  verifyFormat("void __attribute__( ( naked ) ) foo( int bar )", Spaces);
+  verifyFormat("void __attribute__( ( x ) ) foo(int y) { return; }", Spaces);
   verifyFormat("void f() __attribute__( ( asdf ) );", Spaces);

gedare wrote:
> HazardyKnusperkeks wrote:
> > Why change this?
> The original test is incomplete/ambiguous. It's either a declaration missing 
> a semicolon, or it's the start of a definition. I made it a definition.
Then can you also add a declaration?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156360

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

[PATCH] D154755: [clang-format] Fix formatting of if statements with BlockIndent

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:25814-25824
+   "  if (quitelongname < alsolongname ||\n"
+   "  anotherevenlongername <=\n"
+   "  thisreallyreallyreallyreallyreallyreallylongername 
||"
+   "\n"
+   "  othername < thislastname) {\n"
+   "return;\n"
+   "  } else if (quitelongname < alsolongname ||\n"

Please use camel case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154755

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

[PATCH] D154755: [clang-format] Fix formatting of if statements with BlockIndent

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:773-792
   if ((Style.AlignAfterOpenBracket == FormatStyle::BAS_AlwaysBreak ||
Style.AlignAfterOpenBracket == FormatStyle::BAS_BlockIndent) &&
   (Previous.isOneOf(tok::l_paren, TT_TemplateOpener, tok::l_square) ||
(Previous.is(tok::l_brace) && Previous.isNot(BK_Block) &&
 Style.Cpp11BracedListStyle)) &&
   State.Column > getNewLineColumn(State) &&
   (!Previous.Previous ||

This is getting out of hand. Consider writing a lambda with early returns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154755

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

[PATCH] D156370: [clang-format] Fix bug with parsing of function/variable names.

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:-2224
 if (PreviousNotConst->is(tok::r_paren) &&
-PreviousNotConst->is(TT_TypeDeclarationParen)) {
+(PreviousNotConst->isOneOf(TT_TypeDeclarationParen,
+   TT_AttributeRParen))) {

We should annotate parens separately as LParen and RParen like we recently did 
with braces. Then we could simply do:
```
if (PreviousNotConst->isOneOf(TT_TypeDeclarationRParen, TT_AttributeRParen))
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156370

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

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:184
+  for (AnnotatedLine *Line : AnnotatedLines) {
+if (!Line->Affected || Line->InPPDirective)
+  continue;

Why not `InPPDirective`?


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

https://reviews.llvm.org/D150083

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

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:104
+  // for missing values.
+  auto sortIndex = [&](const StringRef ) -> unsigned {
+auto i = SortOrderMap.find(needle);

owenpan wrote:
> owenpan wrote:
> > 
> Start names with uppercase letters except for functions.
Oops! Please ignore.


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

https://reviews.llvm.org/D150083

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

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

See also D153228 .




Comment at: clang/include/clang/Format/Format.h:3204
+  /// \warning
+  ///  Setting this option to ``true`` could lead to incorrect code formatting
+  ///  due to clang-format's lack of complete semantic information. As such,

And reflow the comment.



Comment at: clang/lib/Format/Format.cpp:3671-3675
+if (!Style.ObjCPropertyAttributeOrder.empty()) {
+  Passes.emplace_back([&](const Environment ) {
+return ObjCPropertyAttributeOrderFixer(Env, Expanded).process();
+  });
+}

Move down as it's Objective-C specific. (See below.)



Comment at: clang/lib/Format/Format.cpp:3719
   }
 
   if (Style.isJavaScript() &&





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:18
+
+#include "FormatToken.h"
+#include "llvm/ADT/Sequence.h"

Delete.



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:20-24
+#include "llvm/Support/Debug.h"
+
+#include 
+
+#define DEBUG_TYPE "format-objc-property-attribute-order-fixer"

Delete.



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:35
+  unsigned index = 0;
+  for (auto const  : Style.ObjCPropertyAttributeOrder)
+SortOrderMap[Property] = index++;





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:39
+  // end).
+  SortOrderMax = index;
+}

Delete.



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:55
+const FormatToken *LParenTok, const FormatToken *RParenTok) const {
+  // Skip past any leading comments.
+  const FormatToken *const BeginTok = LParenTok->getNextNonComment();

I strongly suggest that we bail out as well if there are leading and/or 
trailing comments.



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:75
+  continue;
+} else if (isObjCPropertyAttribute(Tok)) {
+  // Memoize the attribute. (Note that 'class' is a legal attribute!)

No `else` after `continue`.



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:77
+  // Memoize the attribute. (Note that 'class' is a legal attribute!)
+  PropertyAttributes.push_back({Tok->TokenText.trim(), StringRef{}});
+

Do we need `trim()`?



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:84-90
+if (Tok->Next->is(tok::identifier)) {
+  Tok = Tok->Next;
+  PropertyAttributes.back().Value = Tok->TokenText.trim();
+} else {
+  // If we hit any other kind of token, just bail. It's 
unusual/illegal.
+  return;
+}

Change:
```
if (cond) {
  ...
} else {
  return;
}
```
To:
```
if (!cond)
  return;
...
```



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:86
+  Tok = Tok->Next;
+  PropertyAttributes.back().Value = Tok->TokenText.trim();
+} else {

Ditto.



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:92-95
+} else {
+  // If we hit any other kind of token, just bail.
+  return;
+}

Ditto.



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:97
+  }
+
+  // Create a "remapping index" on how to reorder the attributes.

Can we assert `PropertyAttributes.size() > 1` here?



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:104
+  // for missing values.
+  auto sortIndex = [&](const StringRef ) -> unsigned {
+auto i = SortOrderMap.find(needle);





Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:104-106
+  auto sortIndex = [&](const StringRef ) -> unsigned {
+auto i = SortOrderMap.find(needle);
+return (i == SortOrderMap.end()) ? SortOrderMax : i->getValue();

owenpan wrote:
> 
Start names with uppercase letters except for functions.



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:113
+
+  // Deduplicate the attributes.
+  Indices.erase(std::unique(Indices.begin(), Indices.end(),

Is it valid in Objective-C to have duplicate attributes?



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:120
+Indices.end());
+
+  // If there are no removals or shuffling, then don't suggest any fixup.

Is it possible that `Indices` becomes empty or has only one element after 
deduplication? 



Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:126
+  // Generate the replacement text.
+  std::string NewText;
+  for (unsigned Index : Indices) {

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D150083#4655479 , @jaredgrubb 
wrote:

> Should I open a GitHub issue for this patch?

That'd be great.


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

https://reviews.llvm.org/D150083

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

[PATCH] D126132: [clang-format] Fix a crash on lambda trailing return type

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.
Herald added a reviewer: rymiel.

In D126132#3546767 , 
@HazardyKnusperkeks wrote:

> One should define what is the difference between `TT_LambdaArrow` and 
> `TT_TrailingReturnArrow`. But if there is a difference, then the lambda 
> arrows should stay `TT_LambdaArrow`, right?

It seems there is no difference. See 
https://github.com/llvm/llvm-project/pull/70519.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126132

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

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

[Owen Pan via Phabricator via cfe-commits]

owenpan commandeered this revision.
owenpan edited reviewers, added: Luis; removed: owenpan.
Herald added a project: All.
Herald added reviewers: rymiel, HazardyKnusperkeks.
Herald added a comment.

NOTE: Clang-Format Team Automated Review Comment

Your review contains a change to clang/include/clang/Format/Format.h but does 
not contain an update to ClangFormatStyleOptions.rst

ClangFormatStyleOptions.rst is generated via 
clang/docs/tools/dump_format_style.py,  please run this to regenerate the .rst

You can validate that the rst is valid by running.

  ./docs/tools/dump_format_style.py
  mkdir -p html
  /usr/bin/sphinx-build -n ./docs ./html


Herald added a comment.

NOTE: Clang-Format Team Automated Review Comment

It looks like your clang-format review does not contain any unit tests, please 
try to ensure all code changes have a unit test (unless this is an `NFC` or 
refactoring, adding documentation etc..)

Add your unit tests in `clang/unittests/Format` and you can build with `ninja 
FormatTests`.  We recommend using the `verifyFormat(xxx)` format of unit tests 
rather than `EXPECT_EQ` as this will ensure you change is tolerant to random 
whitespace changes (see FormatTest.cpp as an example)

For situations where your change is altering the TokenAnnotator.cpp which can 
happen if you are trying to improve the annotation phase to ensure we are 
correctly identifying the type of a token, please add a token annotator test in 
`TokenAnnotatorTest.cpp`


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] D50403: [clang-format]AlignConsecutiveAssignments

[Owen Pan via Phabricator via cfe-commits]

owenpan abandoned this revision.
owenpan added a comment.

We habe `AlignCompound` now.


Herald added a comment.

NOTE: Clang-Format Team Automated Review Comment

Your review contains a change to clang/include/clang/Format/Format.h but does 
not contain an update to ClangFormatStyleOptions.rst

ClangFormatStyleOptions.rst is generated via 
clang/docs/tools/dump_format_style.py,  please run this to regenerate the .rst

You can validate that the rst is valid by running.

  ./docs/tools/dump_format_style.py
  mkdir -p html
  /usr/bin/sphinx-build -n ./docs ./html


Herald added a comment.

NOTE: Clang-Format Team Automated Review Comment

It looks like your clang-format review does not contain any unit tests, please 
try to ensure all code changes have a unit test (unless this is an `NFC` or 
refactoring, adding documentation etc..)

Add your unit tests in `clang/unittests/Format` and you can build with `ninja 
FormatTests`.  We recommend using the `verifyFormat(xxx)` format of unit tests 
rather than `EXPECT_EQ` as this will ensure you change is tolerant to random 
whitespace changes (see FormatTest.cpp as an example)

For situations where your change is altering the TokenAnnotator.cpp which can 
happen if you are trying to improve the annotation phase to ensure we are 
correctly identifying the type of a token, please add a token annotator test in 
`TokenAnnotatorTest.cpp`


Repository:
  rC Clang

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

https://reviews.llvm.org/D50403

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

[PATCH] D61663: [clang-format] Fix a JavaScript import order bug.

[Owen Pan via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG265ed6819409: [clang-format] Fix a JavaScript import order 
bug (authored by owenpan).

Changed prior to commit:
  https://reviews.llvm.org/D61663?vs=557879=557888#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61663

Files:
  clang/lib/Format/SortJavaScriptImports.cpp
  clang/unittests/Format/SortImportsTestJS.cpp


Index: clang/unittests/Format/SortImportsTestJS.cpp
===
--- clang/unittests/Format/SortImportsTestJS.cpp
+++ clang/unittests/Format/SortImportsTestJS.cpp
@@ -514,6 +514,14 @@
  "export {Y};\n");
 }
 
+TEST_F(SortImportsTestJS, TemplateKeyword) {
+  // Reproduces issue where importing "template" disables imports sorting.
+  verifySort("import {template} from './a';\n"
+ "import {b} from './b';\n",
+ "import {b} from './b';\n"
+ "import {template} from './a';");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/SortJavaScriptImports.cpp
===
--- clang/lib/Format/SortJavaScriptImports.cpp
+++ clang/lib/Format/SortJavaScriptImports.cpp
@@ -548,10 +548,12 @@
   nextToken();
   if (Current->is(tok::r_brace))
 break;
-  bool isTypeOnly =
-  Current->is(Keywords.kw_type) && Current->Next &&
-  Current->Next->isOneOf(tok::identifier, tok::kw_default);
-  if (!isTypeOnly && !Current->isOneOf(tok::identifier, tok::kw_default))
+  auto IsIdentifier = [](const auto *Tok) {
+return Tok->isOneOf(tok::identifier, tok::kw_default, 
tok::kw_template);
+  };
+  bool isTypeOnly = Current->is(Keywords.kw_type) && Current->Next &&
+IsIdentifier(Current->Next);
+  if (!isTypeOnly && !IsIdentifier(Current))
 return false;
 
   JsImportedSymbol Symbol;
@@ -565,7 +567,7 @@
 
   if (Current->is(Keywords.kw_as)) {
 nextToken();
-if (!Current->isOneOf(tok::identifier, tok::kw_default))
+if (!IsIdentifier(Current))
   return false;
 Symbol.Alias = Current->TokenText;
 nextToken();


Index: clang/unittests/Format/SortImportsTestJS.cpp
===
--- clang/unittests/Format/SortImportsTestJS.cpp
+++ clang/unittests/Format/SortImportsTestJS.cpp
@@ -514,6 +514,14 @@
  "export {Y};\n");
 }
 
+TEST_F(SortImportsTestJS, TemplateKeyword) {
+  // Reproduces issue where importing "template" disables imports sorting.
+  verifySort("import {template} from './a';\n"
+ "import {b} from './b';\n",
+ "import {b} from './b';\n"
+ "import {template} from './a';");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/SortJavaScriptImports.cpp
===
--- clang/lib/Format/SortJavaScriptImports.cpp
+++ clang/lib/Format/SortJavaScriptImports.cpp
@@ -548,10 +548,12 @@
   nextToken();
   if (Current->is(tok::r_brace))
 break;
-  bool isTypeOnly =
-  Current->is(Keywords.kw_type) && Current->Next &&
-  Current->Next->isOneOf(tok::identifier, tok::kw_default);
-  if (!isTypeOnly && !Current->isOneOf(tok::identifier, tok::kw_default))
+  auto IsIdentifier = [](const auto *Tok) {
+return Tok->isOneOf(tok::identifier, tok::kw_default, tok::kw_template);
+  };
+  bool isTypeOnly = Current->is(Keywords.kw_type) && Current->Next &&
+IsIdentifier(Current->Next);
+  if (!isTypeOnly && !IsIdentifier(Current))
 return false;
 
   JsImportedSymbol Symbol;
@@ -565,7 +567,7 @@
 
   if (Current->is(Keywords.kw_as)) {
 nextToken();
-if (!Current->isOneOf(tok::identifier, tok::kw_default))
+if (!IsIdentifier(Current))
   return false;
 Symbol.Alias = Current->TokenText;
 nextToken();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61663: [clang-format] Fix a JavaScript import order bug.

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 557879.
owenpan added a comment.

Make the test case pass.


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

https://reviews.llvm.org/D61663

Files:
  clang/lib/Format/SortJavaScriptImports.cpp
  clang/unittests/Format/SortImportsTestJS.cpp


Index: clang/unittests/Format/SortImportsTestJS.cpp
===
--- clang/unittests/Format/SortImportsTestJS.cpp
+++ clang/unittests/Format/SortImportsTestJS.cpp
@@ -514,6 +514,14 @@
  "export {Y};\n");
 }
 
+TEST_F(SortImportsTestJS, TemplateKeyword) {
+  // Reproduces issue where importing "template" disables imports sorting.
+  verifySort("import {template} from './a';\n"
+ "import {b} from './b';\n",
+ "import {b} from './b';\n"
+ "import {template} from './a';\n");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/SortJavaScriptImports.cpp
===
--- clang/lib/Format/SortJavaScriptImports.cpp
+++ clang/lib/Format/SortJavaScriptImports.cpp
@@ -548,10 +548,12 @@
   nextToken();
   if (Current->is(tok::r_brace))
 break;
-  bool isTypeOnly =
-  Current->is(Keywords.kw_type) && Current->Next &&
-  Current->Next->isOneOf(tok::identifier, tok::kw_default);
-  if (!isTypeOnly && !Current->isOneOf(tok::identifier, tok::kw_default))
+  auto IsIdentifier = [](const auto *Tok) {
+return Tok->isOneOf(tok::identifier, tok::kw_default, 
tok::kw_template);
+  };
+  bool isTypeOnly = Current->is(Keywords.kw_type) && Current->Next &&
+IsIdentifier(Current->Next);
+  if (!isTypeOnly && !IsIdentifier(Current))
 return false;
 
   JsImportedSymbol Symbol;
@@ -565,7 +567,7 @@
 
   if (Current->is(Keywords.kw_as)) {
 nextToken();
-if (!Current->isOneOf(tok::identifier, tok::kw_default))
+if (!IsIdentifier(Current))
   return false;
 Symbol.Alias = Current->TokenText;
 nextToken();


Index: clang/unittests/Format/SortImportsTestJS.cpp
===
--- clang/unittests/Format/SortImportsTestJS.cpp
+++ clang/unittests/Format/SortImportsTestJS.cpp
@@ -514,6 +514,14 @@
  "export {Y};\n");
 }
 
+TEST_F(SortImportsTestJS, TemplateKeyword) {
+  // Reproduces issue where importing "template" disables imports sorting.
+  verifySort("import {template} from './a';\n"
+ "import {b} from './b';\n",
+ "import {b} from './b';\n"
+ "import {template} from './a';\n");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/SortJavaScriptImports.cpp
===
--- clang/lib/Format/SortJavaScriptImports.cpp
+++ clang/lib/Format/SortJavaScriptImports.cpp
@@ -548,10 +548,12 @@
   nextToken();
   if (Current->is(tok::r_brace))
 break;
-  bool isTypeOnly =
-  Current->is(Keywords.kw_type) && Current->Next &&
-  Current->Next->isOneOf(tok::identifier, tok::kw_default);
-  if (!isTypeOnly && !Current->isOneOf(tok::identifier, tok::kw_default))
+  auto IsIdentifier = [](const auto *Tok) {
+return Tok->isOneOf(tok::identifier, tok::kw_default, tok::kw_template);
+  };
+  bool isTypeOnly = Current->is(Keywords.kw_type) && Current->Next &&
+IsIdentifier(Current->Next);
+  if (!isTypeOnly && !IsIdentifier(Current))
 return false;
 
   JsImportedSymbol Symbol;
@@ -565,7 +567,7 @@
 
   if (Current->is(Keywords.kw_as)) {
 nextToken();
-if (!Current->isOneOf(tok::identifier, tok::kw_default))
+if (!IsIdentifier(Current))
   return false;
 Symbol.Alias = Current->TokenText;
 nextToken();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61663: [clang-format] Fix a JavaScript import order bug.

[Owen Pan via Phabricator via cfe-commits]

owenpan commandeered this revision.
owenpan edited reviewers, added: bowenni; removed: owenpan.
owenpan added a comment.

The test case doesn't pass.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61663

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

[PATCH] D123676: [clang-format] Fix WhitespaceSensitiveMacros not being honoured when macro closing parenthesis is followed by a newline.

[Owen Pan via Phabricator via cfe-commits]

owenpan closed this revision.
owenpan added a comment.
Herald added a subscriber: wangpc.
Herald added a project: clang-format.
Herald added a reviewer: rymiel.

Fixed in D132001 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123676

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

[PATCH] D139834: [clang-format] AllowShortCompoundRequirementOnASingleLine

[Owen Pan via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG69209e30a716: [clang-format] 
AllowShortCompoundRequirementOnASingleLine (authored by Backl1ght, committed by 
owenpan).

Changed prior to commit:
  https://reviews.llvm.org/D139834?vs=482075=557878#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139834

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -2767,6 +2767,40 @@
Style);
 }
 
+TEST_F(FormatTest, ShortCompoundRequirement) {
+  FormatStyle Style = getLLVMStyle();
+  EXPECT_TRUE(Style.AllowShortCompoundRequirementOnASingleLine);
+  verifyFormat("template \n"
+   "concept c = requires(T x) {\n"
+   "  { x + 1 } -> std::same_as;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept c = requires(T x) {\n"
+   "  { x + 1 } -> std::same_as;\n"
+   "  { x + 2 } -> std::same_as;\n"
+   "};",
+   Style);
+  Style.AllowShortCompoundRequirementOnASingleLine = false;
+  verifyFormat("template \n"
+   "concept c = requires(T x) {\n"
+   "  {\n"
+   "x + 1\n"
+   "  } -> std::same_as;\n"
+   "};",
+   Style);
+  verifyFormat("template \n"
+   "concept c = requires(T x) {\n"
+   "  {\n"
+   "x + 1\n"
+   "  } -> std::same_as;\n"
+   "  {\n"
+   "x + 2\n"
+   "  } -> std::same_as;\n"
+   "};",
+   Style);
+}
+
 TEST_F(FormatTest, ShortCaseLabels) {
   FormatStyle Style = getLLVMStyle();
   Style.AllowShortCaseLabelsOnASingleLine = true;
Index: clang/unittests/Format/ConfigParseTest.cpp
===
--- clang/unittests/Format/ConfigParseTest.cpp
+++ clang/unittests/Format/ConfigParseTest.cpp
@@ -154,6 +154,7 @@
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
+  CHECK_PARSE_BOOL(AllowShortCompoundRequirementOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortEnumsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
   CHECK_PARSE_BOOL(BinPackArguments);
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -520,6 +520,8 @@
   // Try to merge records.
   if (TheLine->Last->is(TT_EnumLBrace)) {
 ShouldMerge = Style.AllowShortEnumsOnASingleLine;
+  } else if (TheLine->Last->is(TT_RequiresExpressionLBrace)) {
+ShouldMerge = Style.AllowShortCompoundRequirementOnASingleLine;
   } else if (TheLine->Last->isOneOf(TT_ClassLBrace, TT_StructLBrace)) {
 // NOTE: We use AfterClass (whereas AfterStruct exists) for both classes
 // and structs, but it seems that wrapping is still handled correctly
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -938,6 +938,8 @@
Style.AllowShortBlocksOnASingleLine);
 IO.mapOptional("AllowShortCaseLabelsOnASingleLine",
Style.AllowShortCaseLabelsOnASingleLine);
+IO.mapOptional("AllowShortCompoundRequirementOnASingleLine",
+   Style.AllowShortCompoundRequirementOnASingleLine);
 IO.mapOptional("AllowShortEnumsOnASingleLine",
Style.AllowShortEnumsOnASingleLine);
 IO.mapOptional("AllowShortFunctionsOnASingleLine",
@@ -1441,6 +1443,7 @@
   LLVMStyle.AllowAllParametersOfDeclarationOnNextLine = true;
   LLVMStyle.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Never;
   LLVMStyle.AllowShortCaseLabelsOnASingleLine = false;
+  LLVMStyle.AllowShortCompoundRequirementOnASingleLine = true;
   LLVMStyle.AllowShortEnumsOnASingleLine = true;
   LLVMStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
   LLVMStyle.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_Never;
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -685,6 +685,25 @@
   /// \version 3.6
   

[PATCH] D33944: git-clang-format: Add --cached option to format index

[Owen Pan via Phabricator via cfe-commits]

owenpan commandeered this revision.
owenpan edited reviewers, added: kevinoid; removed: owenpan.
owenpan added a comment.
Herald added a project: All.

We have `--cached` now.


Herald added a comment.

NOTE: Clang-Format Team Automated Review Comment

It looks like your clang-format review does not contain any unit tests, please 
try to ensure all code changes have a unit test (unless this is an `NFC` or 
refactoring, adding documentation etc..)

Add your unit tests in `clang/unittests/Format` and you can build with `ninja 
FormatTests`.  We recommend using the `verifyFormat(xxx)` format of unit tests 
rather than `EXPECT_EQ` as this will ensure you change is tolerant to random 
whitespace changes (see FormatTest.cpp as an example)

For situations where your change is altering the TokenAnnotator.cpp which can 
happen if you are trying to improve the annotation phase to ensure we are 
correctly identifying the type of a token, please add a token annotator test in 
`TokenAnnotatorTest.cpp`


Repository:
  rC Clang

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

https://reviews.llvm.org/D33944

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

[PATCH] D110252: Added note about Whatstyle and Unformat

[Owen Pan via Phabricator via cfe-commits]

owenpan commandeered this revision.
owenpan edited reviewers, added: Volker-Weissmann; removed: owenpan.
Herald added a project: All.
Herald added a comment.

NOTE: Clang-Format Team Automated Review Comment

Your review contains a change to ClangFormatStyleOptions.rst but not a change 
to clang/include/clang/Format/Format.h

ClangFormatStyleOptions.rst is auto generated from Format.h via 
clang/docs/tools/dump_format_style.py,  please run this to regenerate the .rst

You can validate that the rst is valid by running.

  ./docs/tools/dump_format_style.py
  mkdir -p html
  /usr/bin/sphinx-build -n ./docs ./html


Herald added a comment.

NOTE: Clang-Format Team Automated Review Comment

It looks like your clang-format review does not contain any unit tests, please 
try to ensure all code changes have a unit test (unless this is an `NFC` or 
refactoring, adding documentation etc..)

Add your unit tests in `clang/unittests/Format` and you can build with `ninja 
FormatTests`.  We recommend using the `verifyFormat(xxx)` format of unit tests 
rather than `EXPECT_EQ` as this will ensure you change is tolerant to random 
whitespace changes (see FormatTest.cpp as an example)

For situations where your change is altering the TokenAnnotator.cpp which can 
happen if you are trying to improve the annotation phase to ensure we are 
correctly identifying the type of a token, please add a token annotator test in 
`TokenAnnotatorTest.cpp`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110252

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

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

@jaredgrubb, a few quick comments:

- Please undo the (unrelated) changes to `clang-formatted-files.txt`.
- Add a warning to the option in `Format.h`. See here 
 for an 
example.
- Rerun `dump_format_style.py`.
- Rebase if needed.


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

https://reviews.llvm.org/D150083

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

[PATCH] D156370: [clang-format] Fix bug with parsing of function/variable names.

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D156370#4540257 , 
@HazardyKnusperkeks wrote:

> Yes that stuff. Tests are in: 
> https://github.com/llvm/llvm-project/blob/main/clang/unittests/Format/TokenAnnotatorTest.cpp
> If there is none that matches, just create a new one.

@gedare, I think this is the only missing piece for your patch to get accepted. 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156370

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

[PATCH] D127270: [clang-format] Add space in placement new expression

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7c15dd60ec45: [clang-format] Add space in placement new 
expression (authored by omarahmed, committed by owenpan).

Changed prior to commit:
  https://reviews.llvm.org/D127270?vs=445172=557807#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127270

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11189,6 +11189,42 @@
"void delete(link p);",
"void new (link p);\n"
"void delete (link p);");
+
+  FormatStyle AfterPlacementOperator = getLLVMStyle();
+  AfterPlacementOperator.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  EXPECT_EQ(
+  AfterPlacementOperator.SpaceBeforeParensOptions.AfterPlacementOperator,
+  FormatStyle::SpaceBeforeParensCustom::APO_Leave);
+  verifyFormat("new (buf) int;", AfterPlacementOperator);
+  verifyFormat("new(buf) int;", AfterPlacementOperator);
+
+  AfterPlacementOperator.SpaceBeforeParensOptions.AfterPlacementOperator =
+  FormatStyle::SpaceBeforeParensCustom::APO_Never;
+  verifyFormat("struct A {\n"
+   "  int *a;\n"
+   "  A(int *p) : a(new(p) int) {\n"
+   "new(p) int;\n"
+   "int *b = new(p) int;\n"
+   "int *c = new(p) int(3);\n"
+   "delete(b);\n"
+   "  }\n"
+   "};",
+   AfterPlacementOperator);
+  verifyFormat("void operator new(void *foo) ATTRIB;", AfterPlacementOperator);
+
+  AfterPlacementOperator.SpaceBeforeParensOptions.AfterPlacementOperator =
+  FormatStyle::SpaceBeforeParensCustom::APO_Always;
+  verifyFormat("struct A {\n"
+   "  int *a;\n"
+   "  A(int *p) : a(new (p) int) {\n"
+   "new (p) int;\n"
+   "int *b = new (p) int;\n"
+   "int *c = new (p) int(3);\n"
+   "delete (b);\n"
+   "  }\n"
+   "};",
+   AfterPlacementOperator);
+  verifyFormat("void operator new(void *foo) ATTRIB;", AfterPlacementOperator);
 }
 
 TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
Index: clang/unittests/Format/ConfigParseTest.cpp
===
--- clang/unittests/Format/ConfigParseTest.cpp
+++ clang/unittests/Format/ConfigParseTest.cpp
@@ -591,6 +591,24 @@
   SpaceBeforeParens,
   FormatStyle::SBPO_ControlStatementsExceptControlMacros);
 
+  Style.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  Style.SpaceBeforeParensOptions.AfterPlacementOperator =
+  FormatStyle::SpaceBeforeParensCustom::APO_Always;
+  CHECK_PARSE("SpaceBeforeParensOptions:\n"
+  "  AfterPlacementOperator: Never",
+  SpaceBeforeParensOptions.AfterPlacementOperator,
+  FormatStyle::SpaceBeforeParensCustom::APO_Never);
+
+  CHECK_PARSE("SpaceBeforeParensOptions:\n"
+  "  AfterPlacementOperator: Always",
+  SpaceBeforeParensOptions.AfterPlacementOperator,
+  FormatStyle::SpaceBeforeParensCustom::APO_Always);
+
+  CHECK_PARSE("SpaceBeforeParensOptions:\n"
+  "  AfterPlacementOperator: Leave",
+  SpaceBeforeParensOptions.AfterPlacementOperator,
+  FormatStyle::SpaceBeforeParensCustom::APO_Leave);
+
   // For backward compatibility:
   Style.SpacesInParens = FormatStyle::SIPO_Never;
   Style.SpacesInParensOptions = {};
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4234,6 +4234,19 @@
   return Style.SpaceBeforeParensOptions.AfterIfMacros ||
  spaceRequiredBeforeParens(Right);
 }
+if (Style.SpaceBeforeParens == FormatStyle::SBPO_Custom &&
+Left.isOneOf(tok::kw_new, tok::kw_delete) &&
+Right.isNot(TT_OverloadedOperatorLParen) &&
+!(Line.MightBeFunctionDecl && Left.is(TT_FunctionDeclarationName))) {
+  if (Style.SpaceBeforeParensOptions.AfterPlacementOperator ==
+  FormatStyle::SpaceBeforeParensCustom::APO_Always ||
+  (Style.SpaceBeforeParensOptions.AfterPlacementOperator ==
+   FormatStyle::SpaceBeforeParensCustom::APO_Leave &&
+   Right.hasWhitespaceBefore())) {
+return true;
+  }
+  return false;
+}
 if (Line.Type == LT_ObjCDecl)
   return true;
 if 

[PATCH] D145262: [clang-format] Treat AttributeMacros more like __attribute__

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6f46bcc609f1: [clang-format] Treat AttributeMacro more like 
__attribute__ (authored by jaredgrubb, committed by owenpan).

Changed prior to commit:
  https://reviews.llvm.org/D145262?vs=557674=557707#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145262

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestObjC.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1795,6 +1795,116 @@
   EXPECT_TOKEN(Tokens[13], tok::arrow, TT_Unknown);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsAttributeMacros) {
+  // '__attribute__' has special handling.
+  auto Tokens = annotate("__attribute__(X) void Foo(void);");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::kw___attribute, TT_Unknown);
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_AttributeLParen);
+  EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_AttributeRParen);
+
+  // Generic macro has no special handling in this location.
+  Tokens = annotate("A(X) void Foo(void);");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::identifier, TT_Unknown);
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_Unknown);
+
+  // Add a custom AttributeMacro. Test that it has the same behavior.
+  FormatStyle Style = getLLVMStyle();
+  Style.AttributeMacros.push_back("A");
+
+  // An "AttributeMacro" gets annotated like '__attribute__'.
+  Tokens = annotate("A(X) void Foo(void);", Style);
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::identifier, TT_AttributeMacro);
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_AttributeLParen);
+  EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_AttributeRParen);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsAttributeMacrosOnObjCDecl) {
+  // '__attribute__' has special handling.
+  auto Tokens = annotate("__attribute__(X) @interface Foo");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::kw___attribute, TT_Unknown);
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_AttributeLParen);
+  EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_AttributeRParen);
+
+  // Generic macro has no special handling in this location.
+  Tokens = annotate("A(X) @interface Foo");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  // Note: Don't check token-type as a random token in this position is hard to
+  // reason about.
+  EXPECT_TOKEN_KIND(Tokens[0], tok::identifier);
+  EXPECT_TOKEN_KIND(Tokens[1], tok::l_paren);
+
+  // Add a custom AttributeMacro. Test that it has the same behavior.
+  FormatStyle Style = getLLVMStyle();
+  Style.AttributeMacros.push_back("A");
+
+  // An "AttributeMacro" gets annotated like '__attribute__'.
+  Tokens = annotate("A(X) @interface Foo", Style);
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[0], tok::identifier, TT_AttributeMacro);
+  EXPECT_TOKEN(Tokens[1], tok::l_paren, TT_AttributeLParen);
+  EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_AttributeRParen);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsAttributeMacrosOnObjCMethodDecl) {
+  // '__attribute__' has special handling.
+  auto Tokens = annotate("- (id)init __attribute__(X);");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::kw___attribute, TT_Unknown);
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_AttributeLParen);
+  EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_AttributeRParen);
+
+  // Generic macro has no special handling in this location.
+  Tokens = annotate("- (id)init A(X);");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  // Note: Don't check token-type as a random token in this position is hard to
+  // reason about.
+  EXPECT_TOKEN_KIND(Tokens[5], tok::identifier);
+  EXPECT_TOKEN_KIND(Tokens[6], tok::l_paren);
+
+  // Add a custom AttributeMacro. Test that it has the same behavior.
+  FormatStyle Style = getLLVMStyle();
+  Style.AttributeMacros.push_back("A");
+
+  // An "AttributeMacro" gets annotated like '__attribute__'.
+  Tokens = annotate("- (id)init A(X);", Style);
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::identifier, TT_AttributeMacro);
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_AttributeLParen);
+  EXPECT_TOKEN(Tokens[8], tok::r_paren, TT_AttributeRParen);
+}
+
+TEST_F(TokenAnnotatorTest, UnderstandsAttributeMacrosOnObjCProperty) {
+  // '__attribute__' has special handling.
+  auto Tokens = annotate("@property(weak) id delegate __attribute__(X);");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::kw___attribute, TT_Unknown);
+  EXPECT_TOKEN(Tokens[8], tok::l_paren, TT_AttributeLParen);
+  EXPECT_TOKEN(Tokens[10], 

[PATCH] D145262: [clang-format] Treat AttributeMacros more like __attribute__

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

@jaredgrubb please provide your authorship for `git commit --author` so that we 
can land the patch on your behalf.


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

https://reviews.llvm.org/D145262

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

[PATCH] D145262: [clang-format] Treat AttributeMacros more like __attribute__

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan added a comment.

LGTM


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

https://reviews.llvm.org/D145262

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

[PATCH] D145262: [clang-format] Treat AttributeMacros more like __attribute__

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

Can you add a github issue to show the behavior before this patch?


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

https://reviews.llvm.org/D145262

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

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:1338-1341
 PreviousNonComment->isOneOf(
-TT_AttributeRParen, TT_AttributeSquare, 
TT_FunctionAnnotationRParen,
-TT_JavaAnnotation, TT_LeadingJavaAnnotation))) ||
+TT_AttributeRParen, TT_AttributeMacro, TT_AttributeSquare,
+TT_FunctionAnnotationRParen, TT_JavaAnnotation,
+TT_LeadingJavaAnnotation))) ||

Basically, insert the first two lines and undo the changes to the last three 
lines.



Comment at: clang/lib/Format/TokenAnnotator.cpp:4390-4391
 // Space in __attribute__((attr)) ::type.
 if (Left.is(TT_AttributeRParen) && Right.is(tok::coloncolon))
   return true;
 

Please also add a test case for this.



Comment at: clang/lib/Format/TokenAnnotator.cpp:4706
   return true;
-if (Left.is(tok::r_paren) && canBeObjCSelectorComponent(Right)) {
+// Apply this logic for parens that are not function attribute macros.
+if (Left.is(tok::r_paren) && Left.isNot(TT_AttributeRParen) &&

Please delete this comment, which IMO doesn't quite match the code below.


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

https://reviews.llvm.org/D145262

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

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D145262#4651483 , @owenpan wrote:

> This looks OK to me. Please rebase after 
> https://github.com/llvm/llvm-project/pull/67518 is merged.

I have merged the PR.  Commits cef9f40cd4fe 
 and 
151d0a4db321 
 should 
make this patch less brittle.


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

https://reviews.llvm.org/D145262

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

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

This looks OK to me. Please rebase after 
https://github.com/llvm/llvm-project/pull/67518 is merged.

In D145262#4650300 , @jaredgrubb 
wrote:

> Hopefully this will satisfy and we can merge! I didn't notice the Phabricator 
> timeline and I'm hoping we can finish this before Oct 1.

The new deadline is Nov 15 according to 
https://discourse.llvm.org/t/update-on-github-pull-requests/71540/125.




Comment at: clang/docs/ReleaseNotes.rst:474-475
 - Add ``AllowBreakBeforeNoexceptSpecifier`` option.
+- Improve handling of ``AttributeMacros`` to behave more consistently like
+  regular attribute macros (aka ``__attribute__``)
 

Can you delete this as release notes are usually for new options?



Comment at: clang/lib/Format/TokenAnnotator.cpp:5608-5610
+return Left.isNot(TT_AttributeSquare);
+
+  // Don't split `[[` on C++ attributes.





Comment at: clang/unittests/Format/FormatTestObjC.cpp:1619
+  // Reflow after first macro.
+  // FIXME: these should indent but don't.
+  verifyFormat("- (id)init ATTRIBUTE_MACRO(X)\n"

jaredgrubb wrote:
> owenpan wrote:
> > jaredgrubb wrote:
> > > I don't love this FIXME, but I was afraid to add more to this patch, as 
> > > fixing this will require digging into things that have nothing to do with 
> > > `__attribute__` vs `AttributeMacros`.
> > > 
> > > For example, suffix macros in C/C++ also are broken in the same way with 
> > > just plain `__attribute__`. For example, for `ColumnWidth: 50`:
> > > ```
> > > int f(double) __attribute__((overloadable))
> > > __attribute__((overloadable));
> > > 
> > > int ff(double)
> > > __attribute__((overloadable))
> > > __attribute__((overloadable));
> > > ```
> > > 
> > > I think fixing reflowing of suffix macros is best done in another PR 
> > > (which I can take a stab at!)
> > Half of the test cases passed before this patch but now would fail with 
> > this patch. That is, this patch would generate regressions.
> Just saw this comment. Yes, 3 of these did pass, but lots more in this file 
> do NOT pass. I understand the desire to not "regress", but this patch 
> improves so many other examples (as documented in these test cases). I can 
> pick some of the worst if it helps, but otherwise, I'm not sure what I can do 
> to address your comment.
I'm okay with fixing them in another patch.


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

https://reviews.llvm.org/D145262

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

[PATCH] D136154: [clang-format] Fix the continuation indenter

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.
Herald added a reviewer: MyDeveloperDay.

@hel-ableton do you intend to continue working on this? If not, we can 
commandeer it and finish or abandon it.


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

https://reviews.llvm.org/D136154

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

[PATCH] D127270: [clang-format] Add space in placement new expression

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.
Herald added a reviewer: rymiel.



Comment at: clang/include/clang/Format/Format.h:3555
   AfterFunctionDefinitionName(false), AfterIfMacros(false),
-  AfterOverloadedOperator(false), AfterRequiresInClause(false),
-  AfterRequiresInExpression(false), BeforeNonEmptyParentheses(false) {}
+  AfterOverloadedOperator(false), AfterPlacementOperator(APO_Leave),
+  AfterRequiresInClause(false), AfterRequiresInExpression(false),

omarahmed wrote:
> HazardyKnusperkeks wrote:
> > We change the behavior here, I'm still not convinced we should do that. 
> > Although I'm a big supporter of the `Leave` option.
> I am neutral to both cases, so I leave the choice to you as I am not 
> experienced with clang-format users. Let me know which choice you think would 
> be better.
> We change the behavior here, I'm still not convinced we should do that. 
> Although I'm a big supporter of the `Leave` option.

@HazardyKnusperkeks should we merge this patch as is?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127270

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

[PATCH] D144170: [clang-format] Add simple macro replacements in formatting.

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.

It seems that this patch caused a regression. See 
https://github.com/llvm/llvm-project/issues/62768.


Herald added a comment.

NOTE: Clang-Format Team Automated Review Comment

Your review contains a change to clang/include/clang/Format/Format.h but does 
not contain an update to ClangFormatStyleOptions.rst

ClangFormatStyleOptions.rst is generated via 
clang/docs/tools/dump_format_style.py,  please run this to regenerate the .rst

You can validate that the rst is valid by running.

  ./docs/tools/dump_format_style.py
  mkdir -p html
  /usr/bin/sphinx-build -n ./docs ./html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144170

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

[PATCH] D148131: [clang-format] Avoid unnecessarily aggressive line-breaking when using "LambdaBodyIndentation: OuterScope" with argument bin-packing.

[Owen Pan via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG210e7b3ca773: [clang-format] Improve line-breaking in 
LambdaBodyIndentation: OuterScope (authored by jp4a50, committed by owenpan).

Changed prior to commit:
  https://reviews.llvm.org/D148131?vs=556262=556302#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148131

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -22524,8 +22524,7 @@
"  []() -> auto {\n"
"int b = 32;\n"
"return 3;\n"
-   "  },\n"
-   "  foo, bar)\n"
+   "  }, foo, bar)\n"
"  .foo();\n"
"}",
Style);
@@ -22551,32 +22550,12 @@
"  })));\n"
"}",
Style);
-  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
-  verifyFormat("void foo() {\n"
-   "  aFunction(\n"
-   "  1, b(c(\n"
-   " [](d) -> Foo {\n"
-   "auto f = e(d);\n"
-   "return f;\n"
-   "  },\n"
-   " foo, Bar{},\n"
-   " [] {\n"
-   "auto g = h();\n"
-   "return g;\n"
-   "  },\n"
-   " baz)));\n"
-   "}",
-   Style);
   verifyFormat("void foo() {\n"
"  aFunction(1, b(c(foo, Bar{}, baz, [](d) -> Foo {\n"
-   "auto f = e(\n"
-   "foo,\n"
-   "[&] {\n"
+   "auto f = e(foo, [&] {\n"
"  auto g = h();\n"
"  return g;\n"
-   "},\n"
-   "qux,\n"
-   "[&] -> Bar {\n"
+   "}, qux, [&] -> Bar {\n"
"  auto i = j();\n"
"  return i;\n"
"});\n"
@@ -22584,28 +22563,77 @@
"  })));\n"
"}",
Style);
-  verifyFormat("Namespace::Foo::Foo(\n"
-   "LongClassName bar, AnotherLongClassName baz)\n"
+  verifyFormat("Namespace::Foo::Foo(LongClassName bar,\n"
+   "AnotherLongClassName baz)\n"
": baz{baz}, func{[&] {\n"
"  auto qux = bar;\n"
"  return aFunkyFunctionCall(qux);\n"
"}} {}",
Style);
+  Style.AlignAfterOpenBracket = FormatStyle::BAS_AlwaysBreak;
+  // FIXME: The following test should pass, but fails at the time of writing.
+#if 0
+  // As long as all the non-lambda arguments fit on a single line, AlwaysBreak
+  // doesn't force an initial line break, even if lambdas span multiple lines.
+  verifyFormat("void foo() {\n"
+   "  aFunction(\n"
+   "  [](d) -> Foo {\n"
+   "auto f = e(d);\n"
+   "return f;\n"
+   "  }, foo, Bar{}, [] {\n"
+   "auto g = h();\n"
+   "return g;\n"
+   "  }, baz);\n"
+   "}",
+   Style);
+#endif
+  // A long non-lambda argument forces arguments to span multiple lines and thus
+  // forces an initial line break when using AlwaysBreak.
+  verifyFormat("void foo() {\n"
+   "  aFunction(\n"
+   "  1,\n"
+   "  [](d) -> Foo {\n"
+   "auto f = e(d);\n"
+   "return f;\n"
+   "  }, foo, Bar{},\n"
+   "  [] {\n"
+   "auto g = h();\n"
+   "return g;\n"
+   "  }, baz,\n"
+   "  qx);\n"
+   "}",
+   Style);
+  Style.BinPackArguments = false;
+  verifyFormat("void foo() {\n"
+   "  aFunction(\n"
+   "  1,\n"
+   "  [](d) -> Foo {\n"
+   "auto f = e(d);\n"
+   "return f;\n"
+   "  },\n"
+   "  foo,\n"
+   "  Bar{},\n"
+   "  [] {\n"
+   "auto g = h();\n"
+   "return g;\n"
+   "  },\n"
+   "  baz,\n"
+   "  qx);\n"
+   "}",
+   Style);
+  Style.BinPackArguments = true;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.BeforeLambdaBody = true;
   verifyFormat("void foo() 

[PATCH] D148131: Avoid unnecessarily aggressive line-breaking when using "LambdaBodyIndentation: OuterScope" with argument bin-packing.

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:22575
+#if 0
+  // FIXME: As long as all the non-lambda arguments fit on a single line, 
AlwaysBreak
+  // doesn't force an initial line break, even if lambdas span multiple lines.

This line is too long. Will fix it before merging.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148131

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

[PATCH] D129050: [clang-format] Update documentation

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/docs/ClangFormat.rst:20
 
-  $ clang-format -help
+  $ clang-format --help
   OVERVIEW: A tool to format 
C/C++/Java/JavaScript/JSON/Objective-C/Protobuf/C# code.

owenpan wrote:
> This should be edited in `dump_format_help.py` instead.
Fixed in c47c480b1845.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129050

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

[PATCH] D135115: [clang-format] update --files help description

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/docs/ClangFormat.rst:72-73
  Used only with --dry-run or -n
---files=   - Provide a list of files to run 
clang-format
+--files= - A file containing a list of files to 
process, one
+ per line.
 -i - Inplace edit s, if specified.

owenpan wrote:
> It should be edited in `ClangFormat.cpp` as this file is generated by 
> `dump_format_help.py`.
Fixed in c47c480b1845.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135115

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

[PATCH] D148131: Avoid unnecessarily aggressive line-breaking when using "LambdaBodyIndentation: OuterScope" with argument bin-packing.

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan added a comment.

LG with a couple of minor comments.




Comment at: clang/lib/Format/ContinuationIndenter.cpp:334
+  // enabled.
+  !(Current.is(TT_LambdaLBrace) && Style.BraceWrapping.BeforeLambdaBody) &&
   CurrentState.NoLineBreakInOperand) {

To be consistent with line 1116 below.



Comment at: clang/unittests/Format/FormatTest.cpp:22789
+  // doesn't force an initial line break, even if lambdas span multiple lines.
+  // This should probably be considered a bug.
+  verifyFormat("void foo() {\n"

If so, we should put the test case below in a `#if 0`-`#endif` block with a 
`FIXME` comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148131

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

[PATCH] D129050: [clang-format] Update documentation

[Owen Pan via Phabricator via cfe-commits]

Herald added a subscriber: wangpc.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan.
Herald added a comment.

NOTE: Clang-Format Team Automated Review Comment

It looks like your clang-format review does not contain any unit tests, please 
try to ensure all code changes have a unit test (unless this is an `NFC` or 
refactoring, adding documentation etc..)

Add your unit tests in `clang/unittests/Format` and you can build with `ninja 
FormatTests`.  We recommend using the `verifyFormat(xxx)` format of unit tests 
rather than `EXPECT_EQ` as this will ensure you change is tolerant to random 
whitespace changes (see FormatTest.cpp as an example)

For situations where your change is altering the TokenAnnotator.cpp which can 
happen if you are trying to improve the annotation phase to ensure we are 
correctly identifying the type of a token, please add a token annotator test in 
`TokenAnnotatorTest.cpp`




Comment at: clang/docs/ClangFormat.rst:20
 
-  $ clang-format -help
+  $ clang-format --help
   OVERVIEW: A tool to format 
C/C++/Java/JavaScript/JSON/Objective-C/Protobuf/C# code.

This should be edited in `dump_format_help.py` instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129050

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

[PATCH] D135115: [clang-format] update --files help description

[Owen Pan via Phabricator via cfe-commits]

Herald added a subscriber: wangpc.
Herald added a reviewer: rymiel.
Herald added a comment.

NOTE: Clang-Format Team Automated Review Comment

It looks like your clang-format review does not contain any unit tests, please 
try to ensure all code changes have a unit test (unless this is an `NFC` or 
refactoring, adding documentation etc..)

Add your unit tests in `clang/unittests/Format` and you can build with `ninja 
FormatTests`.  We recommend using the `verifyFormat(xxx)` format of unit tests 
rather than `EXPECT_EQ` as this will ensure you change is tolerant to random 
whitespace changes (see FormatTest.cpp as an example)

For situations where your change is altering the TokenAnnotator.cpp which can 
happen if you are trying to improve the annotation phase to ensure we are 
correctly identifying the type of a token, please add a token annotator test in 
`TokenAnnotatorTest.cpp`




Comment at: clang/docs/ClangFormat.rst:29
 
-  USAGE: clang-format [options] [ ...]
+  USAGE: clang-format [options] [@] [ ...]
 

We should not modify the file directly. See below.



Comment at: clang/docs/ClangFormat.rst:72-73
  Used only with --dry-run or -n
---files=   - Provide a list of files to run 
clang-format
+--files= - A file containing a list of files to 
process, one
+ per line.
 -i - Inplace edit s, if specified.

It should be edited in `ClangFormat.cpp` as this file is generated by 
`dump_format_help.py`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135115

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

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf465a482caa9: [clang-format] Fix segmentation fault when 
formatting nested namespaces (authored by d0nc1h0t, committed by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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
@@ -4180,6 +4180,16 @@
"void foo() {}\n"
"} // namespace ns",
Style);
+
+  FormatStyle LLVMWithCompactInnerNamespace = getLLVMStyle();
+  LLVMWithCompactInnerNamespace.CompactNamespaces = true;
+  LLVMWithCompactInnerNamespace.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("namespace ns1 { namespace ns2 { namespace ns3 {\n"
+   "// block for debug mode\n"
+   "#ifndef NDEBUG\n"
+   "#endif\n"
+   "}}} // namespace ns1::ns2::ns3",
+   LLVMWithCompactInnerNamespace);
 }
 
 TEST_F(FormatTest, NamespaceMacros) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -386,11 +386,14 @@
   // Reduce indent level for bodies of namespaces which were compacted,
   // but only if their content was indented in the first place.
   auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
-  auto OutdentBy = I[J]->Level - TheLine->Level;
+  const int OutdentBy = I[J]->Level - TheLine->Level;
+  assert(OutdentBy >= 0);
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
++CompactedLine) {
-if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+if (!(*CompactedLine)->InPPDirective) {
+  const int Level = (*CompactedLine)->Level;
+  (*CompactedLine)->Level = std::max(Level - OutdentBy, 0);
+}
   }
 }
 return J - 1;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -4180,6 +4180,16 @@
"void foo() {}\n"
"} // namespace ns",
Style);
+
+  FormatStyle LLVMWithCompactInnerNamespace = getLLVMStyle();
+  LLVMWithCompactInnerNamespace.CompactNamespaces = true;
+  LLVMWithCompactInnerNamespace.NamespaceIndentation = FormatStyle::NI_Inner;
+  verifyFormat("namespace ns1 { namespace ns2 { namespace ns3 {\n"
+   "// block for debug mode\n"
+   "#ifndef NDEBUG\n"
+   "#endif\n"
+   "}}} // namespace ns1::ns2::ns3",
+   LLVMWithCompactInnerNamespace);
 }
 
 TEST_F(FormatTest, NamespaceMacros) {
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -386,11 +386,14 @@
   // Reduce indent level for bodies of namespaces which were compacted,
   // but only if their content was indented in the first place.
   auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
-  auto OutdentBy = I[J]->Level - TheLine->Level;
+  const int OutdentBy = I[J]->Level - TheLine->Level;
+  assert(OutdentBy >= 0);
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;
++CompactedLine) {
-if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+if (!(*CompactedLine)->InPPDirective) {
+  const int Level = (*CompactedLine)->Level;
+  (*CompactedLine)->Level = std::max(Level - OutdentBy, 0);
+}
   }
 }
 return J - 1;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159233: [clang-format][NFC] Change duplicate config files to symlinks

[Owen Pan via Phabricator via cfe-commits]

owenpan abandoned this revision.
owenpan added a comment.

It seems that git doesn't recreate symbolic links by default on Windows. (See 
e.g. `clang/test/Driver/Inputs/CUDA-symlinks/usr/bin/ptxas`.) I'll abandon this 
patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159233

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

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

We need the name and email you want to use in order to commit the patch for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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

[PATCH] D158795: Fix AlignArrayOfStructures + Cpp11BracedListStyle=false

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG58c67e724f71: [clang-format] Fix AlignArrayOfStructures + 
Cpp11BracedListStyle=false (authored by galenelias, committed by owenpan).

Changed prior to commit:
  https://reviews.llvm.org/D158795?vs=553809=555164#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158795

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
@@ -20651,6 +20651,15 @@
"});",
Style);
 
+  Style.Cpp11BracedListStyle = false;
+  verifyFormat("struct test demo[] = {\n"
+   "  { 56,23, \"hello\" },\n"
+   "  { -1, 93463, \"world\" },\n"
+   "  {  7, 5,\"!!\" }\n"
+   "};",
+   Style);
+  Style.Cpp11BracedListStyle = true;
+
   Style.ColumnLimit = 0;
   verifyFormat(
   "test demo[] = {\n"
@@ -20882,6 +20891,15 @@
"});",
Style);
 
+  Style.Cpp11BracedListStyle = false;
+  verifyFormat("struct test demo[] = {\n"
+   "  { 56, 23,\"hello\" },\n"
+   "  { -1, 93463, \"world\" },\n"
+   "  { 7,  5, \"!!\"}\n"
+   "};",
+   Style);
+  Style.Cpp11BracedListStyle = true;
+
   Style.ColumnLimit = 0;
   verifyFormat(
   "test demo[] = {\n"
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1230,6 +1230,7 @@
   if (!CellDescs.isRectangular())
 return;
 
+  const int BracePadding = Style.Cpp11BracedListStyle ? 0 : 1;
   auto  = CellDescs.Cells;
   // Now go through and fixup the spaces.
   auto *CellIter = Cells.begin();
@@ -1247,7 +1248,7 @@
   do {
 const FormatToken *Previous = Changes[Next->Index].Tok->Previous;
 if (Previous && Previous->isNot(TT_LineComment)) {
-  Changes[Next->Index].Spaces = 0;
+  Changes[Next->Index].Spaces = BracePadding;
   Changes[Next->Index].NewlinesBefore = 0;
 }
 Next = Next->NextColumnElement;
@@ -1280,7 +1281,7 @@
   NetWidth;
   if (Changes[CellIter->Index].NewlinesBefore == 0) {
 Changes[CellIter->Index].Spaces = (CellWidth - (ThisWidth + NetWidth));
-Changes[CellIter->Index].Spaces += (i > 0) ? 1 : 0;
+Changes[CellIter->Index].Spaces += (i > 0) ? 1 : BracePadding;
   }
   alignToStartOfCell(CellIter->Index, CellIter->EndIndex);
   for (const auto *Next = CellIter->NextColumnElement; Next;
@@ -1289,7 +1290,7 @@
 calculateCellWidth(Next->Index, Next->EndIndex, true) + NetWidth;
 if (Changes[Next->Index].NewlinesBefore == 0) {
   Changes[Next->Index].Spaces = (CellWidth - ThisWidth);
-  Changes[Next->Index].Spaces += (i > 0) ? 1 : 0;
+  Changes[Next->Index].Spaces += (i > 0) ? 1 : BracePadding;
 }
 alignToStartOfCell(Next->Index, Next->EndIndex);
   }
@@ -1303,12 +1304,13 @@
   if (!CellDescs.isRectangular())
 return;
 
+  const int BracePadding = Style.Cpp11BracedListStyle ? 0 : 1;
   auto  = CellDescs.Cells;
   // Now go through and fixup the spaces.
   auto *CellIter = Cells.begin();
   // The first cell needs to be against the left brace.
   if (Changes[CellIter->Index].NewlinesBefore == 0)
-Changes[CellIter->Index].Spaces = 0;
+Changes[CellIter->Index].Spaces = BracePadding;
   else
 Changes[CellIter->Index].Spaces = CellDescs.InitialSpaces;
   ++CellIter;
@@ -1321,7 +1323,8 @@
 if (Changes[CellIter->Index].NewlinesBefore == 0) {
   Changes[CellIter->Index].Spaces =
   MaxNetWidth - ThisNetWidth +
-  (Changes[CellIter->Index].Tok->isNot(tok::r_brace) ? 1 : 0);
+  (Changes[CellIter->Index].Tok->isNot(tok::r_brace) ? 1
+ : BracePadding);
 }
 auto RowCount = 1U;
 auto Offset = std::distance(Cells.begin(), CellIter);
@@ -1335,7 +1338,7 @@
   if (Changes[Next->Index].NewlinesBefore == 0) {
 Changes[Next->Index].Spaces =
 MaxNetWidth - ThisNetWidth +
-(Changes[Next->Index].Tok->isNot(tok::r_brace) ? 1 : 0);
+(Changes[Next->Index].Tok->isNot(tok::r_brace) ? 1 : BracePadding);
   }
   ++RowCount;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

Do you need us to commit it for you? See 
https://llvm.org/docs/Phabricator.html#committing-someone-s-change-from-phabricator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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

[PATCH] D137338: Fix dupe word typos

[Owen Pan via Phabricator via cfe-commits]

Herald added a comment.

NOTE: Clang-Format Team Automated Review Comment

It looks like your clang-format review does not contain any unit tests, please 
try to ensure all code changes have a unit test (unless this is an `NFC` or 
refactoring, adding documentation etc..)

Add your unit tests in `clang/unittests/Format` and you can build with `ninja 
FormatTests`.  We recommend using the `verifyFormat(xxx)` format of unit tests 
rather than `EXPECT_EQ` as this will ensure you change is tolerant to random 
whitespace changes (see FormatTest.cpp as an example)

For situations where your change is altering the TokenAnnotator.cpp which can 
happen if you are trying to improve the annotation phase to ensure we are 
correctly identifying the type of a token, please add a token annotator test in 
`TokenAnnotatorTest.cpp`




Comment at: clang/lib/Format/TokenAnnotator.cpp:4899-4902
+  // Note that this allows the "{" to go over the column limit
   // when the column limit is just between ":" and "{", but that does
   // not happen too often and alternative formattings in this case are
   // not much better.

We should reflow the comment:
```
  // Note that this allows the "{" to go over the column limit when the
  // column limit is just between ":" and "{", but that does not happen too
  // often and alternative formattings in this case are not much better.
```



Comment at: clang/lib/Format/WhitespaceManager.h:202-203
 
-// Determine if every row in the the array
+// Determine if every row in the array
 // has the same number of columns.
 bool isRectangular() const {

We should merge the comment into a single line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137338

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

[PATCH] D159233: [clang-format][NFC] Change duplicate config files to symlinks

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, MyDeveloperDay.
owenpan requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159233

Files:
  clang/include/clang/Format/.clang-format
  clang/tools/clang-format/.clang-format
  clang/unittests/Format/.clang-format


Index: clang/unittests/Format/.clang-format
===
--- /dev/null
+++ clang/unittests/Format/.clang-format
@@ -0,0 +1 @@
+../../lib/Format/.clang-format
\ No newline at end of file
Index: clang/tools/clang-format/.clang-format
===
--- /dev/null
+++ clang/tools/clang-format/.clang-format
@@ -0,0 +1 @@
+../../lib/Format/.clang-format
\ No newline at end of file
Index: clang/include/clang/Format/.clang-format
===
--- /dev/null
+++ clang/include/clang/Format/.clang-format
@@ -0,0 +1 @@
+../../../lib/Format/.clang-format
\ No newline at end of file


Index: clang/unittests/Format/.clang-format
===
--- /dev/null
+++ clang/unittests/Format/.clang-format
@@ -0,0 +1 @@
+../../lib/Format/.clang-format
\ No newline at end of file
Index: clang/tools/clang-format/.clang-format
===
--- /dev/null
+++ clang/tools/clang-format/.clang-format
@@ -0,0 +1 @@
+../../lib/Format/.clang-format
\ No newline at end of file
Index: clang/include/clang/Format/.clang-format
===
--- /dev/null
+++ clang/include/clang/Format/.clang-format
@@ -0,0 +1 @@
+../../../lib/Format/.clang-format
\ No newline at end of file
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159051: [clang-format][NFC] Change EXPECT_EQ to verifyFormat or verifyNoChang

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

rG14feca5d14f1 
 concludes 
the cleanup of this gigantic file WRT `EXPECT_EQ`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159051

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

[PATCH] D147969: Add InsertBraces to ChromiumStyle

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

@pbos can you abandon this revision?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147969

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

[PATCH] D157963: [clang-format] Annotate constructor/destructor names

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG086305120887: Reland [clang-format] Annotate 
constructor/destructor names (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157963

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

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1589,6 +1589,59 @@
   Tokens = annotate("void f [[noreturn]] () {}");
   ASSERT_EQ(Tokens.size(), 12u) << Tokens;
   EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
+
+  Tokens = annotate("class Foo { public: Foo(); };");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::identifier, TT_FunctionDeclarationName);
+
+  Tokens = annotate("class Foo { public: ~Foo(); };");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::identifier, TT_FunctionDeclarationName);
+
+  Tokens = annotate("struct Foo { [[deprecated]] Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { [[deprecated]] ~Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { Foo() [[deprecated]] {} };");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { ~Foo() [[deprecated]] {} };");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { [[deprecated]] explicit Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { virtual [[deprecated]] ~Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("Foo::Foo() {}");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[5], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("Foo::~Foo() {}");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[6], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("#define FOO Foo::\n"
+"FOO Foo();");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::identifier, TT_FunctionDeclarationName);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsC11GenericSelection) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16541,7 +16541,7 @@
 
   verifyFormat("int f();", SpaceFuncDef);
   verifyFormat("void f (int a, T b) {}", SpaceFuncDef);
-  verifyFormat("A::A() : a(1) {}", SpaceFuncDef);
+  verifyFormat("A::A () : a(1) {}", SpaceFuncDef);
   verifyFormat("void f() __attribute__((asdf));", SpaceFuncDef);
   verifyFormat("#define A(x) x", SpaceFuncDef);
   verifyFormat("#define A (x) x", SpaceFuncDef);
@@ -16566,7 +16566,7 @@
   // verifyFormat("T A::operator() () {}", SpaceFuncDef);
   verifyFormat("auto lambda = [] () { return 0; };", SpaceFuncDef);
   verifyFormat("int x = int(y);", SpaceFuncDef);
-  verifyFormat("M(std::size_t R, std::size_t C) : C(C), data(R) {}",
+  verifyFormat("M (std::size_t R, std::size_t C) : C(C), data(R) {}",
SpaceFuncDef);
 
   FormatStyle SpaceIfMacros = getLLVMStyle();
@@ -26246,18 +26246,18 @@
   FormatStyle Style = getLLVMStyle();
   EXPECT_EQ(Style.BreakAfterAttributes, FormatStyle::ABS_Never);
 
-  const StringRef Code("[[nodiscard]] inline int f(int );\n"
-   "[[foo([[]])]] [[nodiscard]]\n"
-   "int g(int );\n"
-   "[[nodiscard]]\n"
-   "inline int f(int ) {\n"
-   "  i = 1;\n"
-   "  return 0;\n"
-   "}\n"
-   "[[foo([[]])]] [[nodiscard]] int g(int ) 

[PATCH] D159051: [clang-format][NFC] Change EXPECT_EQ to verifyFormat or verifyNoChang

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:23933
   verifyNoChange("FOO(String-ized+But(: :Still)=Intentional);", Style);
-  EXPECT_EQ(
-  "FOO(String-ized+But\\(: :Still)=Intentional);",
-  format("FOO(String-ized+But\\(: :Still)=Intentional);", Style));
+  verifyFormat("FOO(String-ized+But\\(: :Still)=Intentional);", Style);
   verifyNoChange("FOO(String-ized+But,: :Still=Intentional);", Style);

To be consistent with the surrounding test cases. (Will fix it before landing.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159051

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

[PATCH] D158947: [clang-format][NFC] Test formatting the input before messing it up

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6982f1fc2e75: [clang-format][NFC] Test formatting the input 
before messing it up (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158947

Files:
  clang/unittests/Format/FormatTestBase.h


Index: clang/unittests/Format/FormatTestBase.h
===
--- clang/unittests/Format/FormatTestBase.h
+++ clang/unittests/Format/FormatTestBase.h
@@ -105,7 +105,9 @@
 
   void _verifyFormat(const char *File, int Line, llvm::StringRef Code,
  const std::optional  = {}) {
-_verifyFormat(File, Line, Code, test::messUp(Code), Style);
+_verifyFormat(File, Line, Code, Code, Style);
+if (const auto MessedUpCode{messUp(Code)}; MessedUpCode != Code)
+  _verifyFormat(File, Line, Code, MessedUpCode, Style);
   }
 
   void _verifyIncompleteFormat(const char *File, int Line, llvm::StringRef 
Code,


Index: clang/unittests/Format/FormatTestBase.h
===
--- clang/unittests/Format/FormatTestBase.h
+++ clang/unittests/Format/FormatTestBase.h
@@ -105,7 +105,9 @@
 
   void _verifyFormat(const char *File, int Line, llvm::StringRef Code,
  const std::optional  = {}) {
-_verifyFormat(File, Line, Code, test::messUp(Code), Style);
+_verifyFormat(File, Line, Code, Code, Style);
+if (const auto MessedUpCode{messUp(Code)}; MessedUpCode != Code)
+  _verifyFormat(File, Line, Code, MessedUpCode, Style);
   }
 
   void _verifyIncompleteFormat(const char *File, int Line, llvm::StringRef Code,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157963: [clang-format] Annotate constructor/destructor names

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 554051.
owenpan added a comment.

Fixed the crash and added a test case in `TokenAnnotatorTest.cpp`.


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

https://reviews.llvm.org/D157963

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

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1589,6 +1589,59 @@
   Tokens = annotate("void f [[noreturn]] () {}");
   ASSERT_EQ(Tokens.size(), 12u) << Tokens;
   EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
+
+  Tokens = annotate("class Foo { public: Foo(); };");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::identifier, TT_FunctionDeclarationName);
+
+  Tokens = annotate("class Foo { public: ~Foo(); };");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::identifier, TT_FunctionDeclarationName);
+
+  Tokens = annotate("struct Foo { [[deprecated]] Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { [[deprecated]] ~Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { Foo() [[deprecated]] {} };");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { ~Foo() [[deprecated]] {} };");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { [[deprecated]] explicit Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { virtual [[deprecated]] ~Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("Foo::Foo() {}");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[5], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("Foo::~Foo() {}");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[6], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("#define FOO Foo::\n"
+"FOO Foo();");
+  ASSERT_EQ(Tokens.size(), 11u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::identifier, TT_FunctionDeclarationName);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsC11GenericSelection) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16541,7 +16541,7 @@
 
   verifyFormat("int f();", SpaceFuncDef);
   verifyFormat("void f (int a, T b) {}", SpaceFuncDef);
-  verifyFormat("A::A() : a(1) {}", SpaceFuncDef);
+  verifyFormat("A::A () : a(1) {}", SpaceFuncDef);
   verifyFormat("void f() __attribute__((asdf));", SpaceFuncDef);
   verifyFormat("#define A(x) x", SpaceFuncDef);
   verifyFormat("#define A (x) x", SpaceFuncDef);
@@ -16566,7 +16566,7 @@
   // verifyFormat("T A::operator() () {}", SpaceFuncDef);
   verifyFormat("auto lambda = [] () { return 0; };", SpaceFuncDef);
   verifyFormat("int x = int(y);", SpaceFuncDef);
-  verifyFormat("M(std::size_t R, std::size_t C) : C(C), data(R) {}",
+  verifyFormat("M (std::size_t R, std::size_t C) : C(C), data(R) {}",
SpaceFuncDef);
 
   FormatStyle SpaceIfMacros = getLLVMStyle();
@@ -26239,18 +26239,18 @@
   FormatStyle Style = getLLVMStyle();
   EXPECT_EQ(Style.BreakAfterAttributes, FormatStyle::ABS_Never);
 
-  const StringRef Code("[[nodiscard]] inline int f(int );\n"
-   "[[foo([[]])]] [[nodiscard]]\n"
-   "int g(int );\n"
-   "[[nodiscard]]\n"
-   "inline int f(int ) {\n"
-   "  i = 1;\n"
-   "  return 0;\n"
-   "}\n"
-   "[[foo([[]])]] [[nodiscard]] int g(int ) {\n"
-   "  i = 0;\n"
-   "  return 1;\n"
-   

[PATCH] D157963: [clang-format] Annotate constructor/destructor names

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3135
+  if (!Tok)
+break;
+}

When simplifying the function, I forgot that we are in the inner loop here and 
thus `break` won't exit the outer loop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157963

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

[PATCH] D158795: Fix AlignArrayOfStructures + Cpp11BracedListStyle=false

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:20888
+   "  {  7, 5,\"!!\" }\n"
+   "};\n",
+   Style);

galenelias wrote:
> owenpan wrote:
> > galenelias wrote:
> > > owenpan wrote:
> > > > 
> > > This is consistent with basically every single adjacent test in this 
> > > function.  While I agree that this is unnecessary, in general I error on 
> > > the side of consistency with the surrounding tests.  I'll defer to the 
> > > maintainers, just wanted to make sure this is actually the preferred 
> > > change given the numerous adjacent tests with this form.
> > If you rebase your patch, you'll see that the trailing newlines in the 
> > surrounding tests have been removed. (Even if they had not been removed, we 
> > still wouldn't want new tests to have superfluous trailing newlines.)
> Thanks for the information, looks like I missed your cleanup change by a day. 
>  In general some code-bases favor consistency over some extra gunk, so I tend 
> to default to consistency, but good to know going forward.  I've rebased now.
Np!


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

https://reviews.llvm.org/D158795

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

[PATCH] D158795: Fix AlignArrayOfStructures + Cpp11BracedListStyle=false

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:20888
+   "  {  7, 5,\"!!\" }\n"
+   "};\n",
+   Style);

galenelias wrote:
> owenpan wrote:
> > 
> This is consistent with basically every single adjacent test in this 
> function.  While I agree that this is unnecessary, in general I error on the 
> side of consistency with the surrounding tests.  I'll defer to the 
> maintainers, just wanted to make sure this is actually the preferred change 
> given the numerous adjacent tests with this form.
If you rebase your patch, you'll see that the trailing newlines in the 
surrounding tests have been removed. (Even if they had not been removed, we 
still wouldn't want new tests to have superfluous trailing newlines.)


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

https://reviews.llvm.org/D158795

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

[PATCH] D158947: [clang-format][NFC] Test formatting the input before messing it up

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, MyDeveloperDay.
owenpan requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158947

Files:
  clang/unittests/Format/FormatTestBase.h


Index: clang/unittests/Format/FormatTestBase.h
===
--- clang/unittests/Format/FormatTestBase.h
+++ clang/unittests/Format/FormatTestBase.h
@@ -105,7 +105,9 @@
 
   void _verifyFormat(const char *File, int Line, llvm::StringRef Code,
  const std::optional  = {}) {
-_verifyFormat(File, Line, Code, test::messUp(Code), Style);
+_verifyFormat(File, Line, Code, Code, Style);
+if (const auto MessedUpCode{messUp(Code)}; MessedUpCode != Code)
+  _verifyFormat(File, Line, Code, MessedUpCode, Style);
   }
 
   void _verifyIncompleteFormat(const char *File, int Line, llvm::StringRef 
Code,


Index: clang/unittests/Format/FormatTestBase.h
===
--- clang/unittests/Format/FormatTestBase.h
+++ clang/unittests/Format/FormatTestBase.h
@@ -105,7 +105,9 @@
 
   void _verifyFormat(const char *File, int Line, llvm::StringRef Code,
  const std::optional  = {}) {
-_verifyFormat(File, Line, Code, test::messUp(Code), Style);
+_verifyFormat(File, Line, Code, Code, Style);
+if (const auto MessedUpCode{messUp(Code)}; MessedUpCode != Code)
+  _verifyFormat(File, Line, Code, MessedUpCode, Style);
   }
 
   void _verifyIncompleteFormat(const char *File, int Line, llvm::StringRef Code,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158925: [clang-format][NFC] Skip stability test if input is pre-formatted

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc8521451708a: [clang-format][NFC] Skip stability test if 
input is pre-formatted (authored by owenpan).

Changed prior to commit:
  https://reviews.llvm.org/D158925?vs=553714=553765#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158925

Files:
  clang/unittests/Format/FormatTestBase.h


Index: clang/unittests/Format/FormatTestBase.h
===
--- clang/unittests/Format/FormatTestBase.h
+++ clang/unittests/Format/FormatTestBase.h
@@ -85,9 +85,11 @@
  const std::optional  = {},
  const std::vector  = {}) {
 testing::ScopedTrace t(File, Line, ::testing::Message() << Code.str());
-EXPECT_EQ(Expected.str(),
-  format(Expected, Style, SC_ExpectComplete, Ranges))
-<< "Expected code is not stable";
+if (Expected != Code) {
+  EXPECT_EQ(Expected.str(),
+format(Expected, Style, SC_ExpectComplete, Ranges))
+  << "Expected code is not stable";
+}
 EXPECT_EQ(Expected.str(), format(Code, Style, SC_ExpectComplete, Ranges));
 auto UsedStyle = Style ? Style.value() : getDefaultStyle();
 if (UsedStyle.Language == FormatStyle::LK_Cpp) {


Index: clang/unittests/Format/FormatTestBase.h
===
--- clang/unittests/Format/FormatTestBase.h
+++ clang/unittests/Format/FormatTestBase.h
@@ -85,9 +85,11 @@
  const std::optional  = {},
  const std::vector  = {}) {
 testing::ScopedTrace t(File, Line, ::testing::Message() << Code.str());
-EXPECT_EQ(Expected.str(),
-  format(Expected, Style, SC_ExpectComplete, Ranges))
-<< "Expected code is not stable";
+if (Expected != Code) {
+  EXPECT_EQ(Expected.str(),
+format(Expected, Style, SC_ExpectComplete, Ranges))
+  << "Expected code is not stable";
+}
 EXPECT_EQ(Expected.str(), format(Code, Style, SC_ExpectComplete, Ranges));
 auto UsedStyle = Style ? Style.value() : getDefaultStyle();
 if (UsedStyle.Language == FormatStyle::LK_Cpp) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158925: [clang-format][NFC] Skip stability test if input is pre-formatted

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTestBase.h:88
 testing::ScopedTrace t(File, Line, ::testing::Message() << Code.str());
-EXPECT_EQ(Expected.str(),
-  format(Expected, Style, SC_ExpectComplete, Ranges))
-<< "Expected code is not stable";
+if (!Expected.equals(Code)) {
+  EXPECT_EQ(Expected.str(),

Will fix it before landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158925

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

[PATCH] D158925: [clang-format][NFC] Skip stability test if input is pre-formatted

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, MyDeveloperDay.
owenpan requested review of this revision.

This shortens the run-time of `FormatTests` by about 10% on average (and by up 
to 50% if formatting would not change the input).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158925

Files:
  clang/unittests/Format/FormatTestBase.h


Index: clang/unittests/Format/FormatTestBase.h
===
--- clang/unittests/Format/FormatTestBase.h
+++ clang/unittests/Format/FormatTestBase.h
@@ -85,9 +85,11 @@
  const std::optional  = {},
  const std::vector  = {}) {
 testing::ScopedTrace t(File, Line, ::testing::Message() << Code.str());
-EXPECT_EQ(Expected.str(),
-  format(Expected, Style, SC_ExpectComplete, Ranges))
-<< "Expected code is not stable";
+if (!Expected.equals(Code)) {
+  EXPECT_EQ(Expected.str(),
+format(Expected, Style, SC_ExpectComplete, Ranges))
+  << "Expected code is not stable";
+}
 EXPECT_EQ(Expected.str(), format(Code, Style, SC_ExpectComplete, Ranges));
 auto UsedStyle = Style ? Style.value() : getDefaultStyle();
 if (UsedStyle.Language == FormatStyle::LK_Cpp) {


Index: clang/unittests/Format/FormatTestBase.h
===
--- clang/unittests/Format/FormatTestBase.h
+++ clang/unittests/Format/FormatTestBase.h
@@ -85,9 +85,11 @@
  const std::optional  = {},
  const std::vector  = {}) {
 testing::ScopedTrace t(File, Line, ::testing::Message() << Code.str());
-EXPECT_EQ(Expected.str(),
-  format(Expected, Style, SC_ExpectComplete, Ranges))
-<< "Expected code is not stable";
+if (!Expected.equals(Code)) {
+  EXPECT_EQ(Expected.str(),
+format(Expected, Style, SC_ExpectComplete, Ranges))
+  << "Expected code is not stable";
+}
 EXPECT_EQ(Expected.str(), format(Code, Style, SC_ExpectComplete, Ranges));
 auto UsedStyle = Style ? Style.value() : getDefaultStyle();
 if (UsedStyle.Language == FormatStyle::LK_Cpp) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158795: Fix AlignArrayOfStructures + Cpp11BracedListStyle=false

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/WhitespaceManager.cpp:1247
 if (Previous && Previous->isNot(TT_LineComment)) {
-  Changes[Next->Index].Spaces = 0;
+  Changes[Next->Index].Spaces = BracePadding;
   Changes[Next->Index].NewlinesBefore = 0;

owenpan wrote:
> Can we assert that `Spaces == 0`? If not, we should add a test case.
We can't assert that, but setting `Spaces` here seems superfluous as it's set 
correctly below anyways?


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

https://reviews.llvm.org/D158795

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

[PATCH] D158805: [clang-format][NFC] Remove extraneous newlines at end of test cases

[Owen Pan via Phabricator via cfe-commits]

owenpan marked an inline comment as done.
owenpan added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:24187
 
   // Newlines are important here.
+  verifyFormat("FOO(1+2 )", Style);

HazardyKnusperkeks wrote:
> This.
Good catch! Though `verifyFormat` removed the newlines anyways.


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

https://reviews.llvm.org/D158805

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

[PATCH] D158795: Fix AlignArrayOfStructures + Cpp11BracedListStyle=false

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

> I'm not exactly sure why this function needs to override the Spaces as it 
> seems to generally already be set to either 0 or 1 according to the other 
> formatting settings

If so, can we address the issue without the "explicit fix"?




Comment at: clang/lib/Format/WhitespaceManager.cpp:1229
 
+  const auto BracePadding = Style.Cpp11BracedListStyle ? 0 : 1;
   auto  = CellDescs.Cells;





Comment at: clang/lib/Format/WhitespaceManager.cpp:1247
 if (Previous && Previous->isNot(TT_LineComment)) {
-  Changes[Next->Index].Spaces = 0;
+  Changes[Next->Index].Spaces = BracePadding;
   Changes[Next->Index].NewlinesBefore = 0;

Can we assert that `Spaces == 0`? If not, we should add a test case.



Comment at: clang/lib/Format/WhitespaceManager.cpp:1303
 
+  const auto BracePadding = Style.Cpp11BracedListStyle ? 0 : 1;
   auto  = CellDescs.Cells;

Ditto.



Comment at: clang/unittests/Format/FormatTest.cpp:20888
+   "  {  7, 5,\"!!\" }\n"
+   "};\n",
+   Style);





Comment at: clang/unittests/Format/FormatTest.cpp:21130
+   "  { 7,  5, \"!!\"}\n"
+   "};\n",
+   Style);

Ditto.


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

https://reviews.llvm.org/D158795

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

[PATCH] D154093: [clang-format] Break long string literals in C#, etc.

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D154093#4615948 , @sstwcw wrote:

> In the JavaScript tests that I added, it was wrong to use `SmallString`.  
> Would you prefer me changing it to `string` or expanding the 6 test cases so 
> we don't need a variable for the string?

You mean `std::string`? That would be fine by me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154093

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

[PATCH] D157963: [clang-format] Annotate constructor/destructor names

[Owen Pan via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0e63f1aacc00: [clang-format] Annotate constructor/destructor 
names (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157963

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

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1589,6 +1589,54 @@
   Tokens = annotate("void f [[noreturn]] () {}");
   ASSERT_EQ(Tokens.size(), 12u) << Tokens;
   EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
+
+  Tokens = annotate("class Foo { public: Foo(); };");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::identifier, TT_FunctionDeclarationName);
+
+  Tokens = annotate("class Foo { public: ~Foo(); };");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::identifier, TT_FunctionDeclarationName);
+
+  Tokens = annotate("struct Foo { [[deprecated]] Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { [[deprecated]] ~Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { Foo() [[deprecated]] {} };");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { ~Foo() [[deprecated]] {} };");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { [[deprecated]] explicit Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { virtual [[deprecated]] ~Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("Foo::Foo() {}");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[5], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("Foo::~Foo() {}");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[6], tok::l_brace, TT_FunctionLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsC11GenericSelection) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16542,7 +16542,7 @@
 
   verifyFormat("int f();", SpaceFuncDef);
   verifyFormat("void f (int a, T b) {}", SpaceFuncDef);
-  verifyFormat("A::A() : a(1) {}", SpaceFuncDef);
+  verifyFormat("A::A () : a(1) {}", SpaceFuncDef);
   verifyFormat("void f() __attribute__((asdf));", SpaceFuncDef);
   verifyFormat("#define A(x) x", SpaceFuncDef);
   verifyFormat("#define A (x) x", SpaceFuncDef);
@@ -16567,7 +16567,7 @@
   // verifyFormat("T A::operator() () {}", SpaceFuncDef);
   verifyFormat("auto lambda = [] () { return 0; };", SpaceFuncDef);
   verifyFormat("int x = int(y);", SpaceFuncDef);
-  verifyFormat("M(std::size_t R, std::size_t C) : C(C), data(R) {}",
+  verifyFormat("M (std::size_t R, std::size_t C) : C(C), data(R) {}",
SpaceFuncDef);
 
   FormatStyle SpaceIfMacros = getLLVMStyle();
@@ -26242,18 +26242,18 @@
   FormatStyle Style = getLLVMStyle();
   EXPECT_EQ(Style.BreakAfterAttributes, FormatStyle::ABS_Never);
 
-  const StringRef Code("[[nodiscard]] inline int f(int );\n"
-   "[[foo([[]])]] [[nodiscard]]\n"
-   "int g(int );\n"
-   "[[nodiscard]]\n"
-   "inline int f(int ) {\n"
-   "  i = 1;\n"
-   "  return 0;\n"
-   "}\n"
-   "[[foo([[]])]] [[nodiscard]] int g(int ) {\n"
-   "  i = 0;\n"
-   "  return 1;\n"
-   "}");
+  constexpr StringRef 

[PATCH] D158571: [clang-format][NFC] Replace !is() with isNot()

[Owen Pan via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG91c4db00612b: [clang-format][NFC] Replace !is() with isNot() 
(authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158571

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/DefinitionBlockSeparator.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/FormatTokenSource.h
  clang/lib/Format/MacroExpander.cpp
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/lib/Format/SortJavaScriptImports.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/WhitespaceManager.cpp

Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -590,14 +590,14 @@
 
   // A new line starts, re-initialize line status tracking bools.
   // Keep the match state if a string literal is continued on this line.
-  if (i == 0 || !Changes[i].Tok->is(tok::string_literal) ||
-  !Changes[i - 1].Tok->is(tok::string_literal)) {
+  if (i == 0 || Changes[i].Tok->isNot(tok::string_literal) ||
+  Changes[i - 1].Tok->isNot(tok::string_literal)) {
 FoundMatchOnLine = false;
   }
   LineIsComment = true;
 }
 
-if (!Changes[i].Tok->is(tok::comment))
+if (Changes[i].Tok->isNot(tok::comment))
   LineIsComment = false;
 
 if (Changes[i].Tok->is(tok::comma)) {
@@ -731,10 +731,10 @@
   SpacesRequiredBefore = 0;
 }
 
-if (!Current || !Current->is(tok::identifier))
+if (!Current || Current->isNot(tok::identifier))
   return false;
 
-if (!Current->Previous || !Current->Previous->is(tok::pp_define))
+if (!Current->Previous || Current->Previous->isNot(tok::pp_define))
   return false;
 
 // For a macro function, 0 spaces are required between the
@@ -781,7 +781,7 @@
   LineIsComment = true;
 }
 
-if (!Changes[I].Tok->is(tok::comment))
+if (Changes[I].Tok->isNot(tok::comment))
   LineIsComment = false;
 
 if (!AlignMacrosMatches(Changes[I]))
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -394,7 +394,7 @@
 ParseDefault();
 continue;
   }
-  if (CanContainBracedList && !FormatTok->is(TT_MacroBlockBegin) &&
+  if (CanContainBracedList && FormatTok->isNot(TT_MacroBlockBegin) &&
   tryToParseBracedList()) {
 continue;
   }
@@ -615,7 +615,7 @@
   LBraceStack.pop_back();
   break;
 case tok::identifier:
-  if (!Tok->is(TT_StatementMacro))
+  if (Tok->isNot(TT_StatementMacro))
 break;
   [[fallthrough]];
 case tok::at:
@@ -799,8 +799,8 @@
   if (eof())
 return IfLBrace;
 
-  if (MacroBlock ? !FormatTok->is(TT_MacroBlockEnd)
- : !FormatTok->is(tok::r_brace)) {
+  if (MacroBlock ? FormatTok->isNot(TT_MacroBlockEnd)
+ : FormatTok->isNot(tok::r_brace)) {
 Line->Level = InitialLevel;
 FormatTok->setBlockKind(BK_Block);
 return IfLBrace;
@@ -1080,7 +1080,7 @@
   bool MaybeIncludeGuard = IfNDef;
   if (IncludeGuard == IG_Inited && MaybeIncludeGuard) {
 for (auto  : Lines) {
-  if (!Line.Tokens.front().Tok->is(tok::comment)) {
+  if (Line.Tokens.front().Tok->isNot(tok::comment)) {
 MaybeIncludeGuard = false;
 IncludeGuard = IG_Rejected;
 break;
@@ -1612,7 +1612,7 @@
 nextToken();
 if (FormatTok->is(tok::kw_public))
   nextToken();
-if (!FormatTok->is(tok::string_literal))
+if (FormatTok->isNot(tok::string_literal))
   return;
 nextToken();
 if (FormatTok->is(tok::semi))
@@ -1887,8 +1887,9 @@
   // a new unwrapped line, so they are special cased below.
   size_t TokenCount = Line->Tokens.size();
   if (Style.isJavaScript() && FormatTok->is(Keywords.kw_function) &&
-  (TokenCount > 1 || (TokenCount == 1 && !Line->Tokens.front().Tok->is(
- Keywords.kw_async {
+  (TokenCount > 1 ||
+   (TokenCount == 1 &&
+Line->Tokens.front().Tok->isNot(Keywords.kw_async {
 tryToParseJSFunction();
 break;
   }
@@ -2872,7 +2873,7 @@
   FormatTok->is(tok::l_brace)) {
 do {
   nextToken();
-} while (!FormatTok->is(tok::r_brace));
+} while (FormatTok->isNot(tok::r_brace));
 nextToken();
   }
 
@@ -2895,7 +2896,7 @@
   

[PATCH] D158571: [clang-format][NFC] Replace !is() with isNot()

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 553032.
owenpan added a comment.

Rebased to 825cec2 
 and ran 
`git-clang-format`.


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

https://reviews.llvm.org/D158571

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/DefinitionBlockSeparator.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/FormatTokenSource.h
  clang/lib/Format/MacroExpander.cpp
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/lib/Format/SortJavaScriptImports.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/WhitespaceManager.cpp

Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -590,14 +590,14 @@
 
   // A new line starts, re-initialize line status tracking bools.
   // Keep the match state if a string literal is continued on this line.
-  if (i == 0 || !Changes[i].Tok->is(tok::string_literal) ||
-  !Changes[i - 1].Tok->is(tok::string_literal)) {
+  if (i == 0 || Changes[i].Tok->isNot(tok::string_literal) ||
+  Changes[i - 1].Tok->isNot(tok::string_literal)) {
 FoundMatchOnLine = false;
   }
   LineIsComment = true;
 }
 
-if (!Changes[i].Tok->is(tok::comment))
+if (Changes[i].Tok->isNot(tok::comment))
   LineIsComment = false;
 
 if (Changes[i].Tok->is(tok::comma)) {
@@ -731,10 +731,10 @@
   SpacesRequiredBefore = 0;
 }
 
-if (!Current || !Current->is(tok::identifier))
+if (!Current || Current->isNot(tok::identifier))
   return false;
 
-if (!Current->Previous || !Current->Previous->is(tok::pp_define))
+if (!Current->Previous || Current->Previous->isNot(tok::pp_define))
   return false;
 
 // For a macro function, 0 spaces are required between the
@@ -781,7 +781,7 @@
   LineIsComment = true;
 }
 
-if (!Changes[I].Tok->is(tok::comment))
+if (Changes[I].Tok->isNot(tok::comment))
   LineIsComment = false;
 
 if (!AlignMacrosMatches(Changes[I]))
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -394,7 +394,7 @@
 ParseDefault();
 continue;
   }
-  if (CanContainBracedList && !FormatTok->is(TT_MacroBlockBegin) &&
+  if (CanContainBracedList && FormatTok->isNot(TT_MacroBlockBegin) &&
   tryToParseBracedList()) {
 continue;
   }
@@ -615,7 +615,7 @@
   LBraceStack.pop_back();
   break;
 case tok::identifier:
-  if (!Tok->is(TT_StatementMacro))
+  if (Tok->isNot(TT_StatementMacro))
 break;
   [[fallthrough]];
 case tok::at:
@@ -799,8 +799,8 @@
   if (eof())
 return IfLBrace;
 
-  if (MacroBlock ? !FormatTok->is(TT_MacroBlockEnd)
- : !FormatTok->is(tok::r_brace)) {
+  if (MacroBlock ? FormatTok->isNot(TT_MacroBlockEnd)
+ : FormatTok->isNot(tok::r_brace)) {
 Line->Level = InitialLevel;
 FormatTok->setBlockKind(BK_Block);
 return IfLBrace;
@@ -1080,7 +1080,7 @@
   bool MaybeIncludeGuard = IfNDef;
   if (IncludeGuard == IG_Inited && MaybeIncludeGuard) {
 for (auto  : Lines) {
-  if (!Line.Tokens.front().Tok->is(tok::comment)) {
+  if (Line.Tokens.front().Tok->isNot(tok::comment)) {
 MaybeIncludeGuard = false;
 IncludeGuard = IG_Rejected;
 break;
@@ -1612,7 +1612,7 @@
 nextToken();
 if (FormatTok->is(tok::kw_public))
   nextToken();
-if (!FormatTok->is(tok::string_literal))
+if (FormatTok->isNot(tok::string_literal))
   return;
 nextToken();
 if (FormatTok->is(tok::semi))
@@ -1887,8 +1887,9 @@
   // a new unwrapped line, so they are special cased below.
   size_t TokenCount = Line->Tokens.size();
   if (Style.isJavaScript() && FormatTok->is(Keywords.kw_function) &&
-  (TokenCount > 1 || (TokenCount == 1 && !Line->Tokens.front().Tok->is(
- Keywords.kw_async {
+  (TokenCount > 1 ||
+   (TokenCount == 1 &&
+Line->Tokens.front().Tok->isNot(Keywords.kw_async {
 tryToParseJSFunction();
 break;
   }
@@ -2872,7 +2873,7 @@
   FormatTok->is(tok::l_brace)) {
 do {
   nextToken();
-} while (!FormatTok->is(tok::r_brace));
+} while (FormatTok->isNot(tok::r_brace));
 nextToken();
   }
 
@@ -2895,7 +2896,7 @@
   addUnwrappedLine();
 else
   NeedsUnwrappedLine = true;
-  } else if 

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:389
   auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
-  auto OutdentBy = I[J]->Level - TheLine->Level;
+  int OutdentBy = I[J]->Level - TheLine->Level;
+  assert(OutdentBy >= 0);

I forgot to suggest `const`.



Comment at: clang/unittests/Format/FormatTest.cpp:4231-4234
+"// block for debug mode\n"
+"#ifndef NDEBUG\n"
+"#endif\n"
+"}}} // namespace ns1::ns2::ns3",

Did you run `git-clang-format`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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

[PATCH] D154093: [clang-format] Break long string literals in C#, etc.

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

@sstwcw this patch caused a crash in `FormatTestJS.StringLiteralConcatenation`:

  Assertion failed: ((!RequiresNullTerminator || BufEnd[0] == 0) && "Buffer is 
not null terminated!"), function init, file MemoryBuffer.cpp, line 54.

Can you revert it if you can reproduce the crash?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154093

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

[PATCH] D158697: [clang-format][doc] Correct typos

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/docs/ClangFormatStyleOptions.rst:5129
 
   * ``bool AfterControlStatements`` If ``true``, put space between control 
statement keywords
 (for/if/while...) and opening parentheses.

11e2975810acd fixed the typo `betwee` here while it should have fixed it in 
Format.h instead.



Comment at: clang/include/clang/Format/Format.h:4046
   struct SpaceBeforeParensCustom {
-/// If ``true``, put space betwee control statement keywords
+/// If ``true``, put space between control statement keywords
 /// (for/if/while...) and opening parentheses.

Or to be more precise: `put a space between a control statement keyword 
(for/if/while) and the opening parenthesis that follows.`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158697

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

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Owen Pan via Phabricator via cfe-commits]

owenpan requested changes to this revision.
owenpan added a comment.
This revision now requires changes to proceed.

It seems that you forgot to update the diff?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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

[PATCH] D156259: [clang-format] Fix a bug that erroneously placed function arguments on a new line despite all arguments being able to fit on the same line.

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd06b92391513: [clang-format] Fix a bug that wraps before 
function arguments (authored by jp4a50, committed by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156259

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/ContinuationIndenter.h
  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
@@ -4,8 +4,25 @@
"  }\n"
"};");
 
-  // Multiple lambdas in the same parentheses change indentation rules. These
-  // lambdas are forced to start on new lines.
+  // Lambdas that fit on a single line within an argument list are not forced
+  // onto new lines.
+  verifyFormat("SomeFunction([] {});");
+  verifyFormat("SomeFunction(0, [] {});");
+  verifyFormat("SomeFunction([] {}, 0);");
+  verifyFormat("SomeFunction(0, [] {}, 0);");
+  verifyFormat("SomeFunction([] { return 0; }, 0);");
+  verifyFormat("SomeFunction(a, [] { return 0; }, b);");
+  verifyFormat("SomeFunction([] { return 0; }, [] { return 0; });");
+  verifyFormat("SomeFunction([] { return 0; }, [] { return 0; }, b);");
+  verifyFormat("auto lng =\n"
+   "SomeFunction([] { return 0; }, [] { return 0; }, b);");
+  // Exceeded column limit. We need to break.
+  verifyFormat("auto lngName = SomeFunction(\n"
+   "[] { return anotherLooonngName; }, [] { "
+   "return 0; }, b);");
+
+  // Multiple multi-line lambdas in the same parentheses change indentation
+  // rules. These lambdas are always forced to start on new lines.
   verifyFormat("SomeFunction(\n"
"[]() {\n"
"  //\n"
@@ -22234,7 +22251,7 @@
"  //\n"
"});");
 
-  // A lambda passed as arg0 is always pushed to the next line.
+  // A multi-line lambda passed as arg0 is always pushed to the next line.
   verifyFormat("SomeFunction(\n"
"[this] {\n"
"  //\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -5193,30 +5193,6 @@
   return true;
   }
 
-  // Deal with lambda arguments in C++ - we want consistent line breaks whether
-  // they happen to be at arg0, arg1 or argN. The selection is a bit nuanced
-  // as aggressive line breaks are placed when the lambda is not the last arg.
-  if ((Style.Language == FormatStyle::LK_Cpp ||
-   Style.Language == FormatStyle::LK_ObjC) &&
-  Left.is(tok::l_paren) && Left.BlockParameterCount > 0 &&
-  !Right.isOneOf(tok::l_paren, TT_LambdaLSquare)) {
-// Multiple lambdas in the same function call force line breaks.
-if (Left.BlockParameterCount > 1)
-  return true;
-
-// A lambda followed by another arg forces a line break.
-if (!Left.Role)
-  return false;
-auto Comma = Left.Role->lastComma();
-if (!Comma)
-  return false;
-auto Next = Comma->getNextNonComment();
-if (!Next)
-  return false;
-if (!Next->isOneOf(TT_LambdaLSquare, tok::l_brace, tok::caret))
-  return true;
-  }
-
   return false;
 }
 
Index: clang/lib/Format/ContinuationIndenter.h
===
--- clang/lib/Format/ContinuationIndenter.h
+++ clang/lib/Format/ContinuationIndenter.h
@@ -433,6 +433,9 @@
   /// literal sequence, 0 otherwise.
   unsigned StartOfStringLiteral;
 
+  /// Disallow line breaks for this line.
+  bool NoLineBreak;
+
   /// A stack keeping track of properties applying to parenthesis
   /// levels.
   SmallVector Stack;
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -260,6 +260,7 @@
/*NoLineBreak=*/false));
   State.NoContinuation = false;
   State.StartOfStringLiteral = 0;
+  State.NoLineBreak = false;
   State.StartOfLineLevel = 0;
   State.LowestLevelOnLine = 0;
   State.IgnoreStackForComparison = false;
@@ -342,7 +343,7 @@
 return true;
   }
 
-  return !CurrentState.NoLineBreak;
+  return !State.NoLineBreak && !CurrentState.NoLineBreak;
 }
 
 bool ContinuationIndenter::mustBreak(const LineState ) {
@@ -653,6 +654,38 @@
   const FormatToken  = *State.NextToken->Previous;
   auto  = State.Stack.back();
 
+  bool DisallowLineBreaksOnThisLine = Style.isCpp() && [] {
+// Deal with lambda arguments in C++. The aim here 

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:389
   auto *ClosingLine = AnnotatedLines.begin() + ClosingLineIndex + 1;
   auto OutdentBy = I[J]->Level - TheLine->Level;
   for (auto *CompactedLine = I + J; CompactedLine <= ClosingLine;

See below.



Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:392-393
++CompactedLine) {
 if (!(*CompactedLine)->InPPDirective)
-  (*CompactedLine)->Level -= OutdentBy;
+  (*CompactedLine)->Level -= std::min(OutdentBy, 
(*CompactedLine)->Level);
   }

Or similar.



Comment at: clang/unittests/Format/FormatTest.cpp:4230-4237
+  verifyNoCrash("namespace ns1 { namespace ns2 { namespace ns3 {\n"
+"void func_in_ns() {\n"
+"  int res{0};\n"
+"// block for debug mode\n"
+"#ifndef NDEBUG\n"
+"#endif\n"
+"}\n"

It seems that we can reduce the test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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

[PATCH] D158505: [clang-format] Fix weird handling of AfterColon

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/unittests/Format/FormatTest.cpp:7954
+  verifyFormat(
+  "Class::Class(int some, int arguments, int loonoog,\n"
+  " int more) noexcept :\n"

`lng`?



Comment at: clang/unittests/Format/FormatTest.cpp:7994-7997
   verifyFormat("Constructor(aa ,\n"
-   "aa ) :\n"
-   "aa(aa) {}",
+   "aa ) : "
+   "aa(aa) {}",
Style);

Shorten the ctor name so that we don't have to split the line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158505

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

[PATCH] D156259: [clang-format] Fix a bug that erroneously placed function arguments on a new line despite all arguments being able to fit on the same line.

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D156259#4607177 , @jp4a50 wrote:

> Addressed all comments. Please let me know if there's anything else required 
> before merging.

There are still a couple of comments unaddressed plus another that asked for 
changing `!is()` to `isNot()`. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156259

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

[PATCH] D158571: [clang-format][NFC] Replace !is() with isNot()

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, MyDeveloperDay.
owenpan requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158571

Files:
  clang/lib/Format/ContinuationIndenter.cpp
  clang/lib/Format/DefinitionBlockSeparator.cpp
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Format/FormatTokenSource.h
  clang/lib/Format/MacroExpander.cpp
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/lib/Format/SortJavaScriptImports.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineFormatter.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/WhitespaceManager.cpp

Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -590,14 +590,14 @@
 
   // A new line starts, re-initialize line status tracking bools.
   // Keep the match state if a string literal is continued on this line.
-  if (i == 0 || !Changes[i].Tok->is(tok::string_literal) ||
-  !Changes[i - 1].Tok->is(tok::string_literal)) {
+  if (i == 0 || Changes[i].Tok->isNot(tok::string_literal) ||
+  Changes[i - 1].Tok->isNot(tok::string_literal)) {
 FoundMatchOnLine = false;
   }
   LineIsComment = true;
 }
 
-if (!Changes[i].Tok->is(tok::comment))
+if (Changes[i].Tok->isNot(tok::comment))
   LineIsComment = false;
 
 if (Changes[i].Tok->is(tok::comma)) {
@@ -731,10 +731,10 @@
   SpacesRequiredBefore = 0;
 }
 
-if (!Current || !Current->is(tok::identifier))
+if (!Current || Current->isNot(tok::identifier))
   return false;
 
-if (!Current->Previous || !Current->Previous->is(tok::pp_define))
+if (!Current->Previous || Current->Previous->isNot(tok::pp_define))
   return false;
 
 // For a macro function, 0 spaces are required between the
@@ -781,7 +781,7 @@
   LineIsComment = true;
 }
 
-if (!Changes[I].Tok->is(tok::comment))
+if (Changes[I].Tok->isNot(tok::comment))
   LineIsComment = false;
 
 if (!AlignMacrosMatches(Changes[I]))
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -394,7 +394,7 @@
 ParseDefault();
 continue;
   }
-  if (CanContainBracedList && !FormatTok->is(TT_MacroBlockBegin) &&
+  if (CanContainBracedList && FormatTok->isNot(TT_MacroBlockBegin) &&
   tryToParseBracedList()) {
 continue;
   }
@@ -615,7 +615,7 @@
   LBraceStack.pop_back();
   break;
 case tok::identifier:
-  if (!Tok->is(TT_StatementMacro))
+  if (Tok->isNot(TT_StatementMacro))
 break;
   [[fallthrough]];
 case tok::at:
@@ -799,8 +799,8 @@
   if (eof())
 return IfLBrace;
 
-  if (MacroBlock ? !FormatTok->is(TT_MacroBlockEnd)
- : !FormatTok->is(tok::r_brace)) {
+  if (MacroBlock ? FormatTok->isNot(TT_MacroBlockEnd)
+ : FormatTok->isNot(tok::r_brace)) {
 Line->Level = InitialLevel;
 FormatTok->setBlockKind(BK_Block);
 return IfLBrace;
@@ -1080,7 +1080,7 @@
   bool MaybeIncludeGuard = IfNDef;
   if (IncludeGuard == IG_Inited && MaybeIncludeGuard) {
 for (auto  : Lines) {
-  if (!Line.Tokens.front().Tok->is(tok::comment)) {
+  if (Line.Tokens.front().Tok->isNot(tok::comment)) {
 MaybeIncludeGuard = false;
 IncludeGuard = IG_Rejected;
 break;
@@ -1612,7 +1612,7 @@
 nextToken();
 if (FormatTok->is(tok::kw_public))
   nextToken();
-if (!FormatTok->is(tok::string_literal))
+if (FormatTok->isNot(tok::string_literal))
   return;
 nextToken();
 if (FormatTok->is(tok::semi))
@@ -1887,7 +1887,7 @@
   // a new unwrapped line, so they are special cased below.
   size_t TokenCount = Line->Tokens.size();
   if (Style.isJavaScript() && FormatTok->is(Keywords.kw_function) &&
-  (TokenCount > 1 || (TokenCount == 1 && !Line->Tokens.front().Tok->is(
+  (TokenCount > 1 || (TokenCount == 1 && Line->Tokens.front().Tok->isNot(
  Keywords.kw_async {
 tryToParseJSFunction();
 break;
@@ -2872,7 +2872,7 @@
   FormatTok->is(tok::l_brace)) {
 do {
   nextToken();
-} while (!FormatTok->is(tok::r_brace));
+} while (FormatTok->isNot(tok::r_brace));
 nextToken();
   }
 
@@ -2895,7 +2895,7 @@
   addUnwrappedLine();
 else
   NeedsUnwrappedLine = true;
-  } else if (!FormatTok->is(tok::kw_catch)) {
+  } else if 

[PATCH] D158293: [NFC][CLANG] Fix potential dereferencing of null return values

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2009-2010
(Line.MightBeFunctionDecl || Line.InPPDirective) &&
-   Current.NestingLevel == 0 &&
+   Current.NestingLevel == 0 && Current.Previous &&
!Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
   // not auto operator->() -> xxx;

tahonermann wrote:
> owenpan wrote:
> > `Current.Previous` can't be null here because `AutoFound` is `true`.
> Could you please elaborate on why you believe it is safe to move the check of 
> `Current.Previous` inside the body of the `if` statement? Doing so will short 
> circuit the remaining `else if` cases such that `Current.setType()` will not 
> be called at all. It isn't obvious to me that those cases should not be 
> considered if the previous token was not one of `kw_operator` or 
> `identifier`. This looks like it has potential to change behavior.
> 
> The change that was originally proposed is clearly safe.
> Could you please elaborate on why you believe it is safe to move the check of 
> `Current.Previous` inside the body of the `if` statement? Doing so will short 
> circuit the remaining `else if` cases such that `Current.setType()` will not 
> be called at all. It isn't obvious to me that those cases should not be 
> considered if the previous token was not one of `kw_operator` or 
> `identifier`. This looks like it has potential to change behavior.

Ahh, you are right.

> The change that was originally proposed is clearly safe.

My point that `Previous` can't be null still stands. So we should either make 
no changes here or add an assertion just before the `if` statement at line 1991 
above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158293

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

[PATCH] D158104: [clang-format][NFC] Simplify getFirstNonComment() in the annotator

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG07181e289e3c: [clang-format][NFC] Simplify 
getFirstNonComment() in the annotator (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158104

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h


Index: clang/lib/Format/TokenAnnotator.h
===
--- clang/lib/Format/TokenAnnotator.h
+++ clang/lib/Format/TokenAnnotator.h
@@ -151,6 +151,11 @@
startsWith(tok::kw_export, tok::kw_namespace);
   }
 
+  FormatToken *getFirstNonComment() const {
+assert(First);
+return First->is(tok::comment) ? First->getNextNonComment() : First;
+  }
+
   FormatToken *First;
   FormatToken *Last;
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4763,16 +4763,6 @@
  !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral);
 }
 
-// Returns the first token on the line that is not a comment.
-static const FormatToken *getFirstNonComment(const AnnotatedLine ) {
-  const FormatToken *Next = Line.First;
-  if (!Next)
-return Next;
-  if (Next->is(tok::comment))
-Next = Next->getNextNonComment();
-  return Next;
-}
-
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine ,
  const FormatToken ) const {
   const FormatToken  = *Right.Previous;
@@ -5044,7 +5034,7 @@
 return Right.HasUnescapedNewline;
 
   if (isAllmanBrace(Left) || isAllmanBrace(Right)) {
-auto FirstNonComment = getFirstNonComment(Line);
+auto *FirstNonComment = Line.getFirstNonComment();
 bool AccessSpecifier =
 FirstNonComment &&
 FirstNonComment->isOneOf(Keywords.kw_internal, tok::kw_public,


Index: clang/lib/Format/TokenAnnotator.h
===
--- clang/lib/Format/TokenAnnotator.h
+++ clang/lib/Format/TokenAnnotator.h
@@ -151,6 +151,11 @@
startsWith(tok::kw_export, tok::kw_namespace);
   }
 
+  FormatToken *getFirstNonComment() const {
+assert(First);
+return First->is(tok::comment) ? First->getNextNonComment() : First;
+  }
+
   FormatToken *First;
   FormatToken *Last;
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4763,16 +4763,6 @@
  !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral);
 }
 
-// Returns the first token on the line that is not a comment.
-static const FormatToken *getFirstNonComment(const AnnotatedLine ) {
-  const FormatToken *Next = Line.First;
-  if (!Next)
-return Next;
-  if (Next->is(tok::comment))
-Next = Next->getNextNonComment();
-  return Next;
-}
-
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine ,
  const FormatToken ) const {
   const FormatToken  = *Right.Previous;
@@ -5044,7 +5034,7 @@
 return Right.HasUnescapedNewline;
 
   if (isAllmanBrace(Left) || isAllmanBrace(Right)) {
-auto FirstNonComment = getFirstNonComment(Line);
+auto *FirstNonComment = Line.getFirstNonComment();
 bool AccessSpecifier =
 FirstNonComment &&
 FirstNonComment->isOneOf(Keywords.kw_internal, tok::kw_public,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158293: [NFC][CLANG] Fix potential dereferencing of null return values

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2009-2010
(Line.MightBeFunctionDecl || Line.InPPDirective) &&
-   Current.NestingLevel == 0 &&
+   Current.NestingLevel == 0 && Current.Previous &&
!Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
   // not auto operator->() -> xxx;

`Current.Previous` can't be null here because `AutoFound` is `true`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158293

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

[PATCH] D158155: [clang-format] Exclude kw_decltype in RemoveParentheses

[Owen Pan via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe3a79503a30f: [clang-format] Exclude kw_decltype in 
RemoveParentheses (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158155

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -26313,6 +26313,7 @@
 
   Style.RemoveParentheses = FormatStyle::RPS_MultipleParentheses;
   verifyFormat("int x __attribute__((aligned(16))) = 0;", Style);
+  verifyFormat("decltype((foo->bar)) baz;", Style);
   verifyFormat("class __declspec(dllimport) X {};",
"class __declspec((dllimport)) X {};", Style);
   verifyFormat("int x = (({ 0; }));", "int x = ((({ 0; })));", Style);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2472,7 +2472,7 @@
 const auto *PrevPrev = Prev ? Prev->getPreviousNonComment() : nullptr;
 const bool Blacklisted =
 PrevPrev &&
-(PrevPrev->is(tok::kw___attribute) ||
+(PrevPrev->isOneOf(tok::kw___attribute, tok::kw_decltype) ||
  (SeenEqual &&
   (PrevPrev->isOneOf(tok::kw_if, tok::kw_while) ||
PrevPrev->endsSequence(tok::kw_constexpr, tok::kw_if;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -26313,6 +26313,7 @@
 
   Style.RemoveParentheses = FormatStyle::RPS_MultipleParentheses;
   verifyFormat("int x __attribute__((aligned(16))) = 0;", Style);
+  verifyFormat("decltype((foo->bar)) baz;", Style);
   verifyFormat("class __declspec(dllimport) X {};",
"class __declspec((dllimport)) X {};", Style);
   verifyFormat("int x = (({ 0; }));", "int x = ((({ 0; })));", Style);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2472,7 +2472,7 @@
 const auto *PrevPrev = Prev ? Prev->getPreviousNonComment() : nullptr;
 const bool Blacklisted =
 PrevPrev &&
-(PrevPrev->is(tok::kw___attribute) ||
+(PrevPrev->isOneOf(tok::kw___attribute, tok::kw_decltype) ||
  (SeenEqual &&
   (PrevPrev->isOneOf(tok::kw_if, tok::kw_while) ||
PrevPrev->endsSequence(tok::kw_constexpr, tok::kw_if;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D158155: [clang-format] Exclude kw_decltype in RemoveParentheses

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, MyDeveloperDay.
owenpan requested review of this revision.

>From https://en.cppreference.com/w/cpp/language/decltype:
Note that if the name of an object is parenthesized, it is treated as an 
ordinary lvalue expression, thus `decltype(x)` and `decltype((x))` are often 
different types.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158155

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -26313,6 +26313,7 @@
 
   Style.RemoveParentheses = FormatStyle::RPS_MultipleParentheses;
   verifyFormat("int x __attribute__((aligned(16))) = 0;", Style);
+  verifyFormat("decltype((foo->bar)) baz;", Style);
   verifyFormat("class __declspec(dllimport) X {};",
"class __declspec((dllimport)) X {};", Style);
   verifyFormat("int x = (({ 0; }));", "int x = ((({ 0; })));", Style);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2472,7 +2472,7 @@
 const auto *PrevPrev = Prev ? Prev->getPreviousNonComment() : nullptr;
 const bool Blacklisted =
 PrevPrev &&
-(PrevPrev->is(tok::kw___attribute) ||
+(PrevPrev->isOneOf(tok::kw___attribute, tok::kw_decltype) ||
  (SeenEqual &&
   (PrevPrev->isOneOf(tok::kw_if, tok::kw_while) ||
PrevPrev->endsSequence(tok::kw_constexpr, tok::kw_if;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -26313,6 +26313,7 @@
 
   Style.RemoveParentheses = FormatStyle::RPS_MultipleParentheses;
   verifyFormat("int x __attribute__((aligned(16))) = 0;", Style);
+  verifyFormat("decltype((foo->bar)) baz;", Style);
   verifyFormat("class __declspec(dllimport) X {};",
"class __declspec((dllimport)) X {};", Style);
   verifyFormat("int x = (({ 0; }));", "int x = ((({ 0; })));", Style);
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2472,7 +2472,7 @@
 const auto *PrevPrev = Prev ? Prev->getPreviousNonComment() : nullptr;
 const bool Blacklisted =
 PrevPrev &&
-(PrevPrev->is(tok::kw___attribute) ||
+(PrevPrev->isOneOf(tok::kw___attribute, tok::kw_decltype) ||
  (SeenEqual &&
   (PrevPrev->isOneOf(tok::kw_if, tok::kw_while) ||
PrevPrev->endsSequence(tok::kw_constexpr, tok::kw_if;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157963: [clang-format] Annotate constructor/destructor names

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 551019.
owenpan added a comment.

Rebased to D158104  and simplified 
`getFunctionName` and `isCtorOrDtorName` a little.


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

https://reviews.llvm.org/D157963

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

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1589,6 +1589,54 @@
   Tokens = annotate("void f [[noreturn]] () {}");
   ASSERT_EQ(Tokens.size(), 12u) << Tokens;
   EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
+
+  Tokens = annotate("class Foo { public: Foo(); };");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::identifier, TT_FunctionDeclarationName);
+
+  Tokens = annotate("class Foo { public: ~Foo(); };");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::identifier, TT_FunctionDeclarationName);
+
+  Tokens = annotate("struct Foo { [[deprecated]] Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { [[deprecated]] ~Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { Foo() [[deprecated]] {} };");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { ~Foo() [[deprecated]] {} };");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { [[deprecated]] explicit Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { virtual [[deprecated]] ~Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("Foo::Foo() {}");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[5], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("Foo::~Foo() {}");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[6], tok::l_brace, TT_FunctionLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsC11GenericSelection) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16537,7 +16537,7 @@
 
   verifyFormat("int f();", SpaceFuncDef);
   verifyFormat("void f (int a, T b) {}", SpaceFuncDef);
-  verifyFormat("A::A() : a(1) {}", SpaceFuncDef);
+  verifyFormat("A::A () : a(1) {}", SpaceFuncDef);
   verifyFormat("void f() __attribute__((asdf));", SpaceFuncDef);
   verifyFormat("#define A(x) x", SpaceFuncDef);
   verifyFormat("#define A (x) x", SpaceFuncDef);
@@ -16562,7 +16562,7 @@
   // verifyFormat("T A::operator() () {}", SpaceFuncDef);
   verifyFormat("auto lambda = [] () { return 0; };", SpaceFuncDef);
   verifyFormat("int x = int(y);", SpaceFuncDef);
-  verifyFormat("M(std::size_t R, std::size_t C) : C(C), data(R) {}",
+  verifyFormat("M (std::size_t R, std::size_t C) : C(C), data(R) {}",
SpaceFuncDef);
 
   FormatStyle SpaceIfMacros = getLLVMStyle();
@@ -26220,18 +26220,18 @@
   FormatStyle Style = getLLVMStyle();
   EXPECT_EQ(Style.BreakAfterAttributes, FormatStyle::ABS_Never);
 
-  const StringRef Code("[[nodiscard]] inline int f(int );\n"
-   "[[foo([[]])]] [[nodiscard]]\n"
-   "int g(int );\n"
-   "[[nodiscard]]\n"
-   "inline int f(int ) {\n"
-   "  i = 1;\n"
-   "  return 0;\n"
-   "}\n"
-   "[[foo([[]])]] [[nodiscard]] int g(int ) {\n"
-   "  i = 0;\n"
-   "  return 1;\n"
-   "}");
+  constexpr StringRef Code("[[nodiscard]] inline int f(int );\n"
+   "[[foo([[]])]] [[nodiscard]]\n"

[PATCH] D157963: [clang-format] Annotate constructor/destructor names

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3103
+  if (!Next)
+return Next;
+  if (Next->is(tok::comment))

owenpan wrote:
> HazardyKnusperkeks wrote:
> > Is a bit clearer.
> I moved this function up to here from below and only removed a couple of 
> `const`s. I can clean it up in another patch.
See D158104.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157963

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

[PATCH] D158104: [clang-format][NFC] Simplify getFirstNonComment() in the annotator

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, MyDeveloperDay.
owenpan requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158104

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h


Index: clang/lib/Format/TokenAnnotator.h
===
--- clang/lib/Format/TokenAnnotator.h
+++ clang/lib/Format/TokenAnnotator.h
@@ -151,6 +151,11 @@
startsWith(tok::kw_export, tok::kw_namespace);
   }
 
+  FormatToken *getFirstNonComment() const {
+assert(First);
+return First->is(tok::comment) ? First->getNextNonComment() : First;
+  }
+
   FormatToken *First;
   FormatToken *Last;
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4763,16 +4763,6 @@
  !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral);
 }
 
-// Returns the first token on the line that is not a comment.
-static const FormatToken *getFirstNonComment(const AnnotatedLine ) {
-  const FormatToken *Next = Line.First;
-  if (!Next)
-return Next;
-  if (Next->is(tok::comment))
-Next = Next->getNextNonComment();
-  return Next;
-}
-
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine ,
  const FormatToken ) const {
   const FormatToken  = *Right.Previous;
@@ -5044,7 +5034,7 @@
 return Right.HasUnescapedNewline;
 
   if (isAllmanBrace(Left) || isAllmanBrace(Right)) {
-auto FirstNonComment = getFirstNonComment(Line);
+auto *FirstNonComment = Line.getFirstNonComment();
 bool AccessSpecifier =
 FirstNonComment &&
 FirstNonComment->isOneOf(Keywords.kw_internal, tok::kw_public,


Index: clang/lib/Format/TokenAnnotator.h
===
--- clang/lib/Format/TokenAnnotator.h
+++ clang/lib/Format/TokenAnnotator.h
@@ -151,6 +151,11 @@
startsWith(tok::kw_export, tok::kw_namespace);
   }
 
+  FormatToken *getFirstNonComment() const {
+assert(First);
+return First->is(tok::comment) ? First->getNextNonComment() : First;
+  }
+
   FormatToken *First;
   FormatToken *Last;
 
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -4763,16 +4763,6 @@
  !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral);
 }
 
-// Returns the first token on the line that is not a comment.
-static const FormatToken *getFirstNonComment(const AnnotatedLine ) {
-  const FormatToken *Next = Line.First;
-  if (!Next)
-return Next;
-  if (Next->is(tok::comment))
-Next = Next->getNextNonComment();
-  return Next;
-}
-
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine ,
  const FormatToken ) const {
   const FormatToken  = *Right.Previous;
@@ -5044,7 +5034,7 @@
 return Right.HasUnescapedNewline;
 
   if (isAllmanBrace(Left) || isAllmanBrace(Right)) {
-auto FirstNonComment = getFirstNonComment(Line);
+auto *FirstNonComment = Line.getFirstNonComment();
 bool AccessSpecifier =
 FirstNonComment &&
 FirstNonComment->isOneOf(Keywords.kw_internal, tok::kw_public,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157568: [clang-format] Handle NamespaceMacro string arg for FixNamespaceComments

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG063c42e919c0: [clang-format] Handle NamespaceMacro string 
arg for FixNamespaceComments (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157568

Files:
  clang/lib/Format/NamespaceEndCommentsFixer.cpp
  clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -40,6 +40,18 @@
 Code,
 /*Ranges=*/{1, tooling::Range(0, Code.size())}, Style);
   }
+
+  bool isFormatted(StringRef Code, const std::vector ,
+   const FormatStyle  = getLLVMStyle()) const {
+return clang::format::fixNamespaceEndComments(Style, Code, Ranges,
+  "")
+.empty();
+  }
+
+  bool isFormatted(StringRef Code,
+   const FormatStyle  = getLLVMStyle()) const {
+return isFormatted(Code, {1, tooling::Range(0, Code.size())}, Style);
+  }
 };
 
 TEST_F(NamespaceEndCommentsFixerTest, AddsEndComment) {
@@ -688,48 +700,34 @@
   FormatStyle Style = getLLVMStyle();
   Style.NamespaceMacros.push_back("TESTSUITE");
 
-  EXPECT_EQ("TESTSUITE() {\n"
-"int i;\n"
-"} // end anonymous TESTSUITE()",
-fixNamespaceEndComments("TESTSUITE() {\n"
-"int i;\n"
-"} // end anonymous TESTSUITE()",
-Style));
-  EXPECT_EQ("TESTSUITE(A) {\n"
-"int i;\n"
-"} /* end of TESTSUITE(A) */",
-fixNamespaceEndComments("TESTSUITE(A) {\n"
-"int i;\n"
-"} /* end of TESTSUITE(A) */",
-Style));
-  EXPECT_EQ("TESTSUITE(A) {\n"
-"int i;\n"
-"}   //   TESTSUITE(A)",
-fixNamespaceEndComments("TESTSUITE(A) {\n"
-"int i;\n"
-"}   //   TESTSUITE(A)",
-Style));
-  EXPECT_EQ("TESTSUITE(A::B) {\n"
-"int i;\n"
-"} // end TESTSUITE(A::B)",
-fixNamespaceEndComments("TESTSUITE(A::B) {\n"
-"int i;\n"
-"} // end TESTSUITE(A::B)",
-Style));
-  EXPECT_EQ("TESTSUITE(A) {\n"
-"int i;\n"
-"}; // end TESTSUITE(A)",
-fixNamespaceEndComments("TESTSUITE(A) {\n"
-"int i;\n"
-"}; // end TESTSUITE(A)",
-Style));
-  EXPECT_EQ("TESTSUITE() {\n"
-"int i;\n"
-"}; /* unnamed TESTSUITE() */",
-fixNamespaceEndComments("TESTSUITE() {\n"
-"int i;\n"
-"}; /* unnamed TESTSUITE() */",
-Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE() {\n"
+  "int i;\n"
+  "} // end anonymous TESTSUITE()",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(A) {\n"
+  "int i;\n"
+  "} /* end of TESTSUITE(A) */",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(A) {\n"
+  "int i;\n"
+  "}   //   TESTSUITE(A)",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(A::B) {\n"
+  "int i;\n"
+  "} // end TESTSUITE(A::B)",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(A) {\n"
+  "int i;\n"
+  "}; // end TESTSUITE(A)",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE() {\n"
+  "int i;\n"
+  "}; /* unnamed TESTSUITE() */",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(\"foo\") {\n"
+  "int i;\n"
+  "} // TESTSUITE(\"foo\")",
+  Style));
 }
 
 TEST_F(NamespaceEndCommentsFixerTest, UpdatesInvalidEndLineComment) {
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -174,7 +174,7 @@
   llvm::Regex::IgnoreCase);
   static const llvm::Regex 

[PATCH] D157963: [clang-format] Annotate constructor/destructor names

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3103
+  if (!Next)
+return Next;
+  if (Next->is(tok::comment))

HazardyKnusperkeks wrote:
> Is a bit clearer.
I moved this function up to here from below and only removed a couple of 
`const`s. I can clean it up in another patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157963

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

[PATCH] D157963: [clang-format] Annotate constructor/destructor names

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, MyDeveloperDay.
owenpan requested review of this revision.

Annotate constructor/destructor names as `FunctionDeclarationName`.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157963

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

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -1589,6 +1589,54 @@
   Tokens = annotate("void f [[noreturn]] () {}");
   ASSERT_EQ(Tokens.size(), 12u) << Tokens;
   EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
+
+  Tokens = annotate("class Foo { public: Foo(); };");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::identifier, TT_FunctionDeclarationName);
+
+  Tokens = annotate("class Foo { public: ~Foo(); };");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::identifier, TT_FunctionDeclarationName);
+
+  Tokens = annotate("struct Foo { [[deprecated]] Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { [[deprecated]] ~Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { Foo() [[deprecated]] {} };");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[11], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { ~Foo() [[deprecated]] {} };");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { [[deprecated]] explicit Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[12], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("struct Foo { virtual [[deprecated]] ~Foo() {} };");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[10], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("Foo::Foo() {}");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[5], tok::l_brace, TT_FunctionLBrace);
+
+  Tokens = annotate("Foo::~Foo() {}");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::identifier, TT_FunctionDeclarationName);
+  EXPECT_TOKEN(Tokens[6], tok::l_brace, TT_FunctionLBrace);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsC11GenericSelection) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -16537,7 +16537,7 @@
 
   verifyFormat("int f();", SpaceFuncDef);
   verifyFormat("void f (int a, T b) {}", SpaceFuncDef);
-  verifyFormat("A::A() : a(1) {}", SpaceFuncDef);
+  verifyFormat("A::A () : a(1) {}", SpaceFuncDef);
   verifyFormat("void f() __attribute__((asdf));", SpaceFuncDef);
   verifyFormat("#define A(x) x", SpaceFuncDef);
   verifyFormat("#define A (x) x", SpaceFuncDef);
@@ -16562,7 +16562,7 @@
   // verifyFormat("T A::operator() () {}", SpaceFuncDef);
   verifyFormat("auto lambda = [] () { return 0; };", SpaceFuncDef);
   verifyFormat("int x = int(y);", SpaceFuncDef);
-  verifyFormat("M(std::size_t R, std::size_t C) : C(C), data(R) {}",
+  verifyFormat("M (std::size_t R, std::size_t C) : C(C), data(R) {}",
SpaceFuncDef);
 
   FormatStyle SpaceIfMacros = getLLVMStyle();
@@ -26220,18 +26220,18 @@
   FormatStyle Style = getLLVMStyle();
   EXPECT_EQ(Style.BreakAfterAttributes, FormatStyle::ABS_Never);
 
-  const StringRef Code("[[nodiscard]] inline int f(int );\n"
-   "[[foo([[]])]] [[nodiscard]]\n"
-   "int g(int );\n"
-   "[[nodiscard]]\n"
-   "inline int f(int ) {\n"
-   "  i = 1;\n"
-   "  return 0;\n"
-   "}\n"
-   "[[foo([[]])]] [[nodiscard]] int g(int ) {\n"
-   "  i = 0;\n"
-   "  return 1;\n"
-  

[PATCH] D157568: [clang-format] Handle NamespaceMacro string arg for FixNamespaceComments

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:177
   llvm::Regex("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
-  "([a-zA-Z0-9_]+)\\(([a-zA-Z0-9:_]*)\\)\\.? *(\\*/)?$",
+  "([a-zA-Z0-9_]+)\\(([a-zA-Z0-9:_]*|\".+\")\\)\\.? *(\\*/)?$",
   llvm::Regex::IgnoreCase);

HazardyKnusperkeks wrote:
> Maybe this?
I don't think we'd need it because the `+` is greedy.


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

https://reviews.llvm.org/D157568

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

[PATCH] D157244: [clang-format] Correctly count annoated lines in a namespace body

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2b8542ce8e8c: [clang-format] Correctly count annoated lines 
of a namespace body (authored by owenpan).

Changed prior to commit:
  https://reviews.llvm.org/D157244?vs=547612=549190#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157244

Files:
  clang/lib/Format/NamespaceEndCommentsFixer.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp


Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -1376,6 +1376,22 @@
 "int k;\n"
 "}\n",
 Style));
+
+  // The namespace body has 5 unwrapped/annotated lines.
+  const std::string NestedLambdas{"namespace foo {\n"
+  "auto bar = [] {\n" // line 1
+  "  int i;\n"// line 2
+  "  return [] {\n"   // line 3
+  "  int j;"  // line 4
+  "  return 0;\n" // line 5
+  "  };\n"// part of line 3
+  "};\n"  // part of line 1
+  "}"};
+  Style.ShortNamespaceLines = 4;
+  EXPECT_EQ(NestedLambdas + " // namespace foo",
+fixNamespaceEndComments(NestedLambdas, Style));
+  ++Style.ShortNamespaceLines;
+  EXPECT_EQ(NestedLambdas, fixNamespaceEndComments(NestedLambdas, Style));
 }
 
 TEST_F(ShortNamespaceLinesTest, NamespaceAlias) {
Index: clang/lib/Format/TokenAnnotator.h
===
--- clang/lib/Format/TokenAnnotator.h
+++ clang/lib/Format/TokenAnnotator.h
@@ -91,6 +91,13 @@
 }
   }
 
+  size_t size() const {
+size_t Size = 1;
+for (const auto *Child : Children)
+  Size += Child->size();
+return Size;
+  }
+
   ~AnnotatedLine() {
 for (AnnotatedLine *Child : Children)
   delete Child;
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -359,8 +359,10 @@
 computeEndCommentText(NamespaceName, AddNewline, NamespaceTok,
   Style.SpacesInLineCommentPrefix.Minimum);
 if (!hasEndComment(EndCommentPrevTok)) {
-  bool isShort = I - StartLineIndex <= Style.ShortNamespaceLines + 1;
-  if (!isShort) {
+  unsigned LineCount = 0;
+  for (auto J = StartLineIndex + 1; J < I; ++J)
+LineCount += AnnotatedLines[J]->size();
+  if (LineCount > Style.ShortNamespaceLines) {
 addEndComment(EndCommentPrevTok,
   std::string(Style.SpacesBeforeTrailingComments, ' ') +
   EndCommentText,


Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -1376,6 +1376,22 @@
 "int k;\n"
 "}\n",
 Style));
+
+  // The namespace body has 5 unwrapped/annotated lines.
+  const std::string NestedLambdas{"namespace foo {\n"
+  "auto bar = [] {\n" // line 1
+  "  int i;\n"// line 2
+  "  return [] {\n"   // line 3
+  "  int j;"  // line 4
+  "  return 0;\n" // line 5
+  "  };\n"// part of line 3
+  "};\n"  // part of line 1
+  "}"};
+  Style.ShortNamespaceLines = 4;
+  EXPECT_EQ(NestedLambdas + " // namespace foo",
+fixNamespaceEndComments(NestedLambdas, Style));
+  ++Style.ShortNamespaceLines;
+  EXPECT_EQ(NestedLambdas, fixNamespaceEndComments(NestedLambdas, Style));
 }
 
 TEST_F(ShortNamespaceLinesTest, NamespaceAlias) {
Index: clang/lib/Format/TokenAnnotator.h
===
--- clang/lib/Format/TokenAnnotator.h
+++ clang/lib/Format/TokenAnnotator.h
@@ -91,6 +91,13 @@
 }
   }
 
+  size_t size() const {
+size_t Size = 1;
+for (const auto *Child : Children)
+  Size += Child->size();
+return Size;
+  }
+
   ~AnnotatedLine() {
 for 

[PATCH] D157568: [clang-format] Handle NamespaceMacro string arg for FixNamespaceComments

[Owen Pan via Phabricator via cfe-commits]

owenpan updated this revision to Diff 548878.
owenpan added a comment.

Rebased and updated test cases.


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

https://reviews.llvm.org/D157568

Files:
  clang/lib/Format/NamespaceEndCommentsFixer.cpp
  clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp

Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -40,6 +40,18 @@
 Code,
 /*Ranges=*/{1, tooling::Range(0, Code.size())}, Style);
   }
+
+  bool isFormatted(StringRef Code, const std::vector ,
+   const FormatStyle  = getLLVMStyle()) const {
+return clang::format::fixNamespaceEndComments(Style, Code, Ranges,
+  "")
+.empty();
+  }
+
+  bool isFormatted(StringRef Code,
+   const FormatStyle  = getLLVMStyle()) const {
+return isFormatted(Code, {1, tooling::Range(0, Code.size())}, Style);
+  }
 };
 
 TEST_F(NamespaceEndCommentsFixerTest, AddsEndComment) {
@@ -688,48 +700,34 @@
   FormatStyle Style = getLLVMStyle();
   Style.NamespaceMacros.push_back("TESTSUITE");
 
-  EXPECT_EQ("TESTSUITE() {\n"
-"int i;\n"
-"} // end anonymous TESTSUITE()",
-fixNamespaceEndComments("TESTSUITE() {\n"
-"int i;\n"
-"} // end anonymous TESTSUITE()",
-Style));
-  EXPECT_EQ("TESTSUITE(A) {\n"
-"int i;\n"
-"} /* end of TESTSUITE(A) */",
-fixNamespaceEndComments("TESTSUITE(A) {\n"
-"int i;\n"
-"} /* end of TESTSUITE(A) */",
-Style));
-  EXPECT_EQ("TESTSUITE(A) {\n"
-"int i;\n"
-"}   //   TESTSUITE(A)",
-fixNamespaceEndComments("TESTSUITE(A) {\n"
-"int i;\n"
-"}   //   TESTSUITE(A)",
-Style));
-  EXPECT_EQ("TESTSUITE(A::B) {\n"
-"int i;\n"
-"} // end TESTSUITE(A::B)",
-fixNamespaceEndComments("TESTSUITE(A::B) {\n"
-"int i;\n"
-"} // end TESTSUITE(A::B)",
-Style));
-  EXPECT_EQ("TESTSUITE(A) {\n"
-"int i;\n"
-"}; // end TESTSUITE(A)",
-fixNamespaceEndComments("TESTSUITE(A) {\n"
-"int i;\n"
-"}; // end TESTSUITE(A)",
-Style));
-  EXPECT_EQ("TESTSUITE() {\n"
-"int i;\n"
-"}; /* unnamed TESTSUITE() */",
-fixNamespaceEndComments("TESTSUITE() {\n"
-"int i;\n"
-"}; /* unnamed TESTSUITE() */",
-Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE() {\n"
+  "int i;\n"
+  "} // end anonymous TESTSUITE()",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(A) {\n"
+  "int i;\n"
+  "} /* end of TESTSUITE(A) */",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(A) {\n"
+  "int i;\n"
+  "}   //   TESTSUITE(A)",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(A::B) {\n"
+  "int i;\n"
+  "} // end TESTSUITE(A::B)",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(A) {\n"
+  "int i;\n"
+  "}; // end TESTSUITE(A)",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE() {\n"
+  "int i;\n"
+  "}; /* unnamed TESTSUITE() */",
+  Style));
+  EXPECT_TRUE(isFormatted("TESTSUITE(\"foo\") {\n"
+  "int i;\n"
+  "} // TESTSUITE(\"foo\")",
+  Style));
 }
 
 TEST_F(NamespaceEndCommentsFixerTest, UpdatesInvalidEndLineComment) {
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -174,7 +174,7 @@
   llvm::Regex::IgnoreCase);
   static const llvm::Regex NamespaceMacroCommentPattern =
   llvm::Regex("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
-  "([a-zA-Z0-9_]+)\\(([a-zA-Z0-9:_]*)\\)\\.? 

[PATCH] D157568: [clang-format] Handle NamespaceMacro string arg for FixNamespaceComments

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, MyDeveloperDay.
owenpan requested review of this revision.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157568

Files:
  clang/lib/Format/NamespaceEndCommentsFixer.cpp
  clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp


Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -40,8 +40,30 @@
 Code,
 /*Ranges=*/{1, tooling::Range(0, Code.size())}, Style);
   }
+
+  bool isFormatted(StringRef Code, const std::vector ,
+   const FormatStyle  = getLLVMStyle()) const {
+return clang::format::fixNamespaceEndComments(Style, Code, Ranges,
+  "")
+.empty();
+  }
+
+  bool isFormatted(StringRef Code,
+   const FormatStyle  = getLLVMStyle()) const {
+return isFormatted(Code, {1, tooling::Range(0, Code.size())}, Style);
+  }
 };
 
+TEST_F(NamespaceEndCommentsFixerTest, IsFormatted) {
+  auto Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("SUITE");
+  EXPECT_TRUE(isFormatted("SUITE(\"foo\") {\n"
+  "int i;\n"
+  "int j;\n"
+  "} // SUITE(\"foo\")",
+  Style));
+}
+
 TEST_F(NamespaceEndCommentsFixerTest, AddsEndComment) {
   EXPECT_EQ("namespace {\n"
 "int i;\n"
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -174,7 +174,8 @@
   llvm::Regex::IgnoreCase);
   static const llvm::Regex NamespaceMacroCommentPattern =
   llvm::Regex("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
-  "([a-zA-Z0-9_]+)\\(([a-zA-Z0-9:_]*)\\)\\.? *(\\*/)?$",
+  // NamespaceMacro arguments can also be string literals.
+  "([a-zA-Z0-9_]+)\\(([a-zA-Z0-9:_]*|\".+\")\\)\\.? *(\\*/)?$",
   llvm::Regex::IgnoreCase);
 
   SmallVector Groups;


Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -40,8 +40,30 @@
 Code,
 /*Ranges=*/{1, tooling::Range(0, Code.size())}, Style);
   }
+
+  bool isFormatted(StringRef Code, const std::vector ,
+   const FormatStyle  = getLLVMStyle()) const {
+return clang::format::fixNamespaceEndComments(Style, Code, Ranges,
+  "")
+.empty();
+  }
+
+  bool isFormatted(StringRef Code,
+   const FormatStyle  = getLLVMStyle()) const {
+return isFormatted(Code, {1, tooling::Range(0, Code.size())}, Style);
+  }
 };
 
+TEST_F(NamespaceEndCommentsFixerTest, IsFormatted) {
+  auto Style = getLLVMStyle();
+  Style.NamespaceMacros.push_back("SUITE");
+  EXPECT_TRUE(isFormatted("SUITE(\"foo\") {\n"
+  "int i;\n"
+  "int j;\n"
+  "} // SUITE(\"foo\")",
+  Style));
+}
+
 TEST_F(NamespaceEndCommentsFixerTest, AddsEndComment) {
   EXPECT_EQ("namespace {\n"
 "int i;\n"
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -174,7 +174,8 @@
   llvm::Regex::IgnoreCase);
   static const llvm::Regex NamespaceMacroCommentPattern =
   llvm::Regex("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *"
-  "([a-zA-Z0-9_]+)\\(([a-zA-Z0-9:_]*)\\)\\.? *(\\*/)?$",
+  // NamespaceMacro arguments can also be string literals.
+  "([a-zA-Z0-9_]+)\\(([a-zA-Z0-9:_]*|\".+\")\\)\\.? *(\\*/)?$",
   llvm::Regex::IgnoreCase);
 
   SmallVector Groups;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157244: [clang-format] Correctly count annoated lines in a namespace body

[Owen Pan via Phabricator via cfe-commits]

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.h:95
+  size_t size() const {
+int Size = 1;
+for (const auto *Child : Children)

Will fix it before landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157244

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

[PATCH] D157244: [clang-format] Correctly count annoated lines in a namespace body

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, MyDeveloperDay.
owenpan requested review of this revision.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157244

Files:
  clang/lib/Format/NamespaceEndCommentsFixer.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp


Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -1376,6 +1376,22 @@
 "int k;\n"
 "}\n",
 Style));
+
+  // The namespace body has 5 unwrapped/annotated lines.
+  const std::string NestedLambdas{"namespace foo {\n"
+  "auto bar = [] {\n" // line 1
+  "  int i;\n"// line 2
+  "  return [] {\n"   // line 3
+  "  int j;"  // line 4
+  "  return 0;\n" // line 5
+  "  };\n"// part of line 3
+  "};\n"  // part of line 1
+  "}"};
+  Style.ShortNamespaceLines = 4;
+  EXPECT_EQ(NestedLambdas + " // namespace foo",
+fixNamespaceEndComments(NestedLambdas, Style));
+  ++Style.ShortNamespaceLines;
+  EXPECT_EQ(NestedLambdas, fixNamespaceEndComments(NestedLambdas, Style));
 }
 
 TEST_F(ShortNamespaceLinesTest, NamespaceAlias) {
Index: clang/lib/Format/TokenAnnotator.h
===
--- clang/lib/Format/TokenAnnotator.h
+++ clang/lib/Format/TokenAnnotator.h
@@ -91,6 +91,13 @@
 }
   }
 
+  size_t size() const {
+int Size = 1;
+for (const auto *Child : Children)
+  Size += Child->size();
+return Size;
+  }
+
   ~AnnotatedLine() {
 for (AnnotatedLine *Child : Children)
   delete Child;
Index: clang/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- clang/lib/Format/NamespaceEndCommentsFixer.cpp
+++ clang/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -359,8 +359,10 @@
 computeEndCommentText(NamespaceName, AddNewline, NamespaceTok,
   Style.SpacesInLineCommentPrefix.Minimum);
 if (!hasEndComment(EndCommentPrevTok)) {
-  bool isShort = I - StartLineIndex <= Style.ShortNamespaceLines + 1;
-  if (!isShort) {
+  unsigned LineCount = 0;
+  for (auto J = StartLineIndex + 1; J < I; ++J)
+LineCount += AnnotatedLines[J]->size();
+  if (LineCount > Style.ShortNamespaceLines) {
 addEndComment(EndCommentPrevTok,
   std::string(Style.SpacesBeforeTrailingComments, ' ') +
   EndCommentText,


Index: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
===
--- clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
+++ clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp
@@ -1376,6 +1376,22 @@
 "int k;\n"
 "}\n",
 Style));
+
+  // The namespace body has 5 unwrapped/annotated lines.
+  const std::string NestedLambdas{"namespace foo {\n"
+  "auto bar = [] {\n" // line 1
+  "  int i;\n"// line 2
+  "  return [] {\n"   // line 3
+  "  int j;"  // line 4
+  "  return 0;\n" // line 5
+  "  };\n"// part of line 3
+  "};\n"  // part of line 1
+  "}"};
+  Style.ShortNamespaceLines = 4;
+  EXPECT_EQ(NestedLambdas + " // namespace foo",
+fixNamespaceEndComments(NestedLambdas, Style));
+  ++Style.ShortNamespaceLines;
+  EXPECT_EQ(NestedLambdas, fixNamespaceEndComments(NestedLambdas, Style));
 }
 
 TEST_F(ShortNamespaceLinesTest, NamespaceAlias) {
Index: clang/lib/Format/TokenAnnotator.h
===
--- clang/lib/Format/TokenAnnotator.h
+++ clang/lib/Format/TokenAnnotator.h
@@ -91,6 +91,13 @@
 }
   }
 
+  size_t size() const {
+int Size = 1;
+for (const auto *Child : Children)
+  Size += Child->size();
+return Size;
+  }
+
   ~AnnotatedLine() {
 for (AnnotatedLine *Child : Children)
   delete Child;
Index: 

[PATCH] D157179: [clang-format] Currectly handle PCIS_CurrentLine with no column limit

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa0b1c235afab: [clang-format] Currectly handle 
PCIS_CurrentLine with no column limit (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157179

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7758,6 +7758,13 @@
"(a),\n"
"b(b) {}",
Style);
+
+  Style = getLLVMStyleWithColumns(0);
+  Style.PackConstructorInitializers = FormatStyle::PCIS_CurrentLine;
+  verifyFormat("Foo(Bar bar, Baz baz) : bar(bar), baz(baz) {}", Style);
+  verifyNoChange("Foo(Bar bar, Baz baz)\n"
+ ": bar(bar), baz(baz) {}",
+ Style);
 }
 
 TEST_F(FormatTest, AllowAllArgumentsOnNextLine) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1437,6 +1437,7 @@
 if (Style.PackConstructorInitializers > FormatStyle::PCIS_BinPack) {
   CurrentState.AvoidBinPacking = true;
   CurrentState.BreakBeforeParameter =
+  Style.ColumnLimit > 0 &&
   Style.PackConstructorInitializers != FormatStyle::PCIS_NextLine &&
   Style.PackConstructorInitializers != FormatStyle::PCIS_NextLineOnly;
 } else {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7758,6 +7758,13 @@
"(a),\n"
"b(b) {}",
Style);
+
+  Style = getLLVMStyleWithColumns(0);
+  Style.PackConstructorInitializers = FormatStyle::PCIS_CurrentLine;
+  verifyFormat("Foo(Bar bar, Baz baz) : bar(bar), baz(baz) {}", Style);
+  verifyNoChange("Foo(Bar bar, Baz baz)\n"
+ ": bar(bar), baz(baz) {}",
+ Style);
 }
 
 TEST_F(FormatTest, AllowAllArgumentsOnNextLine) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1437,6 +1437,7 @@
 if (Style.PackConstructorInitializers > FormatStyle::PCIS_BinPack) {
   CurrentState.AvoidBinPacking = true;
   CurrentState.BreakBeforeParameter =
+  Style.ColumnLimit > 0 &&
   Style.PackConstructorInitializers != FormatStyle::PCIS_NextLine &&
   Style.PackConstructorInitializers != FormatStyle::PCIS_NextLineOnly;
 } else {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D88299: [clang-format] Add MacroUnexpander.

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.

@klimek can you have a look at 
https://github.com/llvm/llvm-project/issues/64275?




Comment at: clang/lib/Format/MacroCallReconstructor.cpp:223
+  }
+  assert(!ActiveExpansions.empty());
+  if (ActiveExpansions.back().SpelledI != ActiveExpansions.back().SpelledE) {

This fails as reported in https://github.com/llvm/llvm-project/issues/64275.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88299

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

[PATCH] D156971: [clang-format] Handle "// clang-format on" for SeparateDefinitionBlocks

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG58a71c66db85: [clang-format] Handle // clang-format 
on for SeparateDefinitionBlocks (authored by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156971

Files:
  clang/lib/Format/DefinitionBlockSeparator.cpp
  clang/unittests/Format/DefinitionBlockSeparatorTest.cpp


Index: clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
===
--- clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
+++ clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
@@ -283,6 +283,15 @@
 TEST_F(DefinitionBlockSeparatorTest, Always) {
   FormatStyle Style = getLLVMStyle();
   Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+
+  verifyFormat("// clang-format off\n"
+   "template\n"
+   "concept C = not A>;\n"
+   "// clang-format on\n"
+   "\n"
+   "struct E {};",
+   Style);
+
   std::string Prefix = "namespace {\n";
   std::string Infix = "\n"
   "// Enum test1\n"
Index: clang/lib/Format/DefinitionBlockSeparator.cpp
===
--- clang/lib/Format/DefinitionBlockSeparator.cpp
+++ clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -143,8 +143,10 @@
   if (LikelyDefinition(OperateLine))
 return false;
 
-  if (OperateLine->First->is(tok::comment))
+  if (const auto *Tok = OperateLine->First;
+  Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) {
 return true;
+  }
 
   // A single line identifier that is not in the last line.
   if (OperateLine->First->is(tok::identifier) &&


Index: clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
===
--- clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
+++ clang/unittests/Format/DefinitionBlockSeparatorTest.cpp
@@ -283,6 +283,15 @@
 TEST_F(DefinitionBlockSeparatorTest, Always) {
   FormatStyle Style = getLLVMStyle();
   Style.SeparateDefinitionBlocks = FormatStyle::SDS_Always;
+
+  verifyFormat("// clang-format off\n"
+   "template\n"
+   "concept C = not A>;\n"
+   "// clang-format on\n"
+   "\n"
+   "struct E {};",
+   Style);
+
   std::string Prefix = "namespace {\n";
   std::string Infix = "\n"
   "// Enum test1\n"
Index: clang/lib/Format/DefinitionBlockSeparator.cpp
===
--- clang/lib/Format/DefinitionBlockSeparator.cpp
+++ clang/lib/Format/DefinitionBlockSeparator.cpp
@@ -143,8 +143,10 @@
   if (LikelyDefinition(OperateLine))
 return false;
 
-  if (OperateLine->First->is(tok::comment))
+  if (const auto *Tok = OperateLine->First;
+  Tok->is(tok::comment) && !isClangFormatOn(Tok->TokenText)) {
 return true;
+  }
 
   // A single line identifier that is not in the last line.
   if (OperateLine->First->is(tok::identifier) &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157179: [clang-format] Currectly handle PCIS_CurrentLine with no column limit

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, MyDeveloperDay.
owenpan requested review of this revision.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157179

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7758,6 +7758,13 @@
"(a),\n"
"b(b) {}",
Style);
+
+  Style = getLLVMStyleWithColumns(0);
+  Style.PackConstructorInitializers = FormatStyle::PCIS_CurrentLine;
+  verifyFormat("Foo(Bar bar, Baz baz) : bar(bar), baz(baz) {}", Style);
+  verifyNoChange("Foo(Bar bar, Baz baz)\n"
+ ": bar(bar), baz(baz) {}",
+ Style);
 }
 
 TEST_F(FormatTest, AllowAllArgumentsOnNextLine) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1437,6 +1437,7 @@
 if (Style.PackConstructorInitializers > FormatStyle::PCIS_BinPack) {
   CurrentState.AvoidBinPacking = true;
   CurrentState.BreakBeforeParameter =
+  Style.ColumnLimit > 0 &&
   Style.PackConstructorInitializers != FormatStyle::PCIS_NextLine &&
   Style.PackConstructorInitializers != FormatStyle::PCIS_NextLineOnly;
 } else {


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7758,6 +7758,13 @@
"(a),\n"
"b(b) {}",
Style);
+
+  Style = getLLVMStyleWithColumns(0);
+  Style.PackConstructorInitializers = FormatStyle::PCIS_CurrentLine;
+  verifyFormat("Foo(Bar bar, Baz baz) : bar(bar), baz(baz) {}", Style);
+  verifyNoChange("Foo(Bar bar, Baz baz)\n"
+ ": bar(bar), baz(baz) {}",
+ Style);
 }
 
 TEST_F(FormatTest, AllowAllArgumentsOnNextLine) {
Index: clang/lib/Format/ContinuationIndenter.cpp
===
--- clang/lib/Format/ContinuationIndenter.cpp
+++ clang/lib/Format/ContinuationIndenter.cpp
@@ -1437,6 +1437,7 @@
 if (Style.PackConstructorInitializers > FormatStyle::PCIS_BinPack) {
   CurrentState.AvoidBinPacking = true;
   CurrentState.BreakBeforeParameter =
+  Style.ColumnLimit > 0 &&
   Style.PackConstructorInitializers != FormatStyle::PCIS_NextLine &&
   Style.PackConstructorInitializers != FormatStyle::PCIS_NextLineOnly;
 } else {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits