[clang-format] Treats &/&& as reference when followed by ',' or ')'.

[red1bluelost]

Summary

Ran into an issue where function declarations inside function scopes or uses
of sizeof inside a function would treat the && in 'sizeof(Type &&)' as a binary
operator.

Attempt to fix this by assuming reference when followed by ',' or ')'. Also adds
tests for these.

Also hit an edge case in another test that treated "and" the same as "&&"
since it parses as C++. Changed the "and" to "also" so it is no longer a
keyword.

GitHub issue at: https://github.com/llvm/llvm-project/issues/58923

[MyDeveloperDay]

Its always good if you have a github issue to add that to the summary (just to tie stuff together), if you don't feel free to go over there an log a bug.

clang/lib/Format/TokenAnnotator.cpp

2360

nowadays we add a TokenAnnotator test (they are very easy to write), this lets us ensure its getting annotated correctly. (which is really the bug here),

but your tests below are great too!.

[MyDeveloperDay]

On first inspection this looks ok, but lets wait for the others chime in, please add the annotator test too.

[red1bluelost]

Adds tests for the TokenAnnotator as well.

[red1bluelost]

Made a GitHub issue and addressed the current comments. :)

clang/lib/Format/TokenAnnotator.cpp

2360

Thanks for the suggestion! Didn't realize there were tests for the tokenizer. I got those added now.

[MyDeveloperDay]

For me this looks good, but please wait for one of the others to double confirm.

[red1bluelost]

I noticed that the Premerge seemed ran into an issue with 'InsertBraces'. It looks to me that it is unrelated and probably due to the pre-merge machine using a pre-15 clang-format (I ran into similar issue on my local machine).

If it is unrelated, could a reviewer commit on my behalf? Name and email is "Micah Weston" and [e-mail]

Thanks for the helpful reviews!

clang/lib/Format/TokenAnnotator.cpp

if (!PrevToken)
       return TT_UnaryOperator;
 
     const FormatToken *NextToken = Tok.getNextNonComment();
 
     if (InTemplateArgument && NextToken && NextToken->is(tok::kw_noexcept))
       return TT_BinaryOperator;
 
     if (!NextToken ||
-        NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_noexcept) ||
+        NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_noexcept, tok::comma,
+                           tok::r_paren) ||
         NextToken->canBePointerOrReferenceQualifier() ||
         (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment())) {
       return TT_PointerOrReference;
     }
 
     if (PrevToken->is(tok::coloncolon))
       return TT_PointerOrReference;
 

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
   verifyFormat("int *f1(int *a, int &b, int &&c);", Style);
   verifyFormat("int &f2(int &&c, int *a, int &b);", Style);
   verifyFormat("int &&f3(int &b, int &&c, int *a);", Style);
   verifyFormat("int *f1(int &a) const &;", Style);
   verifyFormat("int *f1(int &a) const & = 0;", Style);
   verifyFormat("int *a = f1();", Style);
   verifyFormat("int &b = f2();", Style);
   verifyFormat("int &&c = f3();", Style);
+  verifyFormat("int f3() { return sizeof(Foo &); }", Style);
+  verifyFormat("int f4() { return sizeof(Foo &&); }", Style);
+  verifyFormat("void f5() { int f6(Foo &, Bar &); }", Style);
+  verifyFormat("void f5() { int f6(Foo &&, Bar &&); }", Style);
   verifyFormat("for (auto a = 0, b = 0; const auto &c : {1, 2, 3})", Style);
   verifyFormat("for (auto a = 0, b = 0; const int &c : {1, 2, 3})", Style);
   verifyFormat("for (auto a = 0, b = 0; const Foo &c : {1, 2, 3})", Style);
   verifyFormat("for (auto a = 0, b = 0; const Foo *c : {1, 2, 3})", Style);
   verifyFormat("for (int a = 0, b = 0; const auto &c : {1, 2, 3})", Style);
   verifyFormat("for (int a = 0, b = 0; const int &c : {1, 2, 3})", Style);
   verifyFormat("for (int a = 0, b = 0; const Foo &c : {1, 2, 3})", Style);
   verifyFormat("for (int a = 0, b++; const auto &c : {1, 2, 3})", Style);
TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
   Style.ReferenceAlignment = FormatStyle::RAS_Pointer;
   verifyFormat("int* f1(int* a, int& b, int&& c);", Style);
   verifyFormat("int& f2(int&& c, int* a, int& b);", Style);
   verifyFormat("int&& f3(int& b, int&& c, int* a);", Style);
   verifyFormat("int* f1(int& a) const& = 0;", Style);
   verifyFormat("int* a = f1();", Style);
   verifyFormat("int& b = f2();", Style);
   verifyFormat("int&& c = f3();", Style);
+  verifyFormat("int f3() { return sizeof(Foo&); }", Style);
+  verifyFormat("int f4() { return sizeof(Foo&&); }", Style);
+  verifyFormat("void f5() { int f6(Foo&, Bar&); }", Style);
+  verifyFormat("void f5() { int f6(Foo&&, Bar&&); }", Style);
   verifyFormat("for (auto a = 0, b = 0; const auto& c : {1, 2, 3})", Style);
   verifyFormat("for (auto a = 0, b = 0; const int& c : {1, 2, 3})", Style);
   verifyFormat("for (auto a = 0, b = 0; const Foo& c : {1, 2, 3})", Style);
   verifyFormat("for (auto a = 0, b = 0; const Foo* c : {1, 2, 3})", Style);
   verifyFormat("for (int a = 0, b = 0; const auto& c : {1, 2, 3})", Style);
   verifyFormat("for (int a = 0, b = 0; const int& c : {1, 2, 3})", Style);
   verifyFormat("for (int a = 0, b = 0; const Foo& c : {1, 2, 3})", Style);
   verifyFormat("for (int a = 0, b = 0; const Foo* c : {1, 2, 3})", Style);
TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
   Style.PointerAlignment = FormatStyle::PAS_Right;
   Style.ReferenceAlignment = FormatStyle::RAS_Left;
   verifyFormat("int *f1(int *a, int& b, int&& c);", Style);
   verifyFormat("int& f2(int&& c, int *a, int& b);", Style);
   verifyFormat("int&& f3(int& b, int&& c, int *a);", Style);
   verifyFormat("int *a = f1();", Style);
   verifyFormat("int& b = f2();", Style);
   verifyFormat("int&& c = f3();", Style);
+  verifyFormat("int f3() { return sizeof(Foo&); }", Style);
+  verifyFormat("int f4() { return sizeof(Foo&&); }", Style);
+  verifyFormat("void f5() { int f6(Foo&, Bar&); }", Style);
+  verifyFormat("void f5() { int f6(Foo&&, Bar&&); }", Style);
   verifyFormat("for (auto a = 0, b = 0; const Foo *c : {1, 2, 3})", Style);
   verifyFormat("for (int a = 0, b = 0; const Foo *c : {1, 2, 3})", Style);
   verifyFormat("for (int a = 0, b++; const Foo *c : {1, 2, 3})", Style);
 
   Style.AlignConsecutiveDeclarations.Enabled = true;
   verifyFormat("Const unsigned int *c;\n"
                "const unsigned int *d;\n"
                "Const unsigned int& e;\n"
                "const unsigned int& f;\n"
                "const unsigned      g;\n"
                "Const unsigned      h;",
                Style);
 
   Style.PointerAlignment = FormatStyle::PAS_Left;
   Style.ReferenceAlignment = FormatStyle::RAS_Middle;
   verifyFormat("int* f1(int* a, int & b, int && c);", Style);
   verifyFormat("int & f2(int && c, int* a, int & b);", Style);
   verifyFormat("int && f3(int & b, int && c, int* a);", Style);
   verifyFormat("int* a = f1();", Style);
   verifyFormat("int & b = f2();", Style);
   verifyFormat("int && c = f3();", Style);
+  verifyFormat("int f3() { return sizeof(Foo &); }", Style);
+  verifyFormat("int f4() { return sizeof(Foo &&); }", Style);
+  verifyFormat("void f5() { int f6(Foo &, Bar &); }", Style);
+  verifyFormat("void f5() { int f6(Foo &&, Bar &&); }", Style);
   verifyFormat("for (auto a = 0, b = 0; const auto & c : {1, 2, 3})", Style);
   verifyFormat("for (auto a = 0, b = 0; const int & c : {1, 2, 3})", Style);
   verifyFormat("for (auto a = 0, b = 0; const Foo & c : {1, 2, 3})", Style);
   verifyFormat("for (auto a = 0, b = 0; const Foo* c : {1, 2, 3})", Style);
   verifyFormat("for (int a = 0, b++; const auto & c : {1, 2, 3})", Style);
   verifyFormat("for (int a = 0, b++; const int & c : {1, 2, 3})", Style);
   verifyFormat("for (int a = 0, b++; const Foo & c : {1, 2, 3})", Style);
   verifyFormat("for (int a = 0, b++; const Foo* c : {1, 2, 3})", Style);
TEST_F(FormatTest, SeparatePointerReferenceAlignment) {
   Style.PointerAlignment = FormatStyle::PAS_Middle;
   Style.ReferenceAlignment = FormatStyle::RAS_Right;
   verifyFormat("int * f1(int * a, int &b, int &&c);", Style);
   verifyFormat("int &f2(int &&c, int * a, int &b);", Style);
   verifyFormat("int &&f3(int &b, int &&c, int * a);", Style);
   verifyFormat("int * a = f1();", Style);
   verifyFormat("int &b = f2();", Style);
   verifyFormat("int &&c = f3();", Style);
+  verifyFormat("int f3() { return sizeof(Foo &); }", Style);
+  verifyFormat("int f4() { return sizeof(Foo &&); }", Style);
+  verifyFormat("void f5() { int f6(Foo &, Bar &); }", Style);
+  verifyFormat("void f5() { int f6(Foo &&, Bar &&); }", Style);
   verifyFormat("for (auto a = 0, b = 0; const Foo * c : {1, 2, 3})", Style);
   verifyFormat("for (int a = 0, b = 0; const Foo * c : {1, 2, 3})", Style);
   verifyFormat("for (int a = 0, b++; const Foo * c : {1, 2, 3})", Style);
 
   // FIXME: we don't handle this yet, so output may be arbitrary until it's
   // specifically handled
   // verifyFormat("int Add2(BTree * &Root, char * szToAdd)", Style);
 }
TEST_F(FormatTest, AlignsStringLiterals) {
   verifyFormat("loooooooooooooooooooooooooongFunction(\"short literal \"\n"
                "                                      \"short literal\");");
   verifyFormat(
       "looooooooooooooooooooooooongFunction(\n"
       "    \"short literal\"\n"
       "    \"looooooooooooooooooooooooooooooooooooooooooooooooong literal\");");
   verifyFormat("someFunction(\"Always break between multi-line\"\n"
                "             \" string literals\",\n"
-               "             and, other, parameters);");
+               "             also, other, parameters);");
   EXPECT_EQ("fun + \"1243\" /* comment */\n"
             "      \"5678\";",
             format("fun + \"1243\" /* comment */\n"
                    "    \"5678\";",
                    getLLVMStyleWithColumns(28)));
   EXPECT_EQ(
       "aaaaaa = \"aaaaaaaaaaaaaaaaaaaaaaaaaa aaaaaaaaaaaaaaaaaaaaaaaaaaaa \"\n"
       "         \"aaaaaaaaaaaaaaaaaaaaa\"\n"

clang/unittests/Format/TokenAnnotatorTest.cpp

TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmp) {
 
   Tokens = annotate("if (Class* obj {getObj()})");
   ASSERT_EQ(Tokens.size(), 12u) << Tokens;
   EXPECT_TOKEN(Tokens[3], tok::star, TT_PointerOrReference);
 
   Tokens = annotate("if (Foo* Bar = getObj())");
   ASSERT_EQ(Tokens.size(), 11u) << Tokens;
   EXPECT_TOKEN(Tokens[3], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("int f3() { return sizeof(Foo&); }");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("int f4() { return sizeof(Foo&&); }");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_PointerOrReference);
+
+  Tokens = annotate("void f5() { int f6(Foo&, Bar&); }");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::amp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[12], tok::amp, TT_PointerOrReference);
+
+  Tokens = annotate("void f7() { int f8(Foo&&, Bar&&); }");
+  ASSERT_EQ(Tokens.size(), 17u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_PointerOrReference);
+  EXPECT_TOKEN(Tokens[12], tok::ampamp, TT_PointerOrReference);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
   auto Tokens = annotate("x - 0");
   ASSERT_EQ(Tokens.size(), 4u) << Tokens;
   EXPECT_TOKEN(Tokens[1], tok::minus, TT_BinaryOperator);
   Tokens = annotate("0 + 0");
   ASSERT_EQ(Tokens.size(), 4u) << Tokens;