Skip to content

Conversation

@bdunkin
Copy link

@bdunkin bdunkin commented Jun 5, 2025

This fixes the SpaceBeforeParensOptions.AfterFunctionDeclarationName and SpaceBeforeParensOptions.AfterFunctionDefinitionName options not adding spaces when the function has an explicit Microsoft calling convention.

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

@github-actions
Copy link

github-actions bot commented Jun 5, 2025

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot
Copy link
Member

llvmbot commented Jun 5, 2025

@llvm/pr-subscribers-clang-format

Author: Ben Dunkin (bdunkin)

Changes

This fixes the SpaceBeforeParensOptions.AfterFunctionDeclarationName and SpaceBeforeParensOptions.AfterFunctionDefinitionName options not adding spaces when the function has an explicit Microsoft calling convention.

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/143047.diff

3 Files Affected:

  • (modified) clang/lib/Format/FormatToken.h (+1)
  • (modified) clang/lib/Format/TokenAnnotator.cpp (+15)
  • (modified) clang/unittests/Format/FormatTest.cpp (+12)
diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index 94014aee3221f..ef1c129b29b0c 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -114,6 +114,7 @@ namespace format {
   TYPE(LineComment)                                                            \
   TYPE(MacroBlockBegin)                                                        \
   TYPE(MacroBlockEnd)                                                          \
+  TYPE(MicrosoftCallingConvention)                                             \
   TYPE(ModulePartitionColon)                                                   \
   TYPE(NamespaceLBrace)                                                        \
   TYPE(NamespaceMacro)                                                         \
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index da279d07b5918..4e23478058499 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -1802,6 +1802,14 @@ class AnnotatingParser {
       if (Style.isTableGen() && !parseTableGenValue())
         return false;
       break;
+    case tok::kw___cdecl:
+    case tok::kw___stdcall:
+    case tok::kw___fastcall:
+    case tok::kw___thiscall:
+    case tok::kw___regcall:
+    case tok::kw___vectorcall:
+      Tok->setType(TT_MicrosoftCallingConvention);
+      break;
     default:
       break;
     }
@@ -2611,6 +2619,13 @@ class AnnotatingParser {
     // Skip "const" as it does not have an influence on whether this is a name.
     FormatToken *PreviousNotConst = Tok.getPreviousNonComment();
 
+    // Skip Microsoft calling conventions, as they come before the function
+    // name, but after the return type
+    while (PreviousNotConst &&
+           PreviousNotConst->is(TT_MicrosoftCallingConvention)) {
+      PreviousNotConst = PreviousNotConst->getPreviousNonComment();
+    }
+
     // For javascript const can be like "let" or "var"
     if (!Style.isJavaScript())
       while (PreviousNotConst && PreviousNotConst->is(tok::kw_const))
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index c0633ba3c29b3..0f96b0f92cdb2 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -17504,6 +17504,12 @@ TEST_F(FormatTest, ConfigurableSpaceBeforeParens) {
   verifyFormat("A::A() : a(1) {}", SpaceFuncDecl);
   verifyFormat("void f () __attribute__((asdf));", SpaceFuncDecl);
   verifyFormat("void __attribute__((asdf)) f ();", SpaceFuncDecl);
+  verifyFormat("void __stdcall f ();", SpaceFuncDecl);
+  verifyFormat("void __cdecl f ();", SpaceFuncDecl);
+  verifyFormat("void __fastcall f ();", SpaceFuncDecl);
+  verifyFormat("void __stdcall f() {}", SpaceFuncDecl);
+  verifyFormat("void __cdecl f() {}", SpaceFuncDecl);
+  verifyFormat("void __fastcall f() {}", SpaceFuncDecl);
   verifyFormat("#define A(x) x", SpaceFuncDecl);
   verifyFormat("#define A (x) x", SpaceFuncDecl);
   verifyFormat("#if defined(x)\n"
@@ -17540,6 +17546,12 @@ TEST_F(FormatTest, ConfigurableSpaceBeforeParens) {
   verifyFormat("A::A () : a(1) {}", SpaceFuncDef);
   verifyFormat("void f() __attribute__((asdf));", SpaceFuncDef);
   verifyFormat("void __attribute__((asdf)) f();", SpaceFuncDef);
+  verifyFormat("void __stdcall f();", SpaceFuncDef);
+  verifyFormat("void __cdecl f();", SpaceFuncDef);
+  verifyFormat("void __fastcall f();", SpaceFuncDef);
+  verifyFormat("void __stdcall f () {}", SpaceFuncDef);
+  verifyFormat("void __cdecl f () {}", SpaceFuncDef);
+  verifyFormat("void __fastcall f () {}", SpaceFuncDef);
   verifyFormat("#define A(x) x", SpaceFuncDef);
   verifyFormat("#define A (x) x", SpaceFuncDef);
   verifyFormat("#if defined(x)\n"

@owenca
Copy link
Contributor

owenca commented Jun 6, 2025

Can you open a GitHub issue for this? I think it's a bug that the function name is not annotated as TT_FunctionDeclarationname:

AnnotatedTokens(L=0, P=0, T=6, C=0):
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=0 Name=void L=4 PPK=2 FakeLParens= FakeRParens=0 II=0x136014038 Text='void'
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=23 Name=__cdecl L=12 PPK=2 FakeLParens= FakeRParens=0 II=0x1360171b0 Text='__cdecl'
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=23 Name=identifier L=14 PPK=2 FakeLParens= FakeRParens=0 II=0x13601d450 Text='f'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=l_paren L=15 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='('
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=140 Name=r_paren L=16 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=')'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=semi L=17 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=';'
----
AnnotatedTokens(L=0, P=0, T=6, C=0):
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=0 Name=eof L=0 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=''
----
Replacements for run 0:
void __cdecl f();

@owenca
Copy link
Contributor

owenca commented Jun 6, 2025

See #143083, which annotates the above as:

AnnotatedTokens(L=0, P=0, T=6, C=0):
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=0 Name=void L=4 PPK=2 FakeLParens= FakeRParens=0 II=0x109813e38 Text='void'
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=23 Name=__cdecl L=12 PPK=2 FakeLParens= FakeRParens=0 II=0x109816fb0 Text='__cdecl'
 M=0 C=1 T=FunctionDeclarationName S=1 F=0 B=0 BK=0 P=80 Name=identifier L=14 PPK=2 FakeLParens= FakeRParens=0 II=0x10981d250 Text='f'
 M=0 C=0 T=FunctionDeclarationLParen S=0 F=0 B=0 BK=0 P=23 Name=l_paren L=15 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='('
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=140 Name=r_paren L=16 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=')'
 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=semi L=17 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=';'
----
AnnotatedTokens(L=0, P=0, T=6, C=0):
 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=0 Name=eof L=0 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=''
----
Replacements for run 0:
void __cdecl f();

@bdunkin
Copy link
Author

bdunkin commented Jun 6, 2025

Ah ok, I understand how your change is a better fix. I will close this PR as yours also fixes things. I have some more fixes coming for this same option, so I will follow your lead on getting the token type of the opening parenthesis correct.

@bdunkin bdunkin closed this Jun 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants