[clang-format] Fix SpaceBeforeParens with explicit template instantiations

[bdunkin]

This fixes explicit template instantiated functions not having spaces added/removed based on the value of SpaceBeforeParens.

Attribution Note - I have been authorized to contribute this change on behalf of my company: ArenaNet LLC

[llvmbot]

@llvm/pr-subscribers-clang-format

Author: Ben Dunkin (bdunkin)

Changes

This fixes explicit template instantiated functions not having spaces added/removed based on the value of SpaceBeforeParens.

Attribution Note - I have been authorized to contribute this change on behalf of my company: ArenaNet LLC

---

Full diff: https://github.com/llvm/llvm-project/pull/183183.diff

2 Files Affected:

• (modified) clang/lib/Format/TokenAnnotator.cpp (+3-1)
• (modified) clang/unittests/Format/FormatTest.cpp (+4)

diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 34e81bbc97578..8f235cfd6458c 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -4913,8 +4913,10 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
     return true;
   // Space before parentheses common for all languages
   if (Right.is(tok::l_paren)) {
-    if (Left.is(TT_TemplateCloser) && Right.isNot(TT_FunctionTypeLParen))
+    if (Left.is(TT_TemplateCloser) && Right.isNot(TT_FunctionTypeLParen) &&
+        !Line.MightBeFunctionDecl) {
       return spaceRequiredBeforeParens(Right);
+    }
     if (Left.isOneOf(TT_RequiresClause,
                      TT_RequiresClauseInARequiresExpression)) {
       return Style.SpaceBeforeParensOptions.AfterRequiresInClause ||
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 43633b582a8ab..dfc21bcefad53 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -17334,6 +17334,8 @@ TEST_F(FormatTest, ConfigurableSpaceBeforeParens) {
   verifyFormat("void f(int a, T b) {}", SpaceFuncDecl);
   verifyFormat("void __attribute__((asdf)) f(int a, T b) {}", SpaceFuncDecl);
   verifyFormat("A::A() : a(1) {}", SpaceFuncDecl);
+  verifyFormat("template <> void A<C> (C x);", SpaceFuncDecl);
+  verifyFormat("template <> void A<C>(C x) {}", SpaceFuncDecl);
   verifyFormat("void f () __attribute__((asdf));", SpaceFuncDecl);
   verifyFormat("void __attribute__((asdf)) f ();", SpaceFuncDecl);
   verifyFormat("#define A(x) x", SpaceFuncDecl);
@@ -17370,6 +17372,8 @@ TEST_F(FormatTest, ConfigurableSpaceBeforeParens) {
   verifyFormat("void f (int a, T b) {}", SpaceFuncDef);
   verifyFormat("void __attribute__((asdf)) f (int a, T b) {}", SpaceFuncDef);
   verifyFormat("A::A () : a(1) {}", SpaceFuncDef);
+  verifyFormat("template <> void A<C>(C x);", SpaceFuncDef);
+  verifyFormat("template <> void A<C> (C x) {}", SpaceFuncDef);
   verifyFormat("void f() __attribute__((asdf));", SpaceFuncDef);
   verifyFormat("void __attribute__((asdf)) f();", SpaceFuncDef);
   verifyFormat("#define A(x) x", SpaceFuncDef);

[HazardyKnusperkeks]

Would it suffice (and not break anything) to move line 4964 up here?

[bdunkin]

I just tried it, and it does pass all the formatting tests including the new ones. Would that be preferred to the current change?

[HazardyKnusperkeks]

I'd say move the if. That's less code, and basically what you did do with your first approach. And I think the intent is clearer.