[clang-format] Don't allow variable decls to have trailing return arrows

[rymiel]

Summary

The heuristic for determining if an arrow is a trailing return arrow
looks for the auto keyword, along with parentheses. This isn't
sufficient, since it also triggers on variable declarations with an auto
type, and with an arrow operator.

This patch makes sure a function declaration is being parsed, instead of
any other declaration.

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

[owenpan]

clang/lib/Format/TokenAnnotator.cpp

1922

It seems we can simply check if the line might be a function declaration here instead of resetting AutoFound below.

clang/unittests/Format/TokenAnnotatorTest.cpp

1493

Can this be valid C++?

[rymiel]

Use MightBeFunctionDecl instead

clang/lib/Format/TokenAnnotator.cpp

1922

I could swear I specifically tried using that flag and it didn't work in all cases, but I guess I did something wrong back then...

[rymiel]

clang/unittests/Format/TokenAnnotatorTest.cpp

1493

Yes! (https://godbolt.org/z/bnYahK4cz) (also I copied it from FormatTest)

[rymiel]

update commit message

clang/lib/Format/TokenAnnotator.cpp

if (Current.is(Keywords.kw_instanceof)) {
       // having multiple variable declarations.
       Contexts.back().FirstStartOfName = nullptr;
     } else if (Current.isOneOf(tok::kw_auto, tok::kw___auto_type)) {
       AutoFound = true;
     } else if (Current.is(tok::arrow) &&
                Style.Language == FormatStyle::LK_Java) {
       Current.setType(TT_LambdaArrow);
     } else if (Current.is(tok::arrow) && AutoFound &&
                (Line.MustBeDeclaration || Line.InPPDirective) &&
                Current.NestingLevel == 0 &&
                !Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) {
       // not auto operator->() -> xxx;
       Current.setType(TT_TrailingReturnArrow);
     } else if (Current.is(tok::arrow) && Current.Previous &&
                Current.Previous->is(tok::r_brace)) {
       // Concept implicit conversion constraint needs to be treated like
       // a trailing return type  ... } -> <type>.
if (Current.is(Keywords.kw_instanceof)) {
           // In Verilog `<=` is assignment if in its own statement. It is a
           // statement instead of an expression, that is it can not be chained.
           Current.ForcedPrecedence = prec::Assignment;
           Current.setFinalizedType(TT_BinaryOperator);
         }
         if (Current.getPrecedence() == prec::Assignment)
           Contexts.back().VerilogAssignmentFound = true;
       }
+      if (AutoFound && Current.is(tok::equal) && Current.Previous &&
+          Current.Previous->isNot(tok::kw_operator)) {
+        // AutoFound is only used for function declarations with `auto`. This
+        // looks like a variable declaration, so unset it.
+        AutoFound = false;
+      }
       Current.setType(TT_BinaryOperator);
     } else if (Current.is(tok::comment)) {
       if (Current.TokenText.startswith("/*")) {
         if (Current.TokenText.endswith("*/")) {
           Current.setType(TT_BlockComment);
         } else {
           // The lexer has for some reason determined a comment here. But we
           // cannot really handle it, if it isn't properly terminated.

clang/unittests/Format/TokenAnnotatorTest.cpp

 TEST_F(TokenAnnotatorTest, UnderstandsC11GenericSelection) {
   auto Tokens = annotate("_Generic(x, int: 1, default: 0)");
   ASSERT_EQ(Tokens.size(), 13u) << Tokens;
   EXPECT_TOKEN(Tokens[0], tok::kw__Generic, TT_Unknown);
   EXPECT_TOKEN(Tokens[5], tok::colon, TT_GenericSelectionColon);
   EXPECT_TOKEN(Tokens[9], tok::colon, TT_GenericSelectionColon);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsTrailingReturnArrow) {
+  auto Tokens = annotate("auto f() -> int;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator->() -> int;");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[2], tok::arrow, TT_OverloadedOperator);
+  EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator++(int) -> int;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator=() -> int;");
+  ASSERT_EQ(Tokens.size(), 9u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto operator=(int) -> int;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("auto foo() -> auto { return Val; }");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+
+  Tokens = annotate("struct S { auto bar() const -> int; };");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::arrow, TT_TrailingReturnArrow);
+
+  // Not trailing return arrows
+  Tokens = annotate("auto a = b->c;");
+  ASSERT_EQ(Tokens.size(), 8u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = (b)->c;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = b()->c;");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("auto a = b->c();");
+  ASSERT_EQ(Tokens.size(), 10u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("decltype(auto) a = b()->c;");
+  ASSERT_EQ(Tokens.size(), 13u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("void f() { auto a = b->c(); }");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::arrow, TT_Unknown);
+
+  Tokens = annotate("void f() { auto a = b()->c; }");
+  ASSERT_EQ(Tokens.size(), 16u) << Tokens;
+  EXPECT_TOKEN(Tokens[11], tok::arrow, TT_Unknown);
+
+  // Mixed
+  Tokens = annotate("auto f() -> int { auto a = b()->c; }");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[4], tok::arrow, TT_TrailingReturnArrow);
+  EXPECT_TOKEN(Tokens[13], tok::arrow, TT_Unknown);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsVerilogOperators) {
   auto Annotate = [this](llvm::StringRef Code) {
     return annotate(Code, getLLVMStyle(FormatStyle::LK_Verilog));
   };
   // Test that unary operators get labeled as such and that operators like '++'
   // don't get split.
   tok::TokenKind Unary[] = {tok::plus,  tok::minus,    tok::exclaim,
                             tok::tilde, tok::amp,      tok::pipe,