[clang-format] Correctly annotate star/amp in function pointer params

[rymiel]

Summary

Inside the arguments part of a function pointer declaration,
determineStarAmpUsage results in a binary operator rather than
pointers, because said parens are assumed to be an expression.

This patch correctly marks the argument parens of a function
pointer type as not an expression. Note that this fix already
existed for Objective-C blocks as part of rGf1f267b447f60528440d2c066b29ab014ae7f90f.
As Objective-C blocks and C/C++ function pointers share a lot
of the same logic, that fix also makes sense here.

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

[MyDeveloperDay]

LGTM

[MyDeveloperDay]

Thanks @rymiel its great to have another active contributor, especially one who seems so focused on github issues. Really appreciate your recent contributions. Can we start including you as one of the default reviewers that we use to review our stuff?

[rymiel]

Thanks for the kind words ❤️ (I've noticed Björn already includes me in his reviewers list); and while I am humbled, and I do look at every patch, I don't *yet* trust myself to actually perform good code review, as it's not something I've really ever done before.

[HazardyKnusperkeks]

In D135707#3859357, @rymiel wrote:

Thanks for the kind words ❤️ (I've noticed Björn already includes me in his reviewers list); and while I am humbled, and I do look at every patch, I don't *yet* trust myself to actually perform good code review, as it's not something I've really ever done before.

It's actually easy. Just give it a thumbs up if everything is good, as far as you can see.

[owenpan]

In D135707#3858097, @MyDeveloperDay wrote:

Thanks @rymiel its great to have another active contributor, especially one who seems so focused on github issues. Really appreciate your recent contributions. Can we start including you as one of the default reviewers that we use to review our stuff?

+1.

In D135707#3859357, @rymiel wrote:

I don't *yet* trust myself to actually perform good code review, as it's not something I've really ever done before.

IMO none of us can claim to be an expert in every area of clang-format. :) Even if you don't feel that you are an "expert" yet, you are still encouraged to review code. See here for the rationale.

[MyDeveloperDay]

I learn a lot from watching the others code review. Definitely not an expert here either, just try to help where I can.

clang/lib/Format/TokenAnnotator.cpp

if (OpeningParen.is(TT_OverloadedOperatorLParen)) {
       // This is the parameter list of an ObjC block.
       Contexts.back().IsExpression = false;
     } else if (OpeningParen.Previous &&
                OpeningParen.Previous->is(TT_ForEachMacro)) {
       // The first argument to a foreach macro is a declaration.
       Contexts.back().ContextType = Context::ForEachMacro;
       Contexts.back().IsExpression = false;
     } else if (OpeningParen.Previous && OpeningParen.Previous->MatchingParen &&
-               OpeningParen.Previous->MatchingParen->is(TT_ObjCBlockLParen)) {
+               OpeningParen.Previous->MatchingParen->isOneOf(
+                   TT_ObjCBlockLParen, TT_FunctionTypeLParen)) {
       Contexts.back().IsExpression = false;
     } else if (!Line.MustBeDeclaration && !Line.InPPDirective) {
       bool IsForOrCatch =
           OpeningParen.Previous &&
           OpeningParen.Previous->isOneOf(tok::kw_for, tok::kw_catch);
       Contexts.back().IsExpression = !IsForOrCatch;
     }
 

clang/unittests/Format/FormatTest.cpp

TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) {
   verifyFormat("[](const typeof(*a) &value) {}");
   verifyFormat("[](const _Atomic(a *) &value) {}");
   verifyFormat("[](const __underlying_type(a) &value) {}");
   verifyFormat("decltype(a * b) F();");
   verifyFormat("typeof(a * b) F();");
   verifyFormat("#define MACRO() [](A *a) { return 1; }");
   verifyFormat("Constructor() : member([](A *a, B *b) {}) {}");
   verifyIndependentOfContext("typedef void (*f)(int *a);");
+  verifyIndependentOfContext("typedef void (*f)(Type *a);");
   verifyIndependentOfContext("int i{a * b};");
   verifyIndependentOfContext("aaa && aaa->f();");
   verifyIndependentOfContext("int x = ~*p;");
   verifyFormat("Constructor() : a(a), area(width * height) {}");
   verifyFormat("Constructor() : a(a), area(a, width * height) {}");
   verifyGoogleFormat("MACRO Constructor(const int& i) : a(a), b(b) {}");
   verifyFormat("void f() { f(a, c * d); }");
   verifyFormat("void f() { f(new a(), c * d); }");

clang/unittests/Format/TokenAnnotatorTest.cpp

TEST_F(TokenAnnotatorTest, UnderstandsUsesOfStarAndAmp) {
   EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
   Tokens = annotate("delete[] **ptr;");
   EXPECT_EQ(Tokens.size(), 8u) << Tokens;
   EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
   EXPECT_TOKEN(Tokens[4], tok::star, TT_UnaryOperator);
   Tokens = annotate("delete[] *(ptr);");
   EXPECT_EQ(Tokens.size(), 9u) << Tokens;
   EXPECT_TOKEN(Tokens[3], tok::star, TT_UnaryOperator);
+
+  Tokens = annotate("void f() { void (*fnptr)(char* foo); }");
+  EXPECT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_FunctionTypeLParen);
+  // FIXME: The star of a function pointer probably makes more sense as
+  // TT_PointerOrReference.
+  EXPECT_TOKEN(Tokens[7], tok::star, TT_UnaryOperator);
+  EXPECT_TOKEN(Tokens[12], tok::star, TT_PointerOrReference);
+
+  Tokens = annotate("void f() { void (*fnptr)(t* foo); }");
+  EXPECT_EQ(Tokens.size(), 18u) << Tokens;
+  EXPECT_TOKEN(Tokens[6], tok::l_paren, TT_FunctionTypeLParen);
+  EXPECT_TOKEN(Tokens[7], tok::star, TT_UnaryOperator);
+  EXPECT_TOKEN(Tokens[12], tok::star, TT_PointerOrReference);
 }
 
 TEST_F(TokenAnnotatorTest, UnderstandsUsesOfPlusAndMinus) {
   auto Tokens = annotate("x - 0");
   ASSERT_EQ(Tokens.size(), 4u) << Tokens;
   EXPECT_TOKEN(Tokens[1], tok::minus, TT_BinaryOperator);
   Tokens = annotate("0 + 0");
   ASSERT_EQ(Tokens.size(), 4u) << Tokens;