[clang-format] fix nested angle brackets parse inside concept definition

[predelnik]

Summary

Due to how parseBracedList always stopped on the first closing angle
bracket and was used in parsing angle bracketed expression inside concept
definition, nested brackets inside concepts were parsed incorrectly.

nextToken() call before calling parseBracedList is required because
we were processing opening angle bracket inside parseBracedList second
time leading to incorrect logic after my fix.

Fixes https://github.com/llvm/llvm-project/issues/54943
Fixes https://github.com/llvm/llvm-project/issues/54837

[curdeius]

Is there an issue report for this bug already?

[curdeius]

Could you please add an annotator test as well?

[predelnik]

In D123896#3455299, @curdeius wrote:

Is there an issue report for this bug already?

I could create an issue if that is required.

In D123896#3455301, @curdeius wrote:

Could you please add an annotator test as well?

Will look into that, thank you for reviewing.

[predelnik]

Added annotator test, checked that it fails without a patch

[predelnik]

I've created the issue here:
https://github.com/llvm/llvm-project/issues/54943
It also seems to fix the other issue I reported which was partially fixed before this change in master:
https://github.com/llvm/llvm-project/issues/54837

[predelnik]

Added issue links to commit message

[HazardyKnusperkeks]

As this is still on my todo list: Have you considered

concept X = Bar<5 < 4, 5 > 8>;
concept X = Bar<5 < 4, false>;

Or stuff like that?

[predelnik]

In D123896#3464171, @HazardyKnusperkeks wrote:

As this is still on my todo list: Have you considered

concept X = Bar<5 < 4, 5 > 8>;
concept X = Bar<5 < 4, false>;

Or stuff like that?

I think it would not work but would not make things much worse than before, have to check that though.
Looks like parseBracedList probably would not be enough for correct parsing of such expressions, is there any better approach?

[HazardyKnusperkeks]

Not yet: https://reviews.llvm.org/D120034

[predelnik]

I see now, would certainly like to help but it sounds like I might not be competent enough for this. I guess this review could be closed unless it's an acceptable half-measure, not sure.

[HazardyKnusperkeks]

No I think it is a step in the right direction. It will help in common cases, and I still don't know when I will have the time to work on that.
But you are very welcome to join the review, and of course adding patches. :)

[curdeius]

LGTM. Thank you!

[curdeius]

Do you need help landing this?

[predelnik]

Probably, yes. I didn't know I have to do it myself, sorry.

[predelnik]

Ok I've found it on a guide here at last https://llvm.org/docs/MyFirstTypoFix.html
Please land a patch for me, @curdeius, my author data is “Sergey Semushin [e-mail]”
Thank you.

clang/lib/Format/UnwrappedLineParser.cpp

do {
     case tok::l_brace:
       // Assume there are no blocks inside a braced init list apart
       // from the ones we explicitly parse out (like lambdas).
       FormatTok->setBlockKind(BK_BracedInit);
       nextToken();
       parseBracedList();
       break;
     case tok::less:
-      if (Style.Language == FormatStyle::LK_Proto) {
+      if (Style.Language == FormatStyle::LK_Proto ||
+          ClosingBraceKind == tok::greater) {
         nextToken();
         parseBracedList(/*ContinueOnSemicolons=*/false, /*IsEnum=*/false,
                         /*ClosingBraceKind=*/tok::greater);
       } else {
         nextToken();
       }
       break;
     case tok::semi:
do {
     case tok::kw_static_cast:
     case tok::kw_const_cast:
     case tok::kw_reinterpret_cast:
     case tok::kw_dynamic_cast:
       nextToken();
       if (!FormatTok->is(tok::less))
         return;
 
+      nextToken();
       parseBracedList(/*ContinueOnSemicolons=*/false, /*IsEnum=*/false,
                       /*ClosingBraceKind=*/tok::greater);
       break;
 
     case tok::kw_bool:
       // bool is only allowed if it is directly followed by a paren for a cast:
       // concept C = bool(...);
       // and bool is the only type, all other types as cast must be inside a
default:
       case tok::equal:       // Initial identifier of a concept declaration.
         break;
       default:
         return;
       }
 
       // Read identifier with optional template declaration.
       nextToken();
-      if (FormatTok->is(tok::less))
+      if (FormatTok->is(tok::less)) {
+        nextToken();
         parseBracedList(/*ContinueOnSemicolons=*/false, /*IsEnum=*/false,
                         /*ClosingBraceKind=*/tok::greater);
+      }
       break;
     }
   } while (!eof());
 }
 
 bool UnwrappedLineParser::parseEnum() {
   const FormatToken &InitialToken = *FormatTok;
 

clang/unittests/Format/FormatTest.cpp

verifyFormat("template <class T>\n"
                "concept is2D = __array_extent(T, 1) == 2;");
 
   verifyFormat("template <class T>\n"
                "concept isRhs = __is_rvalue_expr(std::declval<T>() + 2)");
 
   verifyFormat("template <class T, class T2>\n"
                "concept Same = __is_same_as<T, T2>;");
 
+  verifyFormat(
+      "template <class _InIt, class _OutIt>\n"
+      "concept _Can_reread_dest =\n"
+      "    std::forward_iterator<_OutIt> &&\n"
+      "    std::same_as<std::iter_value_t<_InIt>, std::iter_value_t<_OutIt>>;");
+
   auto Style = getLLVMStyle();
   Style.BreakBeforeConceptDeclarations = FormatStyle::BBCDS_Allowed;
 
   verifyFormat(
       "template <typename T>\n"
       "concept C = requires(T t) {\n"
       "              requires Bar<T> && Foo<T>;\n"
       "              requires((trait<T> && Baz) || (T2<T> && Foo<T>));\n"

clang/unittests/Format/TokenAnnotatorTest.cpp

Tokens = annotate("struct [[nodiscard]] zero_t {\n"
                     "};");
   ASSERT_EQ(Tokens.size(), 44u);
   EXPECT_TOKEN(Tokens[13], tok::kw_requires, TT_RequiresClause);
   EXPECT_TOKEN(Tokens[14], tok::kw_requires, TT_RequiresExpression);
   EXPECT_TOKEN(Tokens[15], tok::l_brace, TT_RequiresExpressionLBrace);
   EXPECT_TOKEN(Tokens[21], tok::r_brace, TT_Unknown);
   EXPECT_EQ(Tokens[21]->MatchingParen, Tokens[15]);
   EXPECT_TRUE(Tokens[21]->ClosesRequiresClause);
+
+  Tokens =
+      annotate("template <class A, class B> concept C ="
+               "std::same_as<std::iter_value_t<A>, std::iter_value_t<B>>;");
+  ASSERT_EQ(Tokens.size(), 31u) << Tokens;
+  EXPECT_TOKEN(Tokens[8], tok::kw_concept, TT_Unknown);
+  EXPECT_TOKEN(Tokens[14], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[18], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[20], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[25], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[27], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[28], tok::greater, TT_TemplateCloser);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsRequiresExpressions) {
   auto Tokens = annotate("bool b = requires(int i) { i + 5; };");
   ASSERT_EQ(Tokens.size(), 16u) << Tokens;
   EXPECT_TOKEN(Tokens[3], tok::kw_requires, TT_RequiresExpression);
   EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_RequiresExpressionLParen);
   EXPECT_TOKEN(Tokens[8], tok::l_brace, TT_RequiresExpressionLBrace);