Skip to content

Conversation

@rmarker
Copy link
Contributor

@rmarker rmarker commented Feb 6, 2025

Fixes #125012

@llvmbot
Copy link
Member

llvmbot commented Feb 6, 2025

@llvm/pr-subscribers-clang-format

Author: None (rmarker)

Changes

Fixes #125012


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

2 Files Affected:

  • (modified) clang/lib/Format/TokenAnnotator.cpp (+15-2)
  • (modified) clang/unittests/Format/TokenAnnotatorTest.cpp (+5)
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index f25332e3a5f4e1a..fc84f29dd04389b 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2915,14 +2915,27 @@ class AnnotatingParser {
     if (Prev->is(tok::r_paren)) {
       if (Prev->is(TT_CastRParen))
         return false;
+
+      // Check for the second pair of parentheses.
       Prev = Prev->MatchingParen;
       if (!Prev)
         return false;
+
+      // Now check for the first pair of parentheses.
       Prev = Prev->Previous;
       if (!Prev || Prev->isNot(tok::r_paren))
         return false;
-      Prev = Prev->MatchingParen;
-      return Prev && Prev->is(TT_FunctionTypeLParen);
+      const auto* PrevMatchingParen = Prev->MatchingParen;
+      if (!PrevMatchingParen)
+        return false;
+
+      // We can quickly tell it is a function pointer type if the paren is the
+      // right type.
+      if (PrevMatchingParen->isNot(TT_Unknown))
+        return PrevMatchingParen->is(TT_FunctionTypeLParen);
+
+      // Otherwise check for the trailing * in the parentheses.
+      return Prev->Previous && Prev->Previous->is(tok::star);
     }
 
     // Search for unexpected tokens.
diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp
index 1b09c4570345622..5dcb9b94a5bb2f5 100644
--- a/clang/unittests/Format/TokenAnnotatorTest.cpp
+++ b/clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -874,6 +874,11 @@ TEST_F(TokenAnnotatorTest, UnderstandsCasts) {
   EXPECT_TOKEN(Tokens[14], tok::r_paren, TT_CastRParen);
   EXPECT_TOKEN(Tokens[15], tok::amp, TT_UnaryOperator);
 
+  Tokens = annotate("func((foo(bar::*)(void))&a);");
+  ASSERT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[12], tok::r_paren, TT_CastRParen);
+  EXPECT_TOKEN(Tokens[13], tok::amp, TT_UnaryOperator);
+
   auto Style = getLLVMStyle();
   Style.TypeNames.push_back("Foo");
   Tokens = annotate("#define FOO(bar) foo((Foo)&bar)", Style);

@github-actions
Copy link

github-actions bot commented Feb 6, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@HazardyKnusperkeks HazardyKnusperkeks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please wait for @owenca or @mydeveloperday.

EXPECT_TOKEN(Tokens[14], tok::r_paren, TT_CastRParen);
EXPECT_TOKEN(Tokens[15], tok::amp, TT_UnaryOperator);

Tokens = annotate("func((foo(bar::*)(void))&a);");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's foo here? Can you show an example in Compiler Explorer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here foo is a type.

It looks like I accidentally trimmed too much when making the test case. Whoops.
Good catch.
It should be func((foo(bar::*)(void))&bar::a);
I've pushed a commit to adjust that.

Here is a Compiler Explorer link with what I was going for.
https://godbolt.org/z/Yhn65zx8P

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So foo is a return type. Then the ( that follows should be annotated as a FunctionLParen, and there should be a space between them like in int (*)().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I was so focused on the behaviour from 19, that I didn't realise about the space after the return type.
Yeah, it seems like even with missing the annotation of the left paren clang-format 19, managed to get lucky with the right cast paren. With the extra fix in 20 revealing the underlying issue.
Nice work.

@owenca
Copy link
Contributor

owenca commented Feb 8, 2025

The root cause is that the l_paren after the return type is not annotated. That's why the space between them is missing.

@owenca
Copy link
Contributor

owenca commented Feb 8, 2025

See #126340.

@rmarker rmarker closed this Feb 9, 2025
@rmarker rmarker deleted the 125012 branch February 9, 2025 05:28
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.

[clang-format] Misformatted address-of operator after cast on 20 versus 19

4 participants