[clang-format] fix template closer followed by >

[Backl1ght]

Summary

This patch is to fix https://github.com/llvm/llvm-project/issues/59785.

related patches: https://reviews.llvm.org/D86581 and https://reviews.llvm.org/D100778.

I think the condition added in D100778 is enough to judge shifts within conditions, the condition added in D86581 is unnecessary and will cause template opener/closer parsed as binary operator, so I simply remove the condition added in D86581.

[MyDeveloperDay]

I'm good with this but I'd be interesting in @owenpan and @HazardyKnusperkeks opinion.

clang/lib/Format/TokenAnnotator.cpp

169–172

I was wondering this is actually saying....

I think its saying... 2 tokens are next to each other ">>" its not so much formatting but ensuring the existing formatting is maintained, is that what you understand?

Whilst this works it won't correct the case where the whitespace is wrong

i.e. I don't think

if (std::tuple_size_v < T >> 0) {
}

will be corrected to be

if (std::tuple_size_v<T> > 0) {
}

I'm a little wary of rules that use the existing whitespace, but I tend to agree that it might be ok without the extra check.

It would be good to capture this as an annotator test (I like the verifyformat one you put in) but the annotator tests we can assert that its actually a templatecloser and binary operatror

[Backl1ght]

In D140843#4022673, @MyDeveloperDay wrote:

I'm good with this but I'd be interesting in @owenpan and @HazardyKnusperkeks opinion.

Yes, this is what I understand.

And I will add an annotator test later.

This patch is just a temporary solution I think, there are further things to do with such case, maybe I should add a TODO or FIXME?

[Backl1ght]

clang/lib/Format/TokenAnnotator.cpp

169–172

I think there should be more infomation to make the correction.

So far, we only know that tuple_size_v is an identifier, but the exact type of tuple_size_v is unknown. Maybe tuple_size_v is a type trait and in this case we should make the correction. Maybe tuple_size_v is an interger and in this case we should not make the correction.

To make the correction, we need at least one of:

1. is tuple_size_v a type trait?
2. is T a typename?
3. is >> a right shift?

[MyDeveloperDay]

LGTM, thank you for adding an annotator test, I'd like one of @owenpan, @HazardyKnusperkeks or @rymiel to comment before commit, just to get another view.

[owenpan]

In D140843#4028142, @MyDeveloperDay wrote:

I'd like one of @owenpan, @HazardyKnusperkeks or @rymiel to comment before commit, just to get another view.

Perhaps clang-format should never insert a space in >>.

[MyDeveloperDay]

In D140843#4029641, @owenpan wrote:

In D140843#4028142, @MyDeveloperDay wrote:

I'd like one of @owenpan, @HazardyKnusperkeks or @rymiel to comment before commit, just to get another view.

Perhaps clang-format should never insert a space in >>.

I seem to remember some "quirkiness" with nested templates that need converting

Foo<Bar<T>>

into

Foo<Bar<T> >

but I think that was sometime back.

clang/lib/Format/TokenAnnotator.cpp

while (CurrentToken) {
         // Try to do a better job at looking for ">>" within the condition of
         // a statement. Conservatively insert spaces between consecutive ">"
         // tokens to prevent splitting right bitshift operators and potentially
         // altering program semantics. This check is overly conservative and
         // will prevent spaces from being inserted in select nested template
         // parameter cases, but should not alter program semantics.
         if (CurrentToken->Next && CurrentToken->Next->is(tok::greater) &&
             Left->ParentBracket != tok::less &&
-            (isKeywordWithCondition(*Line.First) ||
-             CurrentToken->getStartOfNonWhitespace() ==
-                 CurrentToken->Next->getStartOfNonWhitespace().getLocWithOffset(
-                     -1))) {
+                    -1)) {
           return false;
         }
         Left->MatchingParen = CurrentToken;
         CurrentToken->MatchingParen = Left;
         // In TT_Proto, we must distignuish between:
         //   map<key, value>
         //   msg < item: data >
         //   msg: < item: data >
         // In TT_TextProto, map<key, value> does not occur.

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, UnderstandsTemplateParameters) {
   verifyFormat("template <class T> struct S<std::is_arithmetic<T>{}> {};");
   verifyFormat("f(a.operator()<A>());");
   verifyFormat("f(aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
                "      .template operator()<A>());",
                getLLVMStyleWithColumns(35));
   verifyFormat("bool_constant<a && noexcept(f())>");
   verifyFormat("bool_constant<a || noexcept(f())>");
 
+  verifyFormat("if (std::tuple_size_v<T> > 0)");
+
   // Not template parameters.
   verifyFormat("return a < b && c > d;");
   verifyFormat("void f() {\n"
                "  while (a < b && c > d) {\n"
                "  }\n"
                "}");
   verifyFormat("template <typename... Types>\n"
                "typename enable_if<0 < sizeof...(Types)>::type Foo() {}");

clang/unittests/Format/TokenAnnotatorTest.cpp

auto Tokens =
                "  { #typeName, foo<FooType>(new foo<realClass>(#typeName)) }");
   ASSERT_EQ(Tokens.size(), 27u) << Tokens;
   EXPECT_TOKEN(Tokens[11], tok::less, TT_TemplateOpener);
   EXPECT_TOKEN(Tokens[13], tok::greater, TT_TemplateCloser);
   EXPECT_TOKEN(Tokens[17], tok::less, TT_TemplateOpener);
   EXPECT_TOKEN(Tokens[19], tok::greater, TT_TemplateCloser);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsGreaterAfterTemplateCloser) {
+  auto Tokens = annotate("if (std::tuple_size_v<T> > 0)");
+  ASSERT_EQ(Tokens.size(), 12u) << Tokens;
+  EXPECT_TOKEN(Tokens[5], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[7], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[8], tok::greater, TT_BinaryOperator);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsWhitespaceSensitiveMacros) {
   FormatStyle Style = getLLVMStyle();
   Style.WhitespaceSensitiveMacros.push_back("FOO");
 
   auto Tokens = annotate("FOO(1+2 )\n", Style);
   EXPECT_EQ(Tokens.size(), 7u) << Tokens;
   EXPECT_TOKEN(Tokens[0], tok::identifier, TT_UntouchableMacroFunc);