[clang-format] fix regression recognizing casts in Obj-C calls

[krasimir]

Summary

r373922 added checks for a few tokens that, following an ) make it
unlikely that the ) is the closing paren of a cast expression. The
specific check for tok::l_square there introduced a regression for
casts of Obj-C calls, like:

(cast)[func arg]

From the tests added in r373922, I believe the tok::l_square case is added to
capture the case where a non-cast ) is directly followed by an
attribute specifier, like:

int f(int x) [[noreturn]];

I've specialized the code to look for such attribute specifier instead
of tok::l_square in general. Also, I added a regression test and moved
the test cases added in r373922 to an already existing place documenting
other instances of historically misidentified casts.

[MyDeveloperDay]

LGTM, thanks for the patch

[rdwampler]

lib/Format/TokenAnnotator.cpp

1612 ↗

(On Diff #225604)

I think the issue r373922 was fixing is only related to the delete. Can this check be move further up where we explicitly check if the token is the delete keyword?

[krasimir]

lib/Format/TokenAnnotator.cpp

1612 ↗

(On Diff #225604)

Sorry, I didn't notice this comment until now. I can investigate whether all the other cases for non-delete methods are already covered, it would be interesting. + @MyDeveloperDay, who might have better insights into this.

[MyDeveloperDay]

rC373922: [clang-format] [PR27004] omits leading space for noexcept when formatting… was to fix https://bugs.llvm.org/show_bug.cgi?id=27004 which wasn't just related to delete it occurred in other cases. (operator++) and there could be other cases i guess.

I think the check for delete above the code that was added was checking for delete being on the left i.e. delete() and delete(x) is not a cast (sorry I wasn't the author for that part, so might not be correct)

However, I appreciate this patch (or any patch for that matter) that strengthens our tests in this way.

clang/lib/Format/TokenAnnotator.cpp

bool rParenEndsCast(const FormatToken &Tok) {
     }
 
     if (Tok.Next->is(tok::question))
       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_throw, tok::l_square, tok::arrow,
-                          Keywords.kw_override, Keywords.kw_final))
+                          tok::kw_throw, tok::arrow, Keywords.kw_override,
+                          Keywords.kw_final) ||
+        isCpp11AttributeSpecifier(*Tok.Next))
       return false;
 
     // As Java has no function types, a "(" after the ")" likely means that this
     // is a cast.
     if (Style.Language == FormatStyle::LK_Java && Tok.Next->is(tok::l_paren))
       return true;
 
     // If a (non-string) literal follows, this is likely a cast.

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, FormatsCasts) {
   verifyFormat("my_int a = (my_int)1;");
   verifyFormat("my_int a = (my_int *)1;");
   verifyFormat("my_int a = (const my_int)-1;");
   verifyFormat("my_int a = (const my_int *)-1;");
   verifyFormat("my_int a = (my_int)(my_int)-1;");
   verifyFormat("my_int a = (ns::my_int)-2;");
   verifyFormat("case (my_int)ONE:");
   verifyFormat("auto x = (X)this;");
+  // Casts in Obj-C style calls used to not be recognized as such.
+  verifyFormat("int a = [(type*)[((type*)val) arg] arg];", getGoogleStyle());
 
   // FIXME: single value wrapped with paren will be treated as cast.
   verifyFormat("void f(int i = (kValue)*kMask) {}");
 
   verifyFormat("{ (void)F; }");
 
   // Don't break after a cast's
   verifyFormat("int aaaaaaaaaaaaaaaaaaaaaaaaaaa =\n"
TEST_F(FormatTest, FormatsCasts) {
   // These are not casts, but at some point were confused with casts.
   verifyFormat("virtual void foo(int *) override;");
   verifyFormat("virtual void foo(char &) const;");
   verifyFormat("virtual void foo(int *a, char *) const;");
   verifyFormat("int a = sizeof(int *) + b;");
   verifyFormat("int a = alignof(int *) + b;", getGoogleStyle());
   verifyFormat("bool b = f(g<int>) && c;");
   verifyFormat("typedef void (*f)(int i) func;");
+  verifyFormat("void operator++(int) noexcept;");
+  verifyFormat("void operator++(int &) noexcept;");
+  verifyFormat("void operator delete(void *, std::size_t, const std::nothrow_t "
+               "&) noexcept;");
+  verifyFormat(
+      "void operator delete(std::size_t, const std::nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(const std::nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(std::nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(foo &) noexcept;");
+  verifyFormat("void operator delete(foo) noexcept;");
+  verifyFormat("void operator delete(int) noexcept;");
+  verifyFormat("void operator delete(int &) noexcept;");
+  verifyFormat("void operator delete(int &) volatile noexcept;");
+  verifyFormat("void operator delete(int &) const");
+  verifyFormat("void operator delete(int &) = default");
+  verifyFormat("void operator delete(int &) = delete");
+  verifyFormat("void operator delete(int &) [[noreturn]]");
+  verifyFormat("void operator delete(int &) throw();");
+  verifyFormat("void operator delete(int &) throw(int);");
+  verifyFormat("auto operator delete(int &) -> int;");
+  verifyFormat("auto operator delete(int &) override");
+  verifyFormat("auto operator delete(int &) final");
 
   verifyFormat("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa *foo = (aaaaaaaaaaaaaaaaa *)\n"
                "    bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb;");
   // FIXME: The indentation here is not ideal.
   verifyFormat(
       "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\n"
       "    [bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb] = (*cccccccccccccccc)\n"
       "        [dddddddddddddddddddddddddddddddddddddddddddddddddddddddd];");
TEST_F(FormatTest, AlternativeOperators) {
   verifyFormat("foo() <%%>;");      // foo();
   verifyFormat("void foo() <%%>;"); // void foo(){}
   verifyFormat("int a <:1:>;");     // int a[1];[
   verifyFormat("%:define ABC abc"); // #define ABC abc
   verifyFormat("%:%:");             // ##
   */
 }
 
-TEST_F(FormatTest, NotCastRPaen) {
-
-  verifyFormat("void operator++(int) noexcept;");
-  verifyFormat("void operator++(int &) noexcept;");
-  verifyFormat("void operator delete(void *, std::size_t, const std::nothrow_t "
-               "&) noexcept;");
-  verifyFormat(
-      "void operator delete(std::size_t, const std::nothrow_t &) noexcept;");
-  verifyFormat("void operator delete(const std::nothrow_t &) noexcept;");
-  verifyFormat("void operator delete(std::nothrow_t &) noexcept;");
-  verifyFormat("void operator delete(nothrow_t &) noexcept;");
-  verifyFormat("void operator delete(foo &) noexcept;");
-  verifyFormat("void operator delete(foo) noexcept;");
-  verifyFormat("void operator delete(int) noexcept;");
-  verifyFormat("void operator delete(int &) noexcept;");
-  verifyFormat("void operator delete(int &) volatile noexcept;");
-  verifyFormat("void operator delete(int &) const");
-  verifyFormat("void operator delete(int &) = default");
-  verifyFormat("void operator delete(int &) = delete");
-  verifyFormat("void operator delete(int &) [[noreturn]]");
-  verifyFormat("void operator delete(int &) throw();");
-  verifyFormat("void operator delete(int &) throw(int);");
-  verifyFormat("auto operator delete(int &) -> int;");
-  verifyFormat("auto operator delete(int &) override");
-  verifyFormat("auto operator delete(int &) final");
-}
-
 TEST_F(FormatTest, STLWhileNotDefineChed) {
   verifyFormat("#if defined(while)\n"
                "#define while EMIT WARNING C4005\n"
                "#endif // while");
 }
 
 } // namespace
 } // namespace format
 } // namespace clang