[clang-format] Concepts: allow identifiers after negation

[rymiel]

Summary

Previously, the formatter would refuse to treat identifiers within a
compound concept definition as actually part of the definition, if
they were after the negation operator !. It is now made consistent
with the likes of && and ||.

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

[HazardyKnusperkeks]

Nice patch. Could you please also extend the test in TokenAnnotatorTests.cpp?

[rymiel]

Added negation unary operator check to TokenAnnotatorTest

Is this what you meant? Although, I didn't touch the TokenType since it was already correct before i changed anything

[HazardyKnusperkeks]

In D131978#3727084, @rymiel wrote:

Added negation unary operator check to TokenAnnotatorTest

Is this what you meant? Although, I didn't touch the TokenType since it was already correct before i changed anything

It's late (where I am). I thought we had something like ClosesRequiresClause for concepts too.
But on the other hand, this should affect requires clauses, right? So a test for that would fail before your patch.

[rymiel]

In D131978#3727110, @HazardyKnusperkeks wrote:

It's late (where I am). I thought we had something like ClosesRequiresClause for concepts too.
But on the other hand, this should affect requires clauses, right? So a test for that would fail before your patch.

Sorry, I'm not familiar with these tests enough to know what exactly you mean, but thank you for the review
Also, since this is my first change, I cannot commit this myself.

[rymiel]

It seems that even with this patch, there is seemingly weird formatting with the negation in requires clauses, such as:

template <typename T>
  requires !F<T>
           int bar(T t);

This is because the "fake parens" of the unary expression opened is never closed by the special case clean-up made for requires clauses
However, it turns out this isn't even valid syntax, as it requires parentheses for disambiguation. Adding those brings back correct indentation

template <typename T>
  requires(!F<T>)
int bar(T t);

Is it okay to ignore this case, as it wouldn't even compile?

[HazardyKnusperkeks]

In D131978#3729644, @rymiel wrote:

It seems that even with this patch, there is seemingly weird formatting with the negation in requires clauses, such as:

template <typename T>
  requires !F<T>
           int bar(T t);

This is because the "fake parens" of the unary expression opened is never closed by the special case clean-up made for requires clauses
However, it turns out this isn't even valid syntax, as it requires parentheses for disambiguation. Adding those brings back correct indentation

template <typename T>
  requires(!F<T>)
int bar(T t);

Is it okay to ignore this case, as it wouldn't even compile?

Yeah it is okay when invalid code is not formatted "nicely". It wouldn't hurt if we do, but the time can be invested better.

We'll give the others some time to react, I will (most likely) push the commit on the weekend.

[curdeius]

@rymiel, please provide your name and email address for the commit message, so that we can land it for you.

[HazardyKnusperkeks]

Please state a name and email for the commit.

clang/lib/Format/UnwrappedLineParser.cpp

default:
       // We need to differentiate identifiers for a template deduction guide,
       // variables, or function return types (the constraint expression has
       // ended before that), and basically all other cases. But it's easier to
       // check the other way around.
       assert(FormatTok->Previous);
       switch (FormatTok->Previous->Tok.getKind()) {
       case tok::coloncolon:  // Nested identifier.
       case tok::ampamp:      // Start of a function or variable for the
-      case tok::pipepipe:    // constraint expression.
+      case tok::pipepipe:    // constraint expression. (binary)
+      case tok::exclaim:     // The same as above, but unary.
       case tok::kw_requires: // Initial identifier of a requires clause.
       case tok::equal:       // Initial identifier of a concept declaration.
         break;
       default:
         return;
       }
 
       // Read identifier with optional template declaration.

clang/unittests/Format/FormatTest.cpp

verifyFormat("template <typename T>\n"
                "                                 t.baz();\n"
                "                               } && sizeof(T) <= 8;");
 
   verifyFormat("template <typename T>\n"
                "concept DelayedCheck = false || requires(T t) { t.bar(); } && "
                "sizeof(T) <= 8;");
 
   verifyFormat("template <typename T>\n"
+               "concept DelayedCheck = Unit<T> && !DerivedUnit<T>;");
+
+  verifyFormat("template <typename T>\n"
+               "concept DelayedCheck = Unit<T> && !(DerivedUnit<T>);");
+
+  verifyFormat("template <typename T>\n"
+               "concept DelayedCheck = Unit<T> && !!DerivedUnit<T>;");
+
+  verifyFormat("template <typename T>\n"
                "concept DelayedCheck = !!false || requires(T t) { t.bar(); } "
                "&& sizeof(T) <= 8;");
 
   verifyFormat(
       "template <typename T>\n"
       "concept DelayedCheck = static_cast<bool>(0) ||\n"
       "                       requires(T t) { t.bar(); } && sizeof(T) <= 8;");
 

clang/unittests/Format/TokenAnnotatorTest.cpp

auto Tokens = annotate("template <typename T>\n"
                          "concept C = (Foo && Bar) && (Bar && Baz);");
 
   ASSERT_EQ(Tokens.size(), 21u) << Tokens;
   EXPECT_TOKEN(Tokens[10], tok::ampamp, TT_BinaryOperator);
   EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_BinaryOperator);
   EXPECT_TOKEN(Tokens[16], tok::ampamp, TT_BinaryOperator);
 
   Tokens = annotate("template <typename T>\n"
+                    "concept C = Foo && !Bar;");
+
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_TOKEN(Tokens[9], tok::ampamp, TT_BinaryOperator);
+  EXPECT_TOKEN(Tokens[10], tok::exclaim, TT_UnaryOperator);
+
+  Tokens = annotate("template <typename T>\n"
                     "concept C = requires(T t) {\n"
                     "  { t.foo() };\n"
                     "} && Bar<T> && Baz<T>;");
   ASSERT_EQ(Tokens.size(), 35u) << Tokens;
   EXPECT_TOKEN(Tokens[8], tok::kw_requires, TT_RequiresExpression);
   EXPECT_TOKEN(Tokens[9], tok::l_paren, TT_RequiresExpressionLParen);
   EXPECT_TOKEN(Tokens[13], tok::l_brace, TT_RequiresExpressionLBrace);
   EXPECT_TOKEN(Tokens[23], tok::ampamp, TT_BinaryOperator);