[clang-format] Detect pointer qualifiers in cast expressions

[arichardson]

Summary

When guessing whether a closing paren is then end of a cast expression also
skip over pointer qualifiers while looking for TT_PointerOrReference.
This prevents some address-of and dereference operators from being parsed
as a binary operator.

Before:
x = (foo *const) * v;
x = (foo *const volatile restrict attribute((foo)) _Nonnull _Null_unspecified _Nonnull) & v;

After:
x = (foo *const)*v;
x = (foo *const volatile restrict attribute((foo)) _Nonnull _Null_unspecified _Nonnull)&v;

[arichardson]

drop unncessary const from test

[MyDeveloperDay]

This LGTM thank you

clang/lib/Format/FormatToken.h

bool isObjCAtKeyword(tok::ObjCKeywordKind Kind) const {
     return Tok.isObjCAtKeyword(Kind);
   }
 
   bool isAccessSpecifier(bool ColonRequired = true) const {
     return isOneOf(tok::kw_public, tok::kw_protected, tok::kw_private) &&
            (!ColonRequired || (Next && Next->is(tok::colon)));
   }
 
+  bool canBePointerOrReferenceQualifier() const {
+    return isOneOf(tok::kw_const, tok::kw_restrict, tok::kw_volatile,
+                   tok::kw___attribute, tok::kw__Nonnull, tok::kw__Nullable,
+                   tok::kw__Null_unspecified);
+  }
+
   /// Determine whether the token is a simple-type-specifier.
   bool isSimpleTypeSpecifier() const;
 
   bool isObjCAccessSpecifier() const {
     return is(tok::at) && Next &&
            (Next->isObjCAtKeyword(tok::objc_public) ||
             Next->isObjCAtKeyword(tok::objc_protected) ||
             Next->isObjCAtKeyword(tok::objc_package) ||

clang/lib/Format/TokenAnnotator.cpp

bool rParenEndsCast(const FormatToken &Tok) {
 
     // If a (non-string) literal follows, this is likely a cast.
     if (Tok.Next->isNot(tok::string_literal) &&
         (Tok.Next->Tok.isLiteral() ||
          Tok.Next->isOneOf(tok::kw_sizeof, tok::kw_alignof)))
       return true;
 
     // Heuristically try to determine whether the parentheses contain a type.
-    bool ParensAreType =
-        !Tok.Previous ||
-        Tok.Previous->isOneOf(TT_PointerOrReference, TT_TemplateCloser) ||
-        Tok.Previous->isSimpleTypeSpecifier();
+    auto IsQualifiedPointerOrReference = [](FormatToken *T) {
+      // This is used to handle cases such as x = (foo *const)&y;
+      assert(!T->isSimpleTypeSpecifier() && "Should have already been checked");
+      // Strip trailing qualifiers such as const or volatile when checking
+      // whether the parens could be a cast to a pointer/reference type.
+      while (T) {
+        if (T->is(TT_AttributeParen)) {
+          // Handle `x = (foo *__attribute__((foo)))&v;`:
+          if (T->MatchingParen && T->MatchingParen->Previous &&
+              T->MatchingParen->Previous->is(tok::kw___attribute)) {
+            T = T->MatchingParen->Previous->Previous;
+            continue;
+          }
+        } else if (T->canBePointerOrReferenceQualifier()) {
+          T = T->Previous;
+          continue;
+        }
+        break;
+      }
+      return T && T->is(TT_PointerOrReference);
+    };
+    bool ParensAreType = !Tok.Previous || Tok.Previous->is(TT_TemplateCloser) ||
+                         Tok.Previous->isSimpleTypeSpecifier() ||
+                         IsQualifiedPointerOrReference(Tok.Previous);
     bool ParensCouldEndDecl =
         Tok.Next->isOneOf(tok::equal, tok::semi, tok::l_brace, tok::greater);
     if (ParensAreType && !ParensCouldEndDecl)
       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 (Style.isCSharp() && Tok.is(tok::ampamp))
       return TT_BinaryOperator;
 
     const FormatToken *PrevToken = Tok.getPreviousNonComment();
     if (!PrevToken)
       return TT_UnaryOperator;
 
     const FormatToken *NextToken = Tok.getNextNonComment();
     if (!NextToken ||
-        NextToken->isOneOf(
-            tok::arrow, tok::equal, tok::kw_const, tok::kw_restrict,
-            tok::kw_volatile, tok::kw___attribute, tok::kw__Nonnull,
-            tok::kw__Nullable, tok::kw__Null_unspecified, tok::kw_noexcept) ||
+        NextToken->isOneOf(tok::arrow, tok::equal, tok::kw_noexcept) ||
+        NextToken->canBePointerOrReferenceQualifier() ||
         (NextToken->is(tok::l_brace) && !NextToken->getNextNonComment()))
       return TT_PointerOrReference;
 
     if (PrevToken->is(tok::coloncolon))
       return TT_PointerOrReference;
 
     if (PrevToken->isOneOf(tok::l_paren, tok::l_square, tok::l_brace,
                            tok::comma, tok::semi, tok::kw_return, tok::colon,

clang/unittests/Format/FormatTest.cpp

verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa __attribute__((unused))\n"
                "aaaaaaaaaaaaaaaaaaaaaaa(int i);");
   FormatStyle AfterType = getLLVMStyle();
   AfterType.AlwaysBreakAfterReturnType = FormatStyle::RTBS_AllDefinitions;
   verifyFormat("__attribute__((nodebug)) void\n"
                "foo() {}\n",
                AfterType);
 }
 
+TEST_F(FormatTest, UnderstandsPointerQualifiersInCast) {
+  // Check that qualifiers on pointers don't break parsing of casts.
+  verifyFormat("x = (foo *const)*v;");
+  verifyFormat("x = (foo *volatile)*v;");
+  verifyFormat("x = (foo *restrict)*v;");
+  verifyFormat("x = (foo *__attribute__((foo)))*v;");
+  verifyFormat("x = (foo *_Nonnull)*v;");
+  verifyFormat("x = (foo *_Nullable)*v;");
+  verifyFormat("x = (foo *_Null_unspecified)*v;");
+  verifyFormat("x = (foo *_Nonnull)*v;");
+
+  // Check that we handle multiple trailing qualifiers and skip them all to
+  // determine that the expression is a cast to a pointer type.
+  FormatStyle LongPointerRight = getLLVMStyleWithColumns(999);
+  FormatStyle LongPointerLeft = getLLVMStyleWithColumns(999);
+  LongPointerLeft.PointerAlignment = FormatStyle::PAS_Left;
+  StringRef AllQualifiers = "const volatile restrict __attribute__((foo)) "
+                            "_Nonnull _Null_unspecified _Nonnull";
+  verifyFormat(("x = (foo *" + AllQualifiers + ")*v;").str(), LongPointerRight);
+  verifyFormat(("x = (foo* " + AllQualifiers + ")*v;").str(), LongPointerLeft);
+
+  // Also check that address-of is not parsed as a binary bitwise-and:
+  verifyFormat("x = (foo *const)&v;");
+  verifyFormat(("x = (foo *" + AllQualifiers + ")&v;").str(), LongPointerRight);
+  verifyFormat(("x = (foo* " + AllQualifiers + ")&v;").str(), LongPointerLeft);
+}
+
 TEST_F(FormatTest, UnderstandsSquareAttributes) {
   verifyFormat("SomeType s [[unused]] (InitValue);");
   verifyFormat("SomeType s [[gnu::unused]] (InitValue);");
   verifyFormat("SomeType s [[using gnu: unused]] (InitValue);");
   verifyFormat("[[gsl::suppress(\"clang-tidy-check-name\")]] void f() {}");
   verifyFormat("void f() [[deprecated(\"so sorry\")]];");
   verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
                "    [[unused]] aaaaaaaaaaaaaaaaaaaaaaa(int i);");