[clang-format] Do not insert space after new/delete keywords in C function declarations

[penagos]

Summary

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

[penagos]

This comment was removed by penagos.

[penagos]

The windows build failure appears to be unrelated to this change. I found the need to include !Left.Previous in the conditional so as to account for the existing unittest in the form of:

new (expr)

To not wind up formatting this case as:

new(expr)

Though it's still unclear to me whether or not it'd make more sense to continue to pursue a change to introduce C as a language. Thoughts?

[curdeius]

In D120374#3339337, @penagos wrote:

Though it's still unclear to me whether or not it'd make more sense to continue to pursue a change to introduce C as a language. Thoughts?

I prefer your current approach (and there should be not that many cases that we need a special treatment for C).
Adding a new language would be to disruptive and difficult, as well as hard to use in some cases (headers in C and C++ often share the same extension).

clang/unittests/Format/FormatTest.cpp

9926

Why there's no space after delete in the input?

[penagos]

Add missing space in unittest.

[penagos]

In D120374#3339554, @curdeius wrote:

In D120374#3339337, @penagos wrote:

Though it's still unclear to me whether or not it'd make more sense to continue to pursue a change to introduce C as a language. Thoughts?

I prefer your current approach (and there should be not that many cases that we need a special treatment for C).

Makes sense. I'll also need someone to commit on my behalf:

• Luis Penagos
• [e-mail]

Thanks!

clang/unittests/Format/FormatTest.cpp

9926

Whoops, good catch.

[hans]

This seems to have caused https://github.com/llvm/llvm-project/issues/54703

clang/lib/Format/TokenAnnotator.cpp

 //===--- TokenAnnotator.cpp - Format C++ code -----------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
if (Right.is(tok::l_paren)) {
     if (Line.Type != LT_PreprocessorDirective && Left.is(tok::r_square) &&
         Left.MatchingParen && Left.MatchingParen->is(TT_LambdaLSquare))
       return Style.SpaceBeforeParensOptions.AfterFunctionDefinitionName ||
              spaceRequiredBeforeParens(Right);
     if (!Left.Previous || Left.Previous->isNot(tok::period)) {
       if (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch))
         return Style.SpaceBeforeParensOptions.AfterControlStatements ||
                spaceRequiredBeforeParens(Right);
-      if (Left.isOneOf(tok::kw_new, tok::kw_delete) ||
-          (Left.is(tok::r_square) && Left.MatchingParen &&
-           Left.MatchingParen->Previous &&
-           Left.MatchingParen->Previous->is(tok::kw_delete)))
-        return Style.SpaceBeforeParens != FormatStyle::SBPO_Never ||
+      if (Left.isOneOf(tok::kw_new, tok::kw_delete))
+        return ((!Line.MightBeFunctionDecl || !Left.Previous) &&
+                Style.SpaceBeforeParens != FormatStyle::SBPO_Never) ||
+               spaceRequiredBeforeParens(Right);
+
+      if (Left.is(tok::r_square) && Left.MatchingParen &&
+          Left.MatchingParen->Previous->is(tok::kw_delete))
+        return (Style.SpaceBeforeParens != FormatStyle::SBPO_Never) ||
                spaceRequiredBeforeParens(Right);
     }
     if (Line.Type != LT_PreprocessorDirective &&
         (Left.is(tok::identifier) || Left.isFunctionLikeKeyword() ||
          Left.is(tok::r_paren) || Left.isSimpleTypeSpecifier()))
       return spaceRequiredBeforeParens(Right);
     return false;
   }

clang/unittests/Format/FormatTest.cpp

 //===- unittest/Format/FormatTest.cpp - Formatting unit tests -------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
verifyFormat("auto aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa =\n"
                "        typename aaaaaaaaaaaaaaaaaaaaaaaa();");
   verifyFormat("delete[] h->p;");
   verifyFormat("delete[] (void *)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;");
+
+  EXPECT_EQ("void new(link p);\n"
+            "void delete(link p);\n",
+            format("void new (link p);\n"
+                   "void delete(link p);\n"));
 }
 
 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); }");