clang-format: Pointer `*` is seen as multiplication in C-style casts

[mxbOctasic]

Summary

The * was treated as multiplication operator in complex pointer type casts.

e.g.
(type *const)bar
(type *restrict)bar

Patch by cameron314

[djasper]

lib/Format/TokenAnnotator.cpp

1277

I think it would be better to not have such cast detection heuristics in several places. Could we change the type of the "*" to TT_PointerOrReference after the fact, when rParenEndsCast returns true?

[mxbOctasic]

Moved cast detection logic to rParenEndsCast.

lib/Format/TokenAnnotator.cpp

if (Current.is(Keywords.kw_instanceof)) {
         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.Type = TT_LineComment;
       }
     } else if (Current.is(tok::r_paren)) {
-      if (rParenEndsCast(Current))
+      if (rParenEndsCast(Current)) {
         Current.Type = TT_CastRParen;
+        for (FormatToken *tok = Current.MatchingParen; tok && tok != &Current;
+             tok = tok->Next)
+          // `*` in casts are never binary operators
+          if (tok->is(tok::star) && tok->is(TT_BinaryOperator))
+            tok->Type = TT_PointerOrReference;
+      }
       if (Current.MatchingParen && Current.Next &&
           !Current.Next->isBinaryOperator() &&
           !Current.Next->isOneOf(tok::semi, tok::colon, tok::l_brace,
                                  tok::period, tok::arrow, tok::coloncolon))
         if (FormatToken *BeforeParen = Current.MatchingParen->Previous)
           if (BeforeParen->is(tok::identifier) &&
               BeforeParen->TokenText == BeforeParen->TokenText.upper() &&
               (!BeforeParen->Previous ||
bool ParensAreType =
         !Tok.Previous ||
         Tok.Previous->isOneOf(TT_PointerOrReference, TT_TemplateCloser) ||
         Tok.Previous->isSimpleTypeSpecifier();
     bool ParensCouldEndDecl =
         Tok.Next->isOneOf(tok::equal, tok::semi, tok::l_brace, tok::greater);
     if (ParensAreType && !ParensCouldEndDecl)
       return true;
 
+    // Some more obscure casts can leak through the above heuristic, like
+    // (type* const)bar or (type* MACRO)(bar)
+    if (Tok.Next->isOneOf(tok::identifier, tok::l_paren) && Tok.Previous &&
+        Tok.Previous->isOneOf(tok::kw_const, tok::identifier) &&
+        Tok.Previous->Previous && Tok.Previous->Previous->is(tok::star))
+      return true;
+
     // At this point, we heuristically assume that there are no casts at the
     // start of the line. We assume that we have found most cases where there
     // are by the logic above, e.g. "(void)x;".
     if (!LeftOfParens)
       return false;
 
     // If the following token is an identifier or 'this', this is a cast. All
     // cases where this can be something else are handled above.
TokenType determineStarAmpUsage(const FormatToken &Tok, bool IsExpression,
 
     // This catches some cases where evaluation order is used as control flow:
     //   aaa && aaa->f();
     const FormatToken *NextNextToken = NextToken->getNextNonComment();
     if (NextNextToken && NextNextToken->is(tok::arrow))
       return TT_BinaryOperator;
 
     // It is very unlikely that we are going to find a pointer or reference type
     // definition on the RHS of an assignment.
     if (IsExpression && !Contexts.back().CaretFound)
       return TT_BinaryOperator;
 
     return TT_PointerOrReference;
   }
 
   TokenType determinePlusMinusCaretUsage(const FormatToken &Tok) {
     const FormatToken *PrevToken = Tok.getPreviousNonComment();

unittests/Format/FormatTest.cpp

TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
 
   // Member function reference qualifiers aren't binary operators.
   verifyFormat("string // break\n"
                "operator()() & {}");
   verifyFormat("string // break\n"
                "operator()() && {}");
   verifyGoogleFormat("template <typename T>\n"
                      "auto x() & -> int {}");
+
+  // Some casts look like they might be multiplications, but they are definitely
+  // not!
+  // Here, foo could be a macro containing e.g. `const`
+  verifyFormat("(type *foo)bar");
+  verifyFormat("(type *const)bar");
+  verifyFormat("(type *restrict)bar");
+  verifyFormat("(type *foo)(bar)");
+  verifyFormat("(type *const)(bar)");
+  verifyFormat("(type *restrict)(bar)");
+  verifyFormat("type *x = (type *foo)bar");
+  verifyFormat("type *const x = (type *const)bar");
+  verifyFormat("type *x = (type *foo)(bar)");
+
+  FormatStyle LeftPointer = getLLVMStyle();
+  LeftPointer.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("int x = a[(sint16*)pIn - j]", LeftPointer);
+  verifyFormat("int x = a[(int*)pIn - j]", LeftPointer);
 }
 
 TEST_F(FormatTest, UnderstandsAttributes) {
   verifyFormat("SomeType s __attribute__((unused)) (InitValue);");
   verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa __attribute__((unused))\n"
                "aaaaaaaaaaaaaaaaaaaaaaa(int i);");
   FormatStyle AfterType = getLLVMStyle();
   AfterType.AlwaysBreakAfterReturnType = FormatStyle::RTBS_AllDefinitions;