Skip to content

[clang][Sema] Unify getPrototypeLoc helpers in SemaCodeComplete and clangd #143345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

HighCommander4
Copy link
Collaborator

HeuristicResolver houses the unified implementation.

Fixes #143240

…langd

HeuristicResolver houses the unified implementation.

Fixes llvm#143240
@HighCommander4 HighCommander4 requested review from hokein and zyn0217 June 9, 2025 05:02
@llvmbot llvmbot added clang Clang issues not falling into any other category clang-tools-extra clangd clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jun 9, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 9, 2025

@llvm/pr-subscribers-clangd
@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang

Author: Nathan Ridge (HighCommander4)

Changes

HeuristicResolver houses the unified implementation.

Fixes #143240


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

4 Files Affected:

  • (modified) clang-tools-extra/clangd/InlayHints.cpp (+2-49)
  • (modified) clang/include/clang/Sema/HeuristicResolver.h (+7)
  • (modified) clang/lib/Sema/HeuristicResolver.cpp (+55)
  • (modified) clang/lib/Sema/SemaCodeComplete.cpp (+1-49)
diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index 20a238612a7e4..5879c7745c300 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -33,7 +33,6 @@
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
-#include "llvm/ADT/identity.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FormatVariadic.h"
@@ -339,53 +338,6 @@ QualType maybeDesugar(ASTContext &AST, QualType QT) {
   return QT;
 }
 
-// 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.
-// FIXME: This function is mostly duplicated in SemaCodeComplete.cpp; unify.
-static FunctionProtoTypeLoc getPrototypeLoc(Expr *Fn) {
-  TypeLoc Target;
-  Expr *NakedFn = Fn->IgnoreParenCasts();
-  if (const auto *T = NakedFn->getType().getTypePtr()->getAs<TypedefType>()) {
-    Target = T->getDecl()->getTypeSourceInfo()->getTypeLoc();
-  } else if (const auto *DR = dyn_cast<DeclRefExpr>(NakedFn)) {
-    const auto *D = DR->getDecl();
-    if (const auto *const VD = dyn_cast<VarDecl>(D)) {
-      Target = VD->getTypeSourceInfo()->getTypeLoc();
-    }
-  }
-
-  if (!Target)
-    return {};
-
-  // Unwrap types that may be wrapping the function type
-  while (true) {
-    if (auto P = Target.getAs<PointerTypeLoc>()) {
-      Target = P.getPointeeLoc();
-      continue;
-    }
-    if (auto A = Target.getAs<AttributedTypeLoc>()) {
-      Target = A.getModifiedLoc();
-      continue;
-    }
-    if (auto P = Target.getAs<ParenTypeLoc>()) {
-      Target = P.getInnerLoc();
-      continue;
-    }
-    break;
-  }
-
-  if (auto F = Target.getAs<FunctionProtoTypeLoc>()) {
-    // In some edge cases the AST can contain a "trivial" FunctionProtoTypeLoc
-    // which has null parameters. Avoid these as they don't contain useful
-    // information.
-    if (llvm::all_of(F.getParams(), llvm::identity<ParmVarDecl *>()))
-      return F;
-  }
-
-  return {};
-}
-
 ArrayRef<const ParmVarDecl *>
 maybeDropCxxExplicitObjectParameters(ArrayRef<const ParmVarDecl *> Params) {
   if (!Params.empty() && Params.front()->isExplicitObjectParameter())
@@ -514,7 +466,8 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
       Callee.Decl = FD;
     else if (const auto *FTD = dyn_cast<FunctionTemplateDecl>(CalleeDecls[0]))
       Callee.Decl = FTD->getTemplatedDecl();
-    else if (FunctionProtoTypeLoc Loc = getPrototypeLoc(E->getCallee()))
+    else if (FunctionProtoTypeLoc Loc =
+                 Resolver->getProtoTypeLoc(E->getCallee()))
       Callee.Loc = Loc;
     else
       return true;
diff --git a/clang/include/clang/Sema/HeuristicResolver.h b/clang/include/clang/Sema/HeuristicResolver.h
index df60d3359c6a6..5e73aea79a609 100644
--- a/clang/include/clang/Sema/HeuristicResolver.h
+++ b/clang/include/clang/Sema/HeuristicResolver.h
@@ -20,6 +20,7 @@ class CXXBasePath;
 class CXXDependentScopeMemberExpr;
 class DeclarationName;
 class DependentScopeDeclRefExpr;
+class FunctionProtoTypeLoc;
 class NamedDecl;
 class Type;
 class UnresolvedUsingValueDecl;
@@ -93,6 +94,12 @@ class HeuristicResolver {
   // during simplification, and the operation fails if no pointer type is found.
   QualType simplifyType(QualType Type, const Expr *E, bool UnwrapPointer);
 
+  // Given an expression `Fn` representing the callee in a function call,
+  // 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.
+  FunctionProtoTypeLoc getProtoTypeLoc(Expr *Fn) const;
+
 private:
   ASTContext &Ctx;
 };
diff --git a/clang/lib/Sema/HeuristicResolver.cpp b/clang/lib/Sema/HeuristicResolver.cpp
index 0c67f1f2a3878..704a3d13cc2a8 100644
--- a/clang/lib/Sema/HeuristicResolver.cpp
+++ b/clang/lib/Sema/HeuristicResolver.cpp
@@ -13,6 +13,7 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/Type.h"
+#include "llvm/ADT/identity.h"
 
 namespace clang {
 
@@ -50,6 +51,7 @@ class HeuristicResolverImpl {
                       llvm::function_ref<bool(const NamedDecl *ND)> Filter);
   TagDecl *resolveTypeToTagDecl(QualType T);
   QualType simplifyType(QualType Type, const Expr *E, bool UnwrapPointer);
+  FunctionProtoTypeLoc getProtoTypeLoc(Expr *Fn);
 
 private:
   ASTContext &Ctx;
@@ -506,6 +508,55 @@ std::vector<const NamedDecl *> HeuristicResolverImpl::resolveDependentMember(
   }
   return {};
 }
+
+FunctionProtoTypeLoc HeuristicResolverImpl::getProtoTypeLoc(Expr *Fn) {
+  TypeLoc Target;
+  Expr *NakedFn = Fn->IgnoreParenCasts();
+  if (const auto *T = NakedFn->getType().getTypePtr()->getAs<TypedefType>()) {
+    Target = T->getDecl()->getTypeSourceInfo()->getTypeLoc();
+  } else if (const auto *DR = dyn_cast<DeclRefExpr>(NakedFn)) {
+    const auto *D = DR->getDecl();
+    if (const auto *const VD = dyn_cast<VarDecl>(D)) {
+      Target = VD->getTypeSourceInfo()->getTypeLoc();
+    }
+  } else if (const auto *ME = dyn_cast<MemberExpr>(Fn)) {
+    const auto *MD = ME->getMemberDecl();
+    if (const auto *FD = dyn_cast<FieldDecl>(MD)) {
+      Target = FD->getTypeSourceInfo()->getTypeLoc();
+    }
+  }
+
+  if (!Target)
+    return {};
+
+  // Unwrap types that may be wrapping the function type
+  while (true) {
+    if (auto P = Target.getAs<PointerTypeLoc>()) {
+      Target = P.getPointeeLoc();
+      continue;
+    }
+    if (auto A = Target.getAs<AttributedTypeLoc>()) {
+      Target = A.getModifiedLoc();
+      continue;
+    }
+    if (auto P = Target.getAs<ParenTypeLoc>()) {
+      Target = P.getInnerLoc();
+      continue;
+    }
+    break;
+  }
+
+  if (auto F = Target.getAs<FunctionProtoTypeLoc>()) {
+    // In some edge cases the AST can contain a "trivial" FunctionProtoTypeLoc
+    // which has null parameters. Avoid these as they don't contain useful
+    // information.
+    if (llvm::all_of(F.getParams(), llvm::identity<ParmVarDecl *>()))
+      return F;
+  }
+
+  return {};
+}
+
 } // namespace
 
 std::vector<const NamedDecl *> HeuristicResolver::resolveMemberExpr(
@@ -557,4 +608,8 @@ QualType HeuristicResolver::simplifyType(QualType Type, const Expr *E,
   return HeuristicResolverImpl(Ctx).simplifyType(Type, E, UnwrapPointer);
 }
 
+FunctionProtoTypeLoc HeuristicResolver::getProtoTypeLoc(Expr *Fn) const {
+  return HeuristicResolverImpl(Ctx).getProtoTypeLoc(Fn);
+}
+
 } // namespace clang
diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
index f9f7c192f19d2..ec0976d4e9fcf 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -6283,54 +6283,6 @@ 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();
-    }
-  } else if (const auto *ME = dyn_cast<MemberExpr>(Fn)) {
-    const auto *MD = ME->getMemberDecl();
-    if (const auto *FD = dyn_cast<FieldDecl>(MD)) {
-      Target = FD->getTypeSourceInfo()->getTypeLoc();
-    }
-  }
-
-  if (!Target)
-    return {};
-
-  // Unwrap types that may be wrapping the function type
-  while (true) {
-    if (auto P = Target.getAs<PointerTypeLoc>()) {
-      Target = P.getPointeeLoc();
-      continue;
-    }
-    if (auto A = Target.getAs<AttributedTypeLoc>()) {
-      Target = A.getModifiedLoc();
-      continue;
-    }
-    if (auto P = Target.getAs<ParenTypeLoc>()) {
-      Target = P.getInnerLoc();
-      continue;
-    }
-    break;
-  }
-
-  if (auto F = Target.getAs<FunctionProtoTypeLoc>()) {
-    return F;
-  }
-
-  return {};
-}
-
 QualType
 SemaCodeCompletion::ProduceCallSignatureHelp(Expr *Fn, ArrayRef<Expr *> Args,
                                              SourceLocation OpenParLoc) {
@@ -6419,7 +6371,7 @@ SemaCodeCompletion::ProduceCallSignatureHelp(Expr *Fn, ArrayRef<Expr *> Args,
       // Lastly we check whether expression's type is function pointer or
       // function.
 
-      FunctionProtoTypeLoc P = GetPrototypeLoc(NakedFn);
+      FunctionProtoTypeLoc P = Resolver.getProtoTypeLoc(NakedFn);
       QualType T = NakedFn->getType();
       if (!T->getPointeeType().isNull())
         T = T->getPointeeType();

Copy link
Contributor

@zyn0217 zyn0217 left a comment

Choose a reason for hiding this comment

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

thanks
Do we also want to add some tests?

@HighCommander4
Copy link
Collaborator Author

Do we also want to add some tests?

Yeah, good point. The two separate implementations did have test coverage in their respective components (CodeComplete and clangd), but HeuristicResolver has its own test suite so it would be better to have some tests for it there as well. Will revise the patch to add some.

@HighCommander4 HighCommander4 marked this pull request as draft June 9, 2025 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category clang-tools-extra clangd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unify getPrototypeLoc functions in InlayHints.cpp and SemaCodeComplete.cpp
3 participants