Argument name support for function pointer signature hints

Fixes clangd/clangd#1068

Reviewed By: nridge

Differential Revision: https://reviews.llvm.org/D125120

clang-tools-extra/clangd/CodeComplete.cpp

@@ -1019,10 +1019,12 @@ class SignatureHelpCollector final : public CodeCompleteConsumer {
         auto KindPriority = [&](OC::CandidateKind K) {
           switch (K) {
           case OC::CK_Aggregate:
-            return 1;
+            return 0;
           case OC::CK_Function:
-            return 2;
+            return 1;
           case OC::CK_FunctionType:
+            return 2;
+          case OC::CK_FunctionProtoTypeLoc:
             return 3;
           case OC::CK_FunctionTemplate:
             return 4;

clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

@@ -1283,6 +1283,23 @@ TEST(SignatureHelpTest, Overloads) {
   EXPECT_EQ(0, Results.activeParameter);
 }
 
+TEST(SignatureHelpTest, FunctionPointers) {
+  auto FunctionPointerResults = signatures(R"cpp(
+    void (*foo)(int x, int y);
+    int main() { foo(^); }
+  )cpp");
+  EXPECT_THAT(FunctionPointerResults.signatures,
+              UnorderedElementsAre(sig("([[int x]], [[int y]]) -> void")));
+
+  auto FunctionPointerTypedefResults = signatures(R"cpp(
+    typedef void (*fn)(int x, int y);
+    fn foo;
+    int main() { foo(^); }
+  )cpp");
+  EXPECT_THAT(FunctionPointerTypedefResults.signatures,
+              UnorderedElementsAre(sig("([[int x]], [[int y]]) -> void")));
+}
+
 TEST(SignatureHelpTest, Constructors) {
   std::string Top = R"cpp(
     struct S {

clang/include/clang/Sema/CodeCompleteConsumer.h

@@ -1019,6 +1019,10 @@ class CodeCompleteConsumer {
       /// for which we only have a function prototype.
       CK_FunctionType,
 
+      /// The candidate is a variable or expression of function type
+      /// for which we have the location of the prototype declaration.
+      CK_FunctionProtoTypeLoc,
+
       /// The candidate is a template, template arguments are being completed.
       CK_Template,
 
@@ -1043,6 +1047,10 @@ class CodeCompleteConsumer {
       /// when Kind == CK_FunctionType.
       const FunctionType *Type;
 
+      /// The location of the function prototype that describes the entity being
+      /// called, when Kind == CK_FunctionProtoTypeLoc.
+      FunctionProtoTypeLoc ProtoTypeLoc;
+
       /// The template overload candidate, available when
       /// Kind == CK_Template.
       const TemplateDecl *Template;
@@ -1068,6 +1076,11 @@ class CodeCompleteConsumer {
       assert(Type != nullptr);
     }
 
+    OverloadCandidate(FunctionProtoTypeLoc Prototype)
+        : Kind(CK_FunctionProtoTypeLoc), ProtoTypeLoc(Prototype) {
+      assert(!Prototype.isNull());
+    }
+
     OverloadCandidate(const RecordDecl *Aggregate)
         : Kind(CK_Aggregate), AggregateType(Aggregate) {
       assert(Aggregate != nullptr);
@@ -1093,6 +1106,11 @@ class CodeCompleteConsumer {
     /// function is stored.
     const FunctionType *getFunctionType() const;
 
+    /// Retrieve the function ProtoTypeLoc candidate.
+    /// This can be called for any Kind, but returns null for kinds
+    /// other than CK_FunctionProtoTypeLoc.
+    const FunctionProtoTypeLoc getFunctionProtoTypeLoc() const;
+
     const TemplateDecl *getTemplate() const {
       assert(getKind() == CK_Template && "Not a template");
       return Template;

clang/lib/Sema/CodeCompleteConsumer.cpp

@@ -515,7 +515,8 @@ CodeCompleteConsumer::OverloadCandidate::getFunctionType() const {
 
   case CK_FunctionType:
     return Type;
-
+  case CK_FunctionProtoTypeLoc:
+    return ProtoTypeLoc.getTypePtr();
   case CK_Template:
   case CK_Aggregate:
     return nullptr;
@@ -524,6 +525,13 @@ CodeCompleteConsumer::OverloadCandidate::getFunctionType() const {
   llvm_unreachable("Invalid CandidateKind!");
 }
 
+const FunctionProtoTypeLoc
+CodeCompleteConsumer::OverloadCandidate::getFunctionProtoTypeLoc() const {
+  if (Kind == CK_FunctionProtoTypeLoc)
+    return ProtoTypeLoc;
+  return FunctionProtoTypeLoc();
+}
+
 unsigned CodeCompleteConsumer::OverloadCandidate::getNumParams() const {
   if (Kind == CK_Template)
     return Template->getTemplateParameters()->size();
@@ -597,7 +605,12 @@ CodeCompleteConsumer::OverloadCandidate::getParamDecl(unsigned N) const {
   if (const auto *FD = getFunction()) {
     if (N < FD->param_size())
       return FD->getParamDecl(N);
+  } else if (Kind == CK_FunctionProtoTypeLoc) {
+    if (N < ProtoTypeLoc.getNumParams()) {
+      return ProtoTypeLoc.getParam(N);
+    }
   }
+
   return nullptr;
 }
 

clang/lib/Sema/SemaCodeComplete.cpp

@@ -53,6 +53,7 @@
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
+
 #include <list>
 #include <map>
 #include <string>
@@ -3722,13 +3723,11 @@ static void AddOverloadAggregateChunks(const RecordDecl *RD,
 
 /// Add function overload parameter chunks to the given code completion
 /// string.
-static void AddOverloadParameterChunks(ASTContext &Context,
-                                       const PrintingPolicy &Policy,
-                                       const FunctionDecl *Function,
-                                       const FunctionProtoType *Prototype,
-                                       CodeCompletionBuilder &Result,
-                                       unsigned CurrentArg, unsigned Start = 0,
-                                       bool InOptional = false) {
+static void AddOverloadParameterChunks(
+    ASTContext &Context, const PrintingPolicy &Policy,
+    const FunctionDecl *Function, const FunctionProtoType *Prototype,
+    FunctionProtoTypeLoc PrototypeLoc, CodeCompletionBuilder &Result,
+    unsigned CurrentArg, unsigned Start = 0, bool InOptional = false) {
   if (!Function && !Prototype) {
     Result.AddChunk(CodeCompletionString::CK_CurrentParameter, "...");
     return;
@@ -3747,8 +3746,9 @@ static void AddOverloadParameterChunks(ASTContext &Context,
       if (!FirstParameter)
         Opt.AddChunk(CodeCompletionString::CK_Comma);
       // Optional sections are nested.
-      AddOverloadParameterChunks(Context, Policy, Function, Prototype, Opt,
-                                 CurrentArg, P, /*InOptional=*/true);
+      AddOverloadParameterChunks(Context, Policy, Function, Prototype,
+                                 PrototypeLoc, Opt, CurrentArg, P,
+                                 /*InOptional=*/true);
       Result.AddOptionalChunk(Opt.TakeString());
       return;
     }
@@ -3762,8 +3762,10 @@ static void AddOverloadParameterChunks(ASTContext &Context,
 
     // Format the placeholder string.
     std::string Placeholder;
-    if (Function) {
-      const ParmVarDecl *Param = Function->getParamDecl(P);
+    assert(P < Prototype->getNumParams());
+    if (Function || PrototypeLoc) {
+      const ParmVarDecl *Param =
+          Function ? Function->getParamDecl(P) : PrototypeLoc.getParam(P);
       Placeholder = FormatFunctionParameter(Policy, Param);
       if (Param->hasDefaultArg())
         Placeholder += GetDefaultValueString(Param, Context.getSourceManager(),
@@ -3916,8 +3918,8 @@ CodeCompleteConsumer::OverloadCandidate::CreateSignatureString(
   if (getKind() == CK_Aggregate)
     AddOverloadAggregateChunks(getAggregate(), Policy, Result, CurrentArg);
   else
-    AddOverloadParameterChunks(S.getASTContext(), Policy, FDecl, Proto, Result,
-                               CurrentArg);
+    AddOverloadParameterChunks(S.getASTContext(), Policy, FDecl, Proto,
+                               getFunctionProtoTypeLoc(), Result, CurrentArg);
   Result.AddChunk(Braced ? CodeCompletionString::CK_RightBrace
                          : CodeCompletionString::CK_RightParen);
 
@@ -5998,6 +6000,39 @@ ProduceSignatureHelp(Sema &SemaRef, MutableArrayRef<ResultCandidate> Candidates,
   return getParamType(SemaRef, Candidates, CurrentArg);
 }
 
+// Given a callee expression `Fn`, if the call is through a function pointer,
+// try to find the declaration of the corresponding function pointer type,
+// so that we can recover argument names from it.
+static FunctionProtoTypeLoc GetPrototypeLoc(Expr *Fn) {
+  TypeLoc Target;
+  if (const auto *T = Fn->getType().getTypePtr()->getAs<TypedefType>()) {
+    Target = T->getDecl()->getTypeSourceInfo()->getTypeLoc();
+
+  } else if (const auto *DR = dyn_cast<DeclRefExpr>(Fn)) {
+    const auto *D = DR->getDecl();
+    if (const auto *const VD = dyn_cast<VarDecl>(D)) {
+      Target = VD->getTypeSourceInfo()->getTypeLoc();
+    }
+  }
+
+  if (!Target)
+    return {};
+
+  if (auto P = Target.getAs<PointerTypeLoc>()) {
+    Target = P.getPointeeLoc();
+  }
+
+  if (auto P = Target.getAs<ParenTypeLoc>()) {
+    Target = P.getInnerLoc();
+  }
+
+  if (auto F = Target.getAs<FunctionProtoTypeLoc>()) {
+    return F;
+  }
+
+  return {};
+}
+
 QualType Sema::ProduceCallSignatureHelp(Expr *Fn, ArrayRef<Expr *> Args,
                                         SourceLocation OpenParLoc) {
   Fn = unwrapParenList(Fn);
@@ -6079,6 +6114,8 @@ QualType Sema::ProduceCallSignatureHelp(Expr *Fn, ArrayRef<Expr *> Args,
     } else {
       // Lastly we check whether expression's type is function pointer or
       // function.
+
+      FunctionProtoTypeLoc P = GetPrototypeLoc(NakedFn);
       QualType T = NakedFn->getType();
       if (!T->getPointeeType().isNull())
         T = T->getPointeeType();
@@ -6087,8 +6124,13 @@ QualType Sema::ProduceCallSignatureHelp(Expr *Fn, ArrayRef<Expr *> Args,
         if (!TooManyArguments(FP->getNumParams(),
                               ArgsWithoutDependentTypes.size(),
                               /*PartialOverloading=*/true) ||
-            FP->isVariadic())
-          Results.push_back(ResultCandidate(FP));
+            FP->isVariadic()) {
+          if (P) {
+            Results.push_back(ResultCandidate(P));
+          } else {
+            Results.push_back(ResultCandidate(FP));
+          }
+        }
       } else if (auto FT = T->getAs<FunctionType>())
         // No prototype and declaration, it may be a K & R style function.
         Results.push_back(ResultCandidate(FT));