subject:"\[PATCH\] D141959\: \[clang\-format\] Fix inconsistent identification of operator\&"

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-02-05 Thread 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 rG35f2ac1763ad: [clang-format] Fix inconsistent annotation of 
operator (authored by dkt01, committed by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141959

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  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
@@ -175,6 +175,73 @@
   ASSERT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("Type1  = val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("Type1 *val1 = ");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::star, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_UnaryOperator);
+
+  Tokens = annotate("val1 & val2;");
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2.member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2.*member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1.*member & val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2->*member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1->member & val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 & val3;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 // comment\n"
+" & val3;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_BinaryOperator);
+
+  Tokens =
+  annotate("val1 & val2.member & val3.member() & val4 & val5->member;");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[5], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[11], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[13], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("class c {\n"
+"  void func(type ) { a & member; }\n"
+"  anotherType \n"
+"}");
+  ASSERT_EQ(Tokens.size(), 22u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[12], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[17], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("struct S {\n"
+"  auto Mem = C & D;\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11288,6 +11288,13 @@
 
   verifyFormat("int operator()(T (&&)[N]) { return 1; }");
   verifyFormat("int operator()(T (&)[N]) { return 0; }");
+
+  verifyFormat("val1 & val2;");
+  verifyFormat("val1 & val2 & val3;");
+  verifyFormat("class c {\n"
+   "  void func(type ) { a & member; }\n"
+   "  anotherType \n"
+   "}");
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {
Index: clang/lib/Format/TokenAnnotator.h
===
--- clang/lib/Format/TokenAnnotator.h
+++ clang/lib/Format/TokenAnnotator.h
@@ -34,6 +34,15 @@
   LT_CommentAbovePPDirective,
 };
 
+enum ScopeType {
+  // Contained in class declaration/definition.
+  ST_Class,
+  // Contained within function definition.
+  ST_Function,
+  // Contained within other scope block (loop, if/else, etc).
+  ST_Other,
+};
+
 class AnnotatedLine {
 public:
   AnnotatedLine(const UnwrappedLine )
@@ -178,7 +187,7 @@
   // FIXME: Can/should this be done in the UnwrappedLineParser?
   void setCommentLineLevels(SmallVectorImpl ) const;
 
-  void

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-02-05 Thread David K Turner via Phabricator via cfe-commits

dkt01 updated this revision to Diff 494928.
dkt01 marked an inline comment as done.
dkt01 added a comment.

Changes requested in owenpan's review.
Rebased changes onto latest main branch.


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

https://reviews.llvm.org/D141959

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  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
@@ -175,6 +175,73 @@
   ASSERT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("Type1  = val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("Type1 *val1 = ");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::star, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_UnaryOperator);
+
+  Tokens = annotate("val1 & val2;");
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2.member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2.*member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1.*member & val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2->*member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1->member & val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 & val3;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 // comment\n"
+" & val3;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_BinaryOperator);
+
+  Tokens =
+  annotate("val1 & val2.member & val3.member() & val4 & val5->member;");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[5], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[11], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[13], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("class c {\n"
+"  void func(type ) { a & member; }\n"
+"  anotherType \n"
+"}");
+  ASSERT_EQ(Tokens.size(), 22u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[12], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[17], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("struct S {\n"
+"  auto Mem = C & D;\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11288,6 +11288,13 @@
 
   verifyFormat("int operator()(T (&&)[N]) { return 1; }");
   verifyFormat("int operator()(T (&)[N]) { return 0; }");
+
+  verifyFormat("val1 & val2;");
+  verifyFormat("val1 & val2 & val3;");
+  verifyFormat("class c {\n"
+   "  void func(type ) { a & member; }\n"
+   "  anotherType \n"
+   "}");
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {
Index: clang/lib/Format/TokenAnnotator.h
===
--- clang/lib/Format/TokenAnnotator.h
+++ clang/lib/Format/TokenAnnotator.h
@@ -34,6 +34,15 @@
   LT_CommentAbovePPDirective,
 };
 
+enum ScopeType {
+  // Contained in class declaration/definition.
+  ST_Class,
+  // Contained within function definition.
+  ST_Function,
+  // Contained within other scope block (loop, if/else, etc).
+  ST_Other,
+};
+
 class AnnotatedLine {
 public:
   AnnotatedLine(const UnwrappedLine )
@@ -178,7 +187,7 @@
   // FIXME: Can/should this be done in the UnwrappedLineParser?
   void setCommentLineLevels(SmallVectorImpl ) const;
 
-  void annotate(AnnotatedLine ) const;
+  void annotate(AnnotatedLine );
   void calculateFormattingInformation(AnnotatedLine

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-02-05 Thread David K Turner via Phabricator via cfe-commits

dkt01 marked 4 inline comments as done.
dkt01 added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:123
 private:
+  ScopeType getScopeType(FormatToken ) {
+switch (Token.getType()) {

owenpan wrote:
> As suggested before.
Oops... missed the `const`s last time...


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-02-04 Thread Owen Pan via Phabricator via cfe-commits

owenpan accepted this revision.
owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:123
 private:
+  ScopeType getScopeType(FormatToken ) {
+switch (Token.getType()) {

As suggested before.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1195-1198
+  // Handle unbalanced braces.
+  if (!Scopes.empty())
+Scopes.pop_back();
   // Lines can start with '}'.

dkt01 wrote:
> owenpan wrote:
> > dkt01 wrote:
> > > owenpan wrote:
> > > > I don't think it's about unbalanced braces here.
> > > `if (!Scopes.empty())` handles unbalanced braces.  `if(Tok->Previous)` 
> > > handles the case where a line starts with an rbrace.
> > I can't think of an example. Do you have one?
> The format unit test `FormatUnbalancedStructuralElements` feeds in strings 
> that have only right braces, so without this check the pop fails.
Thanks!


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-02-01 Thread David K Turner via Phabricator via cfe-commits

dkt01 updated this revision to Diff 494059.
dkt01 added a comment.

Updates recommended by owenpan in review.


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

https://reviews.llvm.org/D141959

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  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
@@ -175,6 +175,73 @@
   ASSERT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("Type1  = val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("Type1 *val1 = ");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::star, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_UnaryOperator);
+
+  Tokens = annotate("val1 & val2;");
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2.member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2.*member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1.*member & val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2->*member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1->member & val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 & val3;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 // comment\n"
+" & val3;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_BinaryOperator);
+
+  Tokens =
+  annotate("val1 & val2.member & val3.member() & val4 & val5->member;");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[5], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[11], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[13], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("class c {\n"
+"  void func(type ) { a & member; }\n"
+"  anotherType \n"
+"}");
+  ASSERT_EQ(Tokens.size(), 22u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[12], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[17], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("struct S {\n"
+"  auto Mem = C & D;\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11267,6 +11267,13 @@
 
   verifyFormat("int operator()(T (&&)[N]) { return 1; }");
   verifyFormat("int operator()(T (&)[N]) { return 0; }");
+
+  verifyFormat("val1 & val2;");
+  verifyFormat("val1 & val2 & val3;");
+  verifyFormat("class c {\n"
+   "  void func(type ) { a & member; }\n"
+   "  anotherType \n"
+   "}");
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {
Index: clang/lib/Format/TokenAnnotator.h
===
--- clang/lib/Format/TokenAnnotator.h
+++ clang/lib/Format/TokenAnnotator.h
@@ -34,6 +34,15 @@
   LT_CommentAbovePPDirective,
 };
 
+enum ScopeType {
+  // Contained in class declaration/definition.
+  ST_Class,
+  // Contained within function definition.
+  ST_Function,
+  // Contained within other scope block (loop, if/else, etc).
+  ST_Other,
+};
+
 class AnnotatedLine {
 public:
   AnnotatedLine(const UnwrappedLine )
@@ -178,7 +187,7 @@
   // FIXME: Can/should this be done in the UnwrappedLineParser?
   void setCommentLineLevels(SmallVectorImpl ) const;
 
-  void annotate(AnnotatedLine ) const;
+  void annotate(AnnotatedLine );
   void calculateFormattingInformation(AnnotatedLine ) const;
 
 private:
@@ -220,6 +229,8 @@
   const FormatStyle 
 
   const

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-02-01 Thread David K Turner via Phabricator via cfe-commits

dkt01 marked 8 inline comments as done.
dkt01 added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1195-1198
+  // Handle unbalanced braces.
+  if (!Scopes.empty())
+Scopes.pop_back();
   // Lines can start with '}'.

owenpan wrote:
> dkt01 wrote:
> > owenpan wrote:
> > > I don't think it's about unbalanced braces here.
> > `if (!Scopes.empty())` handles unbalanced braces.  `if(Tok->Previous)` 
> > handles the case where a line starts with an rbrace.
> I can't think of an example. Do you have one?
The format unit test `FormatUnbalancedStructuralElements` feeds in strings that 
have only right braces, so without this check the pop fails.


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-02-01 Thread Owen Pan via Phabricator via cfe-commits

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:123-125
+  bool braceTokenMatchesScope(FormatToken ) {
+if (!Token.is(tok::l_brace) || Scopes.empty())
+  return false;





Comment at: clang/lib/Format/TokenAnnotator.cpp:129
+case TT_LambdaLBrace:
+  return Scopes.back() == ST_Function;
+case TT_ClassLBrace:





Comment at: clang/lib/Format/TokenAnnotator.cpp:868
+assert(!Scopes.empty());
+assert(braceTokenMatchesScope(OpeningBrace));
+Scopes.pop_back();





Comment at: clang/lib/Format/TokenAnnotator.cpp:1169-1181
+  switch (Tok->getType()) {
+  case TT_FunctionLBrace:
+  case TT_LambdaLBrace:
+Scopes.push_back(ST_Function);
+break;
+  case TT_ClassLBrace:
+  case TT_StructLBrace:





Comment at: clang/lib/Format/TokenAnnotator.cpp:2491
+auto IsChainedOperatorAmpOrMember = [](const FormatToken *token) {
+  return token && token->isOneOf(tok::amp, tok::period, tok::arrow,
+ tok::arrowstar, tok::periodstar);

Sorry about that!



Comment at: clang/lib/Format/TokenAnnotator.cpp:2498-2499
+if (Tok.is(tok::amp) && PrevToken && PrevToken->Tok.isAnyIdentifier() &&
+(!PrevToken->getPreviousNonComment() ||
+ IsChainedOperatorAmpOrMember(PrevToken->getPreviousNonComment())) &&
+NextToken && NextToken->Tok.isAnyIdentifier()) {

Now we only need the lambda.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1195-1198
+  // Handle unbalanced braces.
+  if (!Scopes.empty())
+Scopes.pop_back();
   // Lines can start with '}'.

dkt01 wrote:
> owenpan wrote:
> > I don't think it's about unbalanced braces here.
> `if (!Scopes.empty())` handles unbalanced braces.  `if(Tok->Previous)` 
> handles the case where a line starts with an rbrace.
I can't think of an example. Do you have one?



Comment at: clang/lib/Format/TokenAnnotator.cpp:2481-2482
+if (Tok.is(tok::amp) && (PrevToken && PrevToken->Tok.isAnyIdentifier()) &&
+(!PrevToken->getPreviousNonComment() ||
+ IsChainedOperatorAmpOrMember(PrevToken->getPreviousNonComment())) &&
+(NextToken && NextToken->Tok.isAnyIdentifier()) &&

dkt01 wrote:
> owenpan wrote:
> > The lambda would check that `PrevToken` is nonnull.
> `!PrevToken->getPreviousNonComment() ||` is a different check than the null 
> check in the lambda.  It's acceptable to have no token two tokens prior 
> because that indicates the ampersand is the second token of the line.
I actually wanted fold this into the lambda. (See above.)


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-29 Thread David K Turner via Phabricator via cfe-commits

dkt01 updated this revision to Diff 493104.
dkt01 added a comment.

Updates suggested in owenpan's review.


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

https://reviews.llvm.org/D141959

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  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
@@ -175,6 +175,73 @@
   ASSERT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("Type1  = val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("Type1 *val1 = ");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::star, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_UnaryOperator);
+
+  Tokens = annotate("val1 & val2;");
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2.member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2.*member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1.*member & val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2->*member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1->member & val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 & val3;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 // comment\n"
+" & val3;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_BinaryOperator);
+
+  Tokens =
+  annotate("val1 & val2.member & val3.member() & val4 & val5->member;");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[5], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[11], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[13], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("class c {\n"
+"  void func(type ) { a & member; }\n"
+"  anotherType \n"
+"}");
+  ASSERT_EQ(Tokens.size(), 22u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[12], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[17], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("struct S {\n"
+"  auto Mem = C & D;\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11267,6 +11267,13 @@
 
   verifyFormat("int operator()(T (&&)[N]) { return 1; }");
   verifyFormat("int operator()(T (&)[N]) { return 0; }");
+
+  verifyFormat("val1 & val2;");
+  verifyFormat("val1 & val2 & val3;");
+  verifyFormat("class c {\n"
+   "  void func(type ) { a & member; }\n"
+   "  anotherType \n"
+   "}");
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {
Index: clang/lib/Format/TokenAnnotator.h
===
--- clang/lib/Format/TokenAnnotator.h
+++ clang/lib/Format/TokenAnnotator.h
@@ -34,6 +34,15 @@
   LT_CommentAbovePPDirective,
 };
 
+enum ScopeType {
+  // Contained in class declaration/definition.
+  ST_Class,
+  // Contained within function definition.
+  ST_Function,
+  // Contained within other scope block (loop, if/else, etc).
+  ST_Other,
+};
+
 class AnnotatedLine {
 public:
   AnnotatedLine(const UnwrappedLine )
@@ -178,7 +187,7 @@
   // FIXME: Can/should this be done in the UnwrappedLineParser?
   void setCommentLineLevels(SmallVectorImpl ) const;
 
-  void annotate(AnnotatedLine ) const;
+  void annotate(AnnotatedLine );
   void calculateFormattingInformation(AnnotatedLine ) const;
 
 private:
@@ -220,6 +229,8 @@
   const FormatStyle 
 
   const

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-29 Thread David K Turner via Phabricator via cfe-commits

dkt01 marked 11 inline comments as done.
dkt01 added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1195-1198
+  // Handle unbalanced braces.
+  if (!Scopes.empty())
+Scopes.pop_back();
   // Lines can start with '}'.

owenpan wrote:
> I don't think it's about unbalanced braces here.
`if (!Scopes.empty())` handles unbalanced braces.  `if(Tok->Previous)` handles 
the case where a line starts with an rbrace.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2481-2482
+if (Tok.is(tok::amp) && (PrevToken && PrevToken->Tok.isAnyIdentifier()) &&
+(!PrevToken->getPreviousNonComment() ||
+ IsChainedOperatorAmpOrMember(PrevToken->getPreviousNonComment())) &&
+(NextToken && NextToken->Tok.isAnyIdentifier()) &&

owenpan wrote:
> The lambda would check that `PrevToken` is nonnull.
`!PrevToken->getPreviousNonComment() ||` is a different check than the null 
check in the lambda.  It's acceptable to have no token two tokens prior because 
that indicates the ampersand is the second token of the line.


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-27 Thread Owen Pan via Phabricator via cfe-commits

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2488-2490
+(NextToken && NextToken->Tok.isAnyIdentifier()) &&
+(NextToken->getNextNonComment() &&
+ (NextToken->getNextNonComment()->isOneOf(

dkt01 wrote:
> owenpan wrote:
> > `NextToken` is guarded against null on line 2488 but not on lines 2489-2490.
> If NextToken is null on 2488, 2489-2490 will be short circuited.
You are right. The redundant parens threw me off.



Comment at: clang/lib/Format/TokenAnnotator.cpp:115-117
+   SmallVector )
   : Style(Style), Line(Line), CurrentToken(Line.First), AutoFound(false),
+Keywords(Keywords), Scopes(TrackedScopes) {

Something like the above. (I prefer `Scopes` to `TrackedScopes` to be 
consistent with the naming of the other parameters.)



Comment at: clang/lib/Format/TokenAnnotator.cpp:1195-1198
+  // Handle unbalanced braces.
+  if (!Scopes.empty())
+Scopes.pop_back();
   // Lines can start with '}'.

I don't think it's about unbalanced braces here.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2474-2475
+auto IsChainedOperatorAmpOrMember = [](const FormatToken *token) {
+  return token->isOneOf(tok::amp, tok::period, tok::arrow, tok::arrowstar,
+tok::periodstar);
+};

To help simplify the `if` statement below.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2480
+// reference.
+if (Tok.is(tok::amp) && (PrevToken && PrevToken->Tok.isAnyIdentifier()) &&
+(!PrevToken->getPreviousNonComment() ||

Redundant parens.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2481-2482
+if (Tok.is(tok::amp) && (PrevToken && PrevToken->Tok.isAnyIdentifier()) &&
+(!PrevToken->getPreviousNonComment() ||
+ IsChainedOperatorAmpOrMember(PrevToken->getPreviousNonComment())) &&
+(NextToken && NextToken->Tok.isAnyIdentifier()) &&

The lambda would check that `PrevToken` is nonnull.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2483-2487
+(NextToken && NextToken->Tok.isAnyIdentifier()) &&
+(NextToken->getNextNonComment() &&
+ (IsChainedOperatorAmpOrMember(NextToken->getNextNonComment()) ||
+  NextToken->getNextNonComment()->is(tok::semi {
+  return TT_BinaryOperator;

Something like that.



Comment at: clang/lib/Format/TokenAnnotator.h:36
 };
 
 class AnnotatedLine {

Consider moving `ScopeType` out of `TokenAnnotator` and keeping it consistent 
with the style of `LineType` above (and replacing `TokenAnnotator::ScopeType` 
with `ScopeType` everywhere).



Comment at: clang/lib/Format/TokenAnnotator.h:184-191
+  enum class ScopeType : int8_t {
+// Contained in class declaration/definition.
+Class,
+// Contained within function definition.
+Function,
+// Contained within other scope block (loop, if/else, etc).
+Other,

See above.


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-26 Thread David K Turner via Phabricator via cfe-commits

dkt01 updated this revision to Diff 492482.
dkt01 marked an inline comment as done.
dkt01 added a comment.

Changes as suggested in review from owenpan


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

https://reviews.llvm.org/D141959

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  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
@@ -175,6 +175,73 @@
   ASSERT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("Type1  = val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("Type1 *val1 = ");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::star, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_UnaryOperator);
+
+  Tokens = annotate("val1 & val2;");
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2.member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2.*member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1.*member & val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2->*member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1->member & val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 & val3;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 // comment\n"
+" & val3;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_BinaryOperator);
+
+  Tokens =
+  annotate("val1 & val2.member & val3.member() & val4 & val5->member;");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[5], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[11], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[13], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("class c {\n"
+"  void func(type ) { a & member; }\n"
+"  anotherType \n"
+"}");
+  ASSERT_EQ(Tokens.size(), 22u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[12], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[17], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("struct S {\n"
+"  auto Mem = C & D;\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11267,6 +11267,13 @@
 
   verifyFormat("int operator()(T (&&)[N]) { return 1; }");
   verifyFormat("int operator()(T (&)[N]) { return 0; }");
+
+  verifyFormat("val1 & val2;");
+  verifyFormat("val1 & val2 & val3;");
+  verifyFormat("class c {\n"
+   "  void func(type ) { a & member; }\n"
+   "  anotherType \n"
+   "}");
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {
Index: clang/lib/Format/TokenAnnotator.h
===
--- clang/lib/Format/TokenAnnotator.h
+++ clang/lib/Format/TokenAnnotator.h
@@ -178,9 +178,18 @@
   // FIXME: Can/should this be done in the UnwrappedLineParser?
   void setCommentLineLevels(SmallVectorImpl ) const;
 
-  void annotate(AnnotatedLine ) const;
+  void annotate(AnnotatedLine );
   void calculateFormattingInformation(AnnotatedLine ) const;
 
+  enum class ScopeType : int8_t {
+// Contained in class declaration/definition.
+Class,
+// Contained within function definition.
+Function,
+// Contained within other scope block (loop, if/else, etc).
+Other,
+  };
+
 private:
   /// Calculate the penalty for splitting before \c Tok.
   unsigned splitPenalty(const AnnotatedLine , const

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-26 Thread David K Turner via Phabricator via cfe-commits

dkt01 marked 11 inline comments as done.
dkt01 added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1162-1169
+  case TT_EnumLBrace:
+  case TT_ControlStatementLBrace:
+  case TT_ElseLBrace:
+  case TT_BracedListLBrace:
+  case TT_CompoundRequirementLBrace:
+  case TT_ObjCBlockLBrace:
+  case TT_RecordLBrace:

owenpan wrote:
> Do we really need these? If we add other special `l_brace` token types, we 
> will have to update this list.
Agreed.  This was left over from a prior implementation that didn't have a 
default case.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2488-2490
+(NextToken && NextToken->Tok.isAnyIdentifier()) &&
+(NextToken->getNextNonComment() &&
+ (NextToken->getNextNonComment()->isOneOf(

owenpan wrote:
> `NextToken` is guarded against null on line 2488 but not on lines 2489-2490.
If NextToken is null on 2488, 2489-2490 will be short circuited.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2766
 
-void TokenAnnotator::annotate(AnnotatedLine ) const {
   for (auto  : Line.Children)

MyDeveloperDay wrote:
> was there a reason it could no longer be const?
Member `Scopes` can be modified.  Alternate would be to make `Scopes` mutable, 
but I thought removing const from this function is cleaner


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-26 Thread Owen Pan via Phabricator via cfe-commits

owenpan added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:116-117
+   SmallVector )
+  : Scopes(TrackedScopes), Style(Style), Line(Line),
+CurrentToken(Line.First), AutoFound(false), Keywords(Keywords) {
 Contexts.push_back(Context(tok::unknown, 1, /*IsExpression=*/false));

Please keep them in the same order as that of the parameters above.



Comment at: clang/lib/Format/TokenAnnotator.cpp:852
+assert(Scopes.size() > 1);
+Scopes.pop_back();
 assert(OpeningBrace.Optional == CurrentToken->Optional);

Should we assert that the token type of `OpeningBrace` matches the `ScopeType` 
before popping?



Comment at: clang/lib/Format/TokenAnnotator.cpp:1162-1169
+  case TT_EnumLBrace:
+  case TT_ControlStatementLBrace:
+  case TT_ElseLBrace:
+  case TT_BracedListLBrace:
+  case TT_CompoundRequirementLBrace:
+  case TT_ObjCBlockLBrace:
+  case TT_RecordLBrace:

Do we really need these? If we add other special `l_brace` token types, we will 
have to update this list.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1207
+  }
   // Lines can start with '}'.
   if (Tok->Previous)

Can you leave the comment at the top of the `case`? If we get rid of `None` for 
`ScopeType`, we don't need to pop here. (See below.)



Comment at: clang/lib/Format/TokenAnnotator.cpp:2485-2487
+ PrevToken->getPreviousNonComment()->isOneOf(tok::amp, tok::period,
+ tok::arrow, 
tok::arrowstar,
+ tok::periodstar)) &&

Can you add a lambda for this so that it can be used below?



Comment at: clang/lib/Format/TokenAnnotator.cpp:2488-2490
+(NextToken && NextToken->Tok.isAnyIdentifier()) &&
+(NextToken->getNextNonComment() &&
+ (NextToken->getNextNonComment()->isOneOf(

`NextToken` is guarded against null on line 2488 but not on lines 2489-2490.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2491-2492
+ (NextToken->getNextNonComment()->isOneOf(
+ tok::amp, tok::semi, tok::period, tok::arrow, tok::arrowstar,
+ tok::periodstar {
+  return TT_BinaryOperator;

Here we can use the lambda suggested above.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2778
+: Style(Style), Keywords(Keywords) {
+  Scopes.push_back(ScopeType::None);
+}

Instead of pushing `None`, would it better to leave `Scopes` empty? Then we 
would not need `None` for the `ScopeType`, and instead of checking for 
`Scopes.size() > 1`, we could check if it's empty().



Comment at: clang/lib/Format/TokenAnnotator.h:183
 
+  enum class ScopeType : std::int8_t {
+// Not contained within scope block.

Drop `std::`.



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:192-194
+  Tokens = annotate("val1 & val2.member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);

Can you add a case for `periodstar`?



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:207-213
+  Tokens =
+  annotate("val1 & val2.member & val3.member() & val4 & val5->member;");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[5], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[11], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[13], tok::amp, TT_BinaryOperator);

Can you add a case for `arrowstar`?


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-26 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.



> Good to know it worked as expected on your project as well.

I have some concerns (mainly because it feels like it could be quite invasive), 
hence I'm delaying on the LGTM, I really want to run this on a large code base, 
which I think if we commit post the branch (which is now branched) we would 
probably see more easily. Historically in the past I think the firefox team 
have kindly picked up recent builds (no obligations) and catch any regressions.




Comment at: clang/lib/Format/TokenAnnotator.cpp:2766
 
-void TokenAnnotator::annotate(AnnotatedLine ) const {
   for (auto  : Line.Children)

was there a reason it could no longer be const?


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-25 Thread David K Turner via Phabricator via cfe-commits

dkt01 added a comment.

David Turner 

Good to know it worked as expected on your project as well.


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-25 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

I think they have branched 
(https://clang.llvm.org/docs/ClangFormatStyleOptions.html) the documentation 
has changed to 17.0

We would need your name and email in order to commit on your behalf.

I checked this patch out and tested it on my large project and I didn't see 
anything immediately jump out.

@owenpan, @rymiel  any concerns?


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-24 Thread David K Turner via Phabricator via cfe-commits

dkt01 added a comment.

Sounds good.  I don't believe I have permissions to add commits to three repo 
anyway, so someone can add on my behalf after the 16 branch.


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-24 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

There is part of me that feels we should not rush this in to get it into 16 
(which was due to branch today), I feel it might be a good idea to commit this 
after branching, and let it live in 'main' so we can take for a real test drive


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-24 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a reviewer: rymiel.
HazardyKnusperkeks added a comment.
This revision is now accepted and ready to land.

Looks good to me, but please wait until another one has given their blessing, 
or at least no veto for a few days.


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-24 Thread David K Turner via Phabricator via cfe-commits

dkt01 updated this revision to Diff 491823.
dkt01 added a comment.

Changes recommended by HazardyKnusperkeks in review.


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

https://reviews.llvm.org/D141959

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  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
@@ -175,6 +175,57 @@
   ASSERT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("Type1  = val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("Type1 *val1 = ");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::star, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_UnaryOperator);
+
+  Tokens = annotate("val1 & val2;");
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2.member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 & val3;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 // comment\n"
+" & val3;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_BinaryOperator);
+
+  Tokens =
+  annotate("val1 & val2.member & val3.member() & val4 & val5->member;");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[5], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[11], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[13], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("class c {\n"
+"  void func(type ) { a & member; }\n"
+"  anotherType \n"
+"}");
+  ASSERT_EQ(Tokens.size(), 22u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[12], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[17], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("struct S {\n"
+"  auto Mem = C & D;\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11267,6 +11267,13 @@
 
   verifyFormat("int operator()(T (&&)[N]) { return 1; }");
   verifyFormat("int operator()(T (&)[N]) { return 0; }");
+
+  verifyFormat("val1 & val2;");
+  verifyFormat("val1 & val2 & val3;");
+  verifyFormat("class c {\n"
+   "  void func(type ) { a & member; }\n"
+   "  anotherType \n"
+   "}");
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {
Index: clang/lib/Format/TokenAnnotator.h
===
--- clang/lib/Format/TokenAnnotator.h
+++ clang/lib/Format/TokenAnnotator.h
@@ -170,17 +170,27 @@
 /// \c UnwrappedLine.
 class TokenAnnotator {
 public:
-  TokenAnnotator(const FormatStyle , const AdditionalKeywords )
-  : Style(Style), Keywords(Keywords) {}
+  TokenAnnotator(const FormatStyle , const AdditionalKeywords );
 
   /// Adapts the indent levels of comment lines to the indent of the
   /// subsequent line.
   // FIXME: Can/should this be done in the UnwrappedLineParser?
   void setCommentLineLevels(SmallVectorImpl ) const;
 
-  void annotate(AnnotatedLine ) const;
+  void annotate(AnnotatedLine );
   void calculateFormattingInformation(AnnotatedLine ) const;
 
+  enum class ScopeType : std::int8_t {
+// Not contained within scope block.
+None,
+// Contained in class declaration/definition.
+Class,
+// Contained within function definition.
+Function,
+// Contained within other scope block (loop, if/else, etc).
+Other,
+  };
+
 private:
   /// Calculate the penalty for splitting before \c Tok.
   unsigned splitPenalty(const AnnotatedLine , const FormatToken ,
@@ -220,6 +230,8 @@
   const FormatStyle 
 
   const AdditionalKeywords 
+
+  SmallVector Scopes;
 };
 
 } // end namespace format
Index: clang/lib/Format/TokenAnnotator.cpp

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-23 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:851
   if (CurrentToken->is(tok::r_brace)) {
+// Handle unbalanced braces
+if (Scopes.size() > 1)

See below



Comment at: clang/lib/Format/TokenAnnotator.cpp:852
+// Handle unbalanced braces
+if (Scopes.size() > 1)
+  Scopes.pop_back();

Is this check also necessary? Here we should only be when we have pushed before.
I'd change the if for an assert.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1204
 case tok::r_brace:
+  // Handle unbalanced braces
+  if (Scopes.size() > 1)

Move the comment in the if? In general it hasn't to be a unbalanced brace, 
right?


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-20 Thread David K Turner via Phabricator via cfe-commits

dkt01 updated this revision to Diff 490859.
dkt01 added a comment.

Changes as suggested by HazardyKnusperkeks in review.


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

https://reviews.llvm.org/D141959

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  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
@@ -175,6 +175,57 @@
   ASSERT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("Type1  = val2;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("Type1 *val1 = ");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::star, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_UnaryOperator);
+
+  Tokens = annotate("val1 & val2;");
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2.member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 & val3;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 // comment\n"
+" & val3;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_BinaryOperator);
+
+  Tokens =
+  annotate("val1 & val2.member & val3.member() & val4 & val5->member;");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[5], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[11], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[13], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("class c {\n"
+"  void func(type ) { a & member; }\n"
+"  anotherType \n"
+"}");
+  ASSERT_EQ(Tokens.size(), 22u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[12], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[17], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("struct S {\n"
+"  auto Mem = C & D;\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11267,6 +11267,13 @@
 
   verifyFormat("int operator()(T (&&)[N]) { return 1; }");
   verifyFormat("int operator()(T (&)[N]) { return 0; }");
+
+  verifyFormat("val1 & val2;");
+  verifyFormat("val1 & val2 & val3;");
+  verifyFormat("class c {\n"
+   "  void func(type ) { a & member; }\n"
+   "  anotherType \n"
+   "}");
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {
Index: clang/lib/Format/TokenAnnotator.h
===
--- clang/lib/Format/TokenAnnotator.h
+++ clang/lib/Format/TokenAnnotator.h
@@ -170,17 +170,27 @@
 /// \c UnwrappedLine.
 class TokenAnnotator {
 public:
-  TokenAnnotator(const FormatStyle , const AdditionalKeywords )
-  : Style(Style), Keywords(Keywords) {}
+  TokenAnnotator(const FormatStyle , const AdditionalKeywords );
 
   /// Adapts the indent levels of comment lines to the indent of the
   /// subsequent line.
   // FIXME: Can/should this be done in the UnwrappedLineParser?
   void setCommentLineLevels(SmallVectorImpl ) const;
 
-  void annotate(AnnotatedLine ) const;
+  void annotate(AnnotatedLine );
   void calculateFormattingInformation(AnnotatedLine ) const;
 
+  enum class ScopeType : std::int8_t {
+// Not contained within scope block.
+None,
+// Contained in class declaration/definition.
+Class,
+// Contained within function definition.
+Function,
+// Contained within other scope block (loop, if/else, etc).
+Other,
+  };
+
 private:
   /// Calculate the penalty for splitting before \c Tok.
   unsigned splitPenalty(const AnnotatedLine , const FormatToken ,
@@ -220,6 +230,8 @@
   const FormatStyle 
 
   const AdditionalKeywords 
+
+  SmallVector Scopes;
 };
 
 } // end namespace format
Index: clang/lib/Format/TokenAnnotator.cpp

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-20 Thread David K Turner via Phabricator via cfe-commits

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



Comment at: clang/lib/Format/TokenAnnotator.cpp:2544
   SmallVector Contexts;
+  static SmallVector Scopes;
 

HazardyKnusperkeks wrote:
> dkt01 wrote:
> > HazardyKnusperkeks wrote:
> > > Why static?
> > The AnnotatingParser object lifetime is too short to track the scope 
> > information I wanted.  From my testing, it seems like this object is 
> > created once per line.  This is why I haven't added the scope information 
> > as part of the contexts member.
> > 
> > I would like to make this a non-static member, but I'm not sure which 
> > object it should be a member of.  Do you have a recommendation?
> The `TokenAnnotator`? And then by reference to the parser.
I was over-thinking it...  Adding to `TokenAnnotator` works great!
I did have to re-introduce the size checks before popping scopes because 
otherwise the UnbalancedStructuralElements test fails.



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:181
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+

HazardyKnusperkeks wrote:
> dkt01 wrote:
> > MyDeveloperDay wrote:
> > > how does this differentiate from 
> > > 
> > > `MyType & val2;`
> > > 
> > > is that a binary operator? I don't think so?
> > The heuristic I'm using is that [symbol] & [symbol]; outside a class/struct 
> > declaration is more likely an instance of operator& than an uninitialized 
> > reference.  Tests on line 206 and 215 demonstrate how the two cases differ.
> > The heuristic I'm using is that [symbol] & [symbol]; outside a class/struct 
> > declaration is more likely an instance of operator& than an uninitialized 
> > reference.  Tests on line 206 and 215 demonstrate how the two cases differ.
> 
> I'm with this one. A reference without initialization is an error.
> 
> Can you add a test with an assignment, I didn't see one right now.
Added two new tests for assignment to pointer using unary operator & and 
initializing assignment to reference because neither of these assignments was 
in existing format tests I could find.


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-20 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2544
   SmallVector Contexts;
+  static SmallVector Scopes;
 

dkt01 wrote:
> HazardyKnusperkeks wrote:
> > Why static?
> The AnnotatingParser object lifetime is too short to track the scope 
> information I wanted.  From my testing, it seems like this object is created 
> once per line.  This is why I haven't added the scope information as part of 
> the contexts member.
> 
> I would like to make this a non-static member, but I'm not sure which object 
> it should be a member of.  Do you have a recommendation?
The `TokenAnnotator`? And then by reference to the parser.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2935
 return true; // Empty parentheses.
+
   // If there is an &/&& after the r_paren, this is likely a function.

Unrelated.



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:181
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+

dkt01 wrote:
> MyDeveloperDay wrote:
> > how does this differentiate from 
> > 
> > `MyType & val2;`
> > 
> > is that a binary operator? I don't think so?
> The heuristic I'm using is that [symbol] & [symbol]; outside a class/struct 
> declaration is more likely an instance of operator& than an uninitialized 
> reference.  Tests on line 206 and 215 demonstrate how the two cases differ.
> The heuristic I'm using is that [symbol] & [symbol]; outside a class/struct 
> declaration is more likely an instance of operator& than an uninitialized 
> reference.  Tests on line 206 and 215 demonstrate how the two cases differ.

I'm with this one. A reference without initialization is an error.

Can you add a test with an assignment, I didn't see one right now.


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-18 Thread David K Turner via Phabricator via cfe-commits

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



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:181
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+

MyDeveloperDay wrote:
> how does this differentiate from 
> 
> `MyType & val2;`
> 
> is that a binary operator? I don't think so?
The heuristic I'm using is that [symbol] & [symbol]; outside a class/struct 
declaration is more likely an instance of operator& than an uninitialized 
reference.  Tests on line 206 and 215 demonstrate how the two cases differ.


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-18 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:181
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+

how does this differentiate from 

`MyType & val2;`

is that a binary operator? I don't think so?


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-17 Thread David K Turner via Phabricator via cfe-commits

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



Comment at: clang/lib/Format/TokenAnnotator.cpp:1205
 case tok::r_brace:
+  if (Scopes.size() > 1)
+Scopes.pop_back();

HazardyKnusperkeks wrote:
> How does this happen? The only `push` I can see is before `parseBrace`, where 
> a `pop` is following directly after.
I previously had a double pop for some braces, but now the checks are no longer 
necessary as pushes and pops match.


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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-17 Thread David K Turner via Phabricator via cfe-commits

dkt01 updated this revision to Diff 489967.
dkt01 added a comment.

Further changes based on HazardyKnusperkeks' review


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

https://reviews.llvm.org/D141959

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
@@ -175,6 +175,48 @@
   ASSERT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("val1 & val2;");
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2.member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 & val3;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 // comment\n"
+" & val3;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_BinaryOperator);
+
+  Tokens =
+  annotate("val1 & val2.member & val3.member() & val4 & val5->member;");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[5], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[11], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[13], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("class c {\n"
+"  void func(type ) { a & member; }\n"
+"  anotherType \n"
+"}");
+  ASSERT_EQ(Tokens.size(), 22u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[12], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[17], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("struct S {\n"
+"  auto Mem = C & D;\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11267,6 +11267,13 @@
 
   verifyFormat("int operator()(T (&&)[N]) { return 1; }");
   verifyFormat("int operator()(T (&)[N]) { return 0; }");
+
+  verifyFormat("val1 & val2;");
+  verifyFormat("val1 & val2 & val3;");
+  verifyFormat("class c {\n"
+   "  void func(type ) { a & member; }\n"
+   "  anotherType \n"
+   "}");
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -847,6 +847,7 @@
 unsigned CommaCount = 0;
 while (CurrentToken) {
   if (CurrentToken->is(tok::r_brace)) {
+Scopes.pop_back();
 assert(OpeningBrace.Optional == CurrentToken->Optional);
 OpeningBrace.MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = 
@@ -1146,6 +1147,27 @@
 if (Previous && Previous->getType() != TT_DictLiteral)
   Previous->setType(TT_SelectorName);
   }
+  switch (Tok->getType()) {
+  case TT_FunctionLBrace:
+  case TT_LambdaLBrace:
+Scopes.push_back(ScopeType::Function);
+break;
+  case TT_ClassLBrace:
+  case TT_StructLBrace:
+  case TT_UnionLBrace:
+Scopes.push_back(ScopeType::Class);
+break;
+  case TT_EnumLBrace:
+  case TT_ControlStatementLBrace:
+  case TT_ElseLBrace:
+  case TT_BracedListLBrace:
+  case TT_CompoundRequirementLBrace:
+  case TT_ObjCBlockLBrace:
+  case TT_RecordLBrace:
+  case TT_RequiresExpressionLBrace:
+  default:
+Scopes.push_back(ScopeType::Other);
+  }
   if (!parseBrace())
 return false;
   break;
@@ -1176,6 +1198,7 @@
 case tok::r_square:
   return false;
 case tok::r_brace:
+  Scopes.pop_back();
   // Lines can start with '}'.
   if (Tok->Previous)
 return false;
@@ -1643,6 +1666,17 @@
 } ContextType = Unknown;
   };
 
+  enum class ScopeType : std::int8_t {
+// Not contained within scope block.
+None,
+// Contained in class declaration/definition.
+Class,
+// Contained within function definition.
+Function,
+// Contained

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-17 Thread David K Turner via Phabricator via cfe-commits

dkt01 updated this revision to Diff 489962.
dkt01 added a reviewer: HazardyKnusperkeks.
dkt01 added a comment.

Changes based on HazardyKnusperkeks' review


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

https://reviews.llvm.org/D141959

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
@@ -175,6 +175,48 @@
   ASSERT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("val1 & val2;");
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2.member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 & val3;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 // comment\n"
+" & val3;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_BinaryOperator);
+
+  Tokens =
+  annotate("val1 & val2.member & val3.member() & val4 & val5->member;");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[5], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[11], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[13], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("class c {\n"
+"  void func(type ) { a & member; }\n"
+"  anotherType \n"
+"}");
+  ASSERT_EQ(Tokens.size(), 22u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[12], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[17], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("struct S {\n"
+"  auto Mem = C & D;\n"
+"}");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_BinaryOperator);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11267,6 +11267,13 @@
 
   verifyFormat("int operator()(T (&&)[N]) { return 1; }");
   verifyFormat("int operator()(T (&)[N]) { return 0; }");
+
+  verifyFormat("val1 & val2;");
+  verifyFormat("val1 & val2 & val3;");
+  verifyFormat("class c {\n"
+   "  void func(type ) { a & member; }\n"
+   "  anotherType \n"
+   "}");
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -847,6 +847,8 @@
 unsigned CommaCount = 0;
 while (CurrentToken) {
   if (CurrentToken->is(tok::r_brace)) {
+if (Scopes.size() > 1)
+  Scopes.pop_back();
 assert(OpeningBrace.Optional == CurrentToken->Optional);
 OpeningBrace.MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = 
@@ -1146,8 +1148,32 @@
 if (Previous && Previous->getType() != TT_DictLiteral)
   Previous->setType(TT_SelectorName);
   }
-  if (!parseBrace())
+  switch (Tok->getType()) {
+  case TT_FunctionLBrace:
+  case TT_LambdaLBrace:
+Scopes.push_back(ScopeType::Function);
+break;
+  case TT_ClassLBrace:
+  case TT_StructLBrace:
+  case TT_UnionLBrace:
+Scopes.push_back(ScopeType::Class);
+break;
+  case TT_EnumLBrace:
+  case TT_ControlStatementLBrace:
+  case TT_ElseLBrace:
+  case TT_BracedListLBrace:
+  case TT_CompoundRequirementLBrace:
+  case TT_ObjCBlockLBrace:
+  case TT_RecordLBrace:
+  case TT_RequiresExpressionLBrace:
+  default:
+Scopes.push_back(ScopeType::Other);
+  }
+  if (!parseBrace()) {
+if (Scopes.size() > 1)
+  Scopes.pop_back();
 return false;
+  }
   break;
 case tok::less:
   if (parseAngle()) {
@@ -1176,6 +1202,8 @@
 case tok::r_square:
   return false;
 case tok::r_brace:
+  if (Scopes.size() > 1)
+Scopes.pop_back();
   // Lines can start with '}'.
   if (Tok->Previous)
 return false;
@@ -1643,6 +1671,17 @@
 } ContextType =

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-17 Thread David K Turner via Phabricator via cfe-commits

dkt01 marked 6 inline comments as done.
dkt01 added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2490-2491
+// Opeartors at class scope are likely pointer or reference members
+if (TokScope == ScopeType::Class)
+  return TT_PointerOrReference;
+

HazardyKnusperkeks wrote:
> What about
> ```
> struct S {
>   auto Mem = C & D;
> };
> ```
> That would be annotated, or is there another rule above with would kick in? 
> If there isn't a test with such a construct already please add one.
This case was already handled properly by the `IsExpression && 
!Contexts.back().CaretFound` condition, but I'll add this test case as there 
isn't an equivalent test yet.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2544
   SmallVector Contexts;
+  static SmallVector Scopes;
 

HazardyKnusperkeks wrote:
> Why static?
The AnnotatingParser object lifetime is too short to track the scope 
information I wanted.  From my testing, it seems like this object is created 
once per line.  This is why I haven't added the scope information as part of 
the contexts member.

I would like to make this a non-static member, but I'm not sure which object it 
should be a member of.  Do you have a recommendation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-17 Thread Björn Schäpers via Phabricator via cfe-commits

HazardyKnusperkeks added a comment.

Could this be added to `Context` instead of a new type?




Comment at: clang/lib/Format/TokenAnnotator.cpp:1205
 case tok::r_brace:
+  if (Scopes.size() > 1)
+Scopes.pop_back();

How does this happen? The only `push` I can see is before `parseBrace`, where a 
`pop` is following directly after.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1674
 
+  enum class ScopeType {
+// Not contained within scope block





Comment at: clang/lib/Format/TokenAnnotator.cpp:1675
+  enum class ScopeType {
+// Not contained within scope block
+None,

Full stop at the end of your comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1937
   Current.setType(determineStarAmpUsage(
-  Current,
+  Current, Scopes.back(),
   Contexts.back().CanBeExpression && Contexts.back().IsExpression,

You don't need to pass that, you can access it directly from 
`determineStarAmpUsage`, right?



Comment at: clang/lib/Format/TokenAnnotator.cpp:2385
+  TokenType determineStarAmpUsage(const FormatToken ,
+  const ScopeType TokScope, bool IsExpression,
   bool InTemplateArgument) {

That `const` doesn't seem fit with the LLVM style.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2385
+  TokenType determineStarAmpUsage(const FormatToken ,
+  const ScopeType TokScope, bool IsExpression,
   bool InTemplateArgument) {

HazardyKnusperkeks wrote:
> That `const` doesn't seem fit with the LLVM style.
I'd prefer you write it out.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2490-2491
+// Opeartors at class scope are likely pointer or reference members
+if (TokScope == ScopeType::Class)
+  return TT_PointerOrReference;
+

What about
```
struct S {
  auto Mem = C & D;
};
```
That would be annotated, or is there another rule above with would kick in? If 
there isn't a test with such a construct already please add one.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2496-2502
+((PrevToken->getPreviousNonComment() &&
+  (PrevToken->getPreviousNonComment()->is(tok::amp) ||
+   PrevToken->getPreviousNonComment()->is(tok::period) ||
+   PrevToken->getPreviousNonComment()->is(tok::arrow) ||
+   PrevToken->getPreviousNonComment()->is(tok::arrowstar) ||
+   PrevToken->getPreviousNonComment()->is(tok::periodstar))) ||
+ !PrevToken->getPreviousNonComment()) &&

That seems better.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2544
   SmallVector Contexts;
+  static SmallVector Scopes;
 

Why static?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141959

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

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

2023-01-17 Thread David K Turner via Phabricator via cfe-commits

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

Token annotator for clang-format incorrectly identifies operator& as a 
reference type in situations like Boost serialization archives 
.

Add annotation rules for standalone and chained operator& instances while 
preserving behavior for reference declarations at class scope.  Add tests to 
validate annotation and formatting behavior.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141959

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
@@ -175,6 +175,42 @@
   ASSERT_EQ(Tokens.size(), 17u) << Tokens;
   EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_PointerOrReference);
   EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("val1 & val2;");
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2.member;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 & val3;");
+  ASSERT_EQ(Tokens.size(), 7u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[3], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("val1 & val2 // comment\n"
+" & val3;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[4], tok::amp, TT_BinaryOperator);
+
+  Tokens =
+  annotate("val1 & val2.member & val3.member() & val4 & val5->member;");
+  ASSERT_EQ(Tokens.size(), 19u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[5], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[11], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[13], tok::amp, TT_BinaryOperator);
+
+  Tokens = annotate("class c {\n"
+"  void func(type ) { a & member; }\n"
+"  anotherType \n"
+"}");
+  ASSERT_EQ(Tokens.size(), 22u) << Tokens;
+  EXPECT_TOKEN(Tokens[7], tok::amp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[12], tok::amp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[17], tok::amp, TT_PointerOrReference);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -11267,6 +11267,13 @@
 
   verifyFormat("int operator()(T (&&)[N]) { return 1; }");
   verifyFormat("int operator()(T (&)[N]) { return 0; }");
+
+  verifyFormat("val1 & val2;");
+  verifyFormat("val1 & val2 & val3;");
+  verifyFormat("class c {\n"
+   "  void func(type ) { a & member; }\n"
+   "  anotherType \n"
+   "}");
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -847,6 +847,8 @@
 unsigned CommaCount = 0;
 while (CurrentToken) {
   if (CurrentToken->is(tok::r_brace)) {
+if (Scopes.size() > 1)
+  Scopes.pop_back();
 assert(OpeningBrace.Optional == CurrentToken->Optional);
 OpeningBrace.MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = 
@@ -1146,8 +1148,32 @@
 if (Previous && Previous->getType() != TT_DictLiteral)
   Previous->setType(TT_SelectorName);
   }
-  if (!parseBrace())
+  switch (Tok->getType()) {
+  case TT_FunctionLBrace:
+  case TT_LambdaLBrace:
+Scopes.push_back(ScopeType::Function);
+break;
+  case TT_ClassLBrace:
+  case TT_StructLBrace:
+  case TT_UnionLBrace:
+Scopes.push_back(ScopeType::Class);
+break;
+  case TT_EnumLBrace:
+  case TT_ControlStatementLBrace:
+  case TT_ElseLBrace:
+  case TT_BracedListLBrace:
+  case TT_CompoundRequirementLBrace:
+  case TT_ObjCBlockLBrace:
+  case TT_RecordLBrace:
+  case TT_RequiresExpressionLBrace:
+  default:
+Scopes.push_back(ScopeType::Other);
+  }
+  if (!parseBrace()) {
+if (Scopes.size() > 1)
+  Scopes.pop_back();
 return false;
+  }
   break;
 case tok::less:
   if (parseAngle()) {
@@

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

[PATCH] D141959: [clang-format] Fix inconsistent identification of operator

32 matches

Site Navigation

Mail list logo

Footer information