[clang-format] Avoid inserting space after C++ casts.

[curdeius]

Summary

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

This is a solution for standard C++ casts: const_cast, dynamic_cast, reinterpret_cast, static_cast.

A general approach handling all possible casts is not possible without semantic information.
Consider the code:

static_cast<T>(*function_pointer_variable)(arguments);

vs.

some_return_type<T> (*function_pointer_variable)(parameters);
// Later used as:
function_pointer_variable = &some_function;
return function_pointer_variable(args);

In the latter case, it's not a cast but a variable declaration of a pointer to function.
Without knowing what some_return_type<T> is (and clang-format does not know it), it's hard to distinguish between the two cases. Theoretically, one could check whether "parameters" are types (not a cast) and "arguments" are value/expressions (a cast), but that might be inefficient (needs lots of lookahead).

[curdeius]

clang/lib/Format/TokenAnnotator.cpp

1744

To add some context, in the failing cases, the opening parenthesis was set to type TT_FunctionTypeLParen that we don't want because combined with a * after the next paren, it adds a space (cf. https://github.com/llvm/llvm-project/blob/331e8e4e27be5dd673898a89a7cf00e76903216a/clang/lib/Format/TokenAnnotator.cpp#L3105-L3109).

[lichray]

clang/lib/Format/TokenAnnotator.cpp

1971

Very unlikely bit_cast gives you something callable; any_cast may. I think keywords are ok for now, and cheap. Others may need a different strategy.

[curdeius]

clang/lib/Format/TokenAnnotator.cpp

1971

Indeed.

[curdeius]

This commit provokes failures in formatting tests of polly.
Cf. https://lab.llvm.org/buildbot/#/builders/205/builds/3320.

That's probably because of ) being annotated as CastRParen instead of Unknown before, hence being kept on the same line with the next token.

[curdeius]

• Fix. Add tests from inspired by polly and lib/Format that had misformats.

[curdeius]

I took another approach to fix the problem found in polly tests.
I added CppCastLParen kind so as not to interfere with C-style casts.
I also added a test case taken from lib/Format that got misformatted by the previous version (if (static_cast<int>(A) + B >= 0)\n ; got the spaces around + removed: if (static_cast<int>(A)+B >= 0)\n ;).

clang/lib/Format/FormatToken.h

#define LIST_TOKEN_TYPES                                                       \
   TYPE(BracedListLBrace)                                                       \
   TYPE(CastRParen)                                                             \
   TYPE(ClassLBrace)                                                            \
   TYPE(CompoundRequirementLBrace)                                              \
   TYPE(ConditionalExpr)                                                        \
   TYPE(ConflictAlternative)                                                    \
   TYPE(ConflictEnd)                                                            \
   TYPE(ConflictStart)                                                          \
+  TYPE(CppCastLParen)                                                          \
   TYPE(CtorInitializerColon)                                                   \
   TYPE(CtorInitializerComma)                                                   \
   TYPE(DesignatedInitializerLSquare)                                           \
   TYPE(DesignatedInitializerPeriod)                                            \
   TYPE(DictLiteral)                                                            \
   TYPE(EnumLBrace)                                                             \
   TYPE(FatArrow)                                                               \
   TYPE(ForEachMacro)                                                           \

clang/lib/Format/TokenAnnotator.cpp

while (CurrentToken) {
           CurrentToken->Previous->Previous->isOneOf(tok::l_paren,
                                                     tok::coloncolon))
         ProbablyFunctionType = true;
       if (CurrentToken->is(tok::comma))
         MightBeFunctionType = false;
       if (CurrentToken->Previous->is(TT_BinaryOperator))
         Contexts.back().IsExpression = true;
       if (CurrentToken->is(tok::r_paren)) {
-        if (MightBeFunctionType && ProbablyFunctionType && CurrentToken->Next &&
+        if (Left->isNot(TT_CppCastLParen) && MightBeFunctionType &&
+            ProbablyFunctionType && CurrentToken->Next &&
             (CurrentToken->Next->is(tok::l_paren) ||
              (CurrentToken->Next->is(tok::l_square) && Line.MustBeDeclaration)))
           Left->setType(Left->Next->is(tok::caret) ? TT_ObjCBlockLParen
                                                    : TT_FunctionTypeLParen);
         Left->MatchingParen = CurrentToken;
         CurrentToken->MatchingParen = Left;
 
         if (CurrentToken->Next && CurrentToken->Next->is(tok::l_brace) &&
if (Current.is(Keywords.kw_instanceof)) {
         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.
           Current.Tok.setKind(tok::unknown);
       else
         Current.setType(TT_LineComment);
+    } else if (Current.is(tok::l_paren)) {
+      if (lParenStartsCppCast(Current))
+        Current.setType(TT_CppCastLParen);
     } else if (Current.is(tok::r_paren)) {
       if (rParenEndsCast(Current))
         Current.setType(TT_CastRParen);
       if (Current.MatchingParen && Current.Next &&
           !Current.Next->isBinaryOperator() &&
           !Current.Next->isOneOf(tok::semi, tok::colon, tok::l_brace,
                                  tok::comma, tok::period, tok::arrow,
                                  tok::coloncolon))
         if (FormatToken *AfterParen = Current.MatchingParen->Next) {
           // Make sure this isn't the return type of an Obj-C block declaration
           if (AfterParen->isNot(tok::caret)) {
             if (FormatToken *BeforeParen = Current.MatchingParen->Previous)
               if (BeforeParen->is(tok::identifier) &&
bool isStartOfName(const FormatToken &Tok) {
     // MyClass a;
     if (PreviousNotConst->isSimpleTypeSpecifier())
       return true;
 
     // const a = in JavaScript.
     return Style.isJavaScript() && PreviousNotConst->is(tok::kw_const);
   }
 
+  /// Determine whether '(' is starting a C++ cast.
+  bool lParenStartsCppCast(const FormatToken &Tok) {
+    // C-style casts are only used in C++.
+    if (!Style.isCpp())
+      return false;
+
+    FormatToken *LeftOfParens = Tok.getPreviousNonComment();
+    if (LeftOfParens && LeftOfParens->is(TT_TemplateCloser) &&
+        LeftOfParens->MatchingParen) {
+      auto *Prev = LeftOfParens->MatchingParen->getPreviousNonComment();
+      if (Prev && Prev->isOneOf(tok::kw_const_cast, tok::kw_dynamic_cast,
+                                tok::kw_reinterpret_cast, tok::kw_static_cast))
+        // FIXME: Maybe we should handle identifiers ending with "_cast",
+        // e.g. any_cast?
+        return true;
+    }
+    return false;
+  }
+
   /// Determine whether ')' is ending a cast.
   bool rParenEndsCast(const FormatToken &Tok) {
     // C-style casts are only used in C++, C# and Java.
     if (!Style.isCSharp() && !Style.isCpp() &&
         Style.Language != FormatStyle::LK_Java)
       return false;
 
     // Empty parens aren't casts and there are no casts at the end of the line.
if (LeftOfParens) {
       // this cannot be a cast.
       if (LeftOfParens->isOneOf(tok::at, tok::r_square, TT_OverloadedOperator,
                                 TT_TemplateCloser, tok::ellipsis))
         return false;
     }
 
     if (Tok.Next->is(tok::question))
       return false;
 
     // `foreach((A a, B b) in someList)` should not be seen as a cast.
     if (Tok.Next->is(Keywords.kw_in) && Style.isCSharp())
       return false;
 
     // Functions which end with decorations like volatile, noexcept are unlikely
     // to be casts.
     if (Tok.Next->isOneOf(tok::kw_noexcept, tok::kw_volatile, tok::kw_const,
                           tok::kw_requires, tok::kw_throw, tok::arrow,

clang/unittests/Format/FormatTest.cpp

verifyFormat("void f() {\n"
                "  x[aaaaaaaaa -\n"
                "    b] = 23;\n"
                "}",
                getLLVMStyleWithColumns(15));
 }
 
 TEST_F(FormatTest, FormatsCasts) {
   verifyFormat("Type *A = static_cast<Type *>(P);");
+  verifyFormat("static_cast<Type *>(P);");
+  verifyFormat("static_cast<Type &>(Fun)(Args);");
+  verifyFormat("static_cast<Type &>(*Fun)(Args);");
+  verifyFormat("if (static_cast<int>(A) + B >= 0)\n  ;");
+  // Check that static_cast<...>(...) does not require the next token to be on
+  // the same line.
+  verifyFormat("some_loooong_output << something_something__ << "
+               "static_cast<const void *>(R)\n"
+               "                    << something;");
+  verifyFormat("a = static_cast<Type &>(*Fun)(Args);");
+  verifyFormat("const_cast<Type &>(*Fun)(Args);");
+  verifyFormat("dynamic_cast<Type &>(*Fun)(Args);");
+  verifyFormat("reinterpret_cast<Type &>(*Fun)(Args);");
   verifyFormat("Type *A = (Type *)P;");
   verifyFormat("Type *A = (vector<Type *, int *>)P;");
   verifyFormat("int a = (int)(2.0f);");
   verifyFormat("int a = (int)2.0f;");
   verifyFormat("x[(int32)y];");
   verifyFormat("x = (int32)y;");
   verifyFormat("#define AA(X) sizeof(((X *)NULL)->a)");
   verifyFormat("int a = (int)*b;");