[clang-format] clang-format eats space in front of attributes for operator delete

[MyDeveloperDay]

Summary

https://github.com/llvm/llvm-project/issues/27037

Sorry its taken so long to get to this issue! (got it before it hit its 6th birthday!)

void operator delete(void *foo)ATTRIB;

(void *foo) is incorrectly determined to be a C-Style Cast resulting in the space being removed after the ) and before the attrib, due to the detection of

delete (A* )a;

The following was previously unaffected

void operator new(void *foo) ATTRIB;

Fixes #27037

[curdeius]

clang/lib/Format/TokenAnnotator.cpp

1729

The current solution looks a bit like a hack to me.
Unless I'm mistaken, casts can appear only in expressions, but not in declarations (except for inside decltype stuff in templates or concepts, but these are still expressions at this level).
Given that void operator delete... is not an expression but a declaration, we shouldn't check rParenEndsCast in this case at all, no?
So, would it be possible to check here for e.g. Line.MightBeFunctionDecl or something like this to avoid it?

Also, how is it that you don't need to special-case new operator?

[MyDeveloperDay]

clang/lib/Format/TokenAnnotator.cpp

1729

I will check to see if we can use that..

To answer your second question, the existing code was this...

!isOneOf(Keywords.kw_in, tok::kw_return, tok::kw_case,tok::kw_delete)

its the kw_delete here that breaks the previous code.

to cover this being a cast

delete (A *)a;

my change is to use more context by avoid operator delete (A* a) a; from being seen as a cast

[curdeius]

clang/unittests/Format/FormatTest.cpp

9463

Please add a test for operator delete[].

[MyDeveloperDay]

Line.MightBeFunctionDecl is not true in this case

Added new tests

[MyDeveloperDay]

clang-format

[MyDeveloperDay]

remove additional space

[curdeius]

LGTM. Don't forget to reformat.

It's a pity though that we cannot use MightBeFunctionDecl.

clang/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.setType(TT_LineComment);
       }
     } 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 (LeftOfParens) {
       // parentheses, look past it as these might be chained casts.
       if (LeftOfParens->is(tok::r_paren) &&
           LeftOfParens->isNot(TT_CastRParen)) {
         if (!LeftOfParens->MatchingParen ||
             !LeftOfParens->MatchingParen->Previous)
           return false;
         LeftOfParens = LeftOfParens->MatchingParen->Previous;
       }
       
+      // The Condition directly below this one will see the operator arguments
+      // as a (void *foo) cast.
+      //   void operator delete(void *foo) ATTRIB;
+      if (LeftOfParens->Tok.getIdentifierInfo() &&
+          LeftOfParens->Previous &&
+          LeftOfParens->Previous->is(tok::kw_operator))
+        return false;        
+
       // If there is an identifier (or with a few exceptions a keyword) right
       // before the parentheses, this is unlikely to be a cast.
       if (LeftOfParens->Tok.getIdentifierInfo() &&
           !LeftOfParens->isOneOf(Keywords.kw_in, tok::kw_return, tok::kw_case,
                                  tok::kw_delete))
         return false;
 
       // Certain other tokens right before the parentheses are also signals that

clang/unittests/Format/FormatTest.cpp

verifyFormat("void f() {\n"
                "  delete (A *)a;\n"
                "}");
   verifyFormat("new (aaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaa))\n"
                "    typename aaaaaaaaaaaaaaaaaaaaaaaa();");
   verifyFormat("auto aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa =\n"
                "    new (aaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaaaa))\n"
                "        typename aaaaaaaaaaaaaaaaaaaaaaaa();");
   verifyFormat("delete[] h->p;");
+
+  verifyFormat("void operator delete(void *foo) ATTRIB;");
+  verifyFormat("void operator new(void *foo) ATTRIB;");
+  verifyFormat("void operator delete[](void *foo) ATTRIB;");
+  verifyFormat("void operator delete(void* ptr ) noexcept;");
 }
 
 TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
   verifyFormat("int *f(int *a) {}");
   verifyFormat("int main(int argc, char **argv) {}");
   verifyFormat("Test::Test(int b) : a(b * b) {}");
   verifyIndependentOfContext("f(a, *a);");
   verifyFormat("void g() { f(*a); }");