[clang] 53c593c - [clang] Make signature help work with dependent args

Kadir Cetinkaya via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 17 01:15:23 PDT 2020


Author: Kadir Cetinkaya
Date: 2020-08-17T10:06:36+02:00
New Revision: 53c593c2c893a40083771789e3d3e164eea1892d

URL: https://github.com/llvm/llvm-project/commit/53c593c2c893a40083771789e3d3e164eea1892d
DIFF: https://github.com/llvm/llvm-project/commit/53c593c2c893a40083771789e3d3e164eea1892d.diff

LOG: [clang] Make signature help work with dependent args

Fixes https://github.com/clangd/clangd/issues/490

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
    clang/lib/Sema/SemaCodeComplete.cpp
    clang/test/CodeCompletion/call.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index fd144d9391d3..635e036039a0 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -1166,46 +1166,75 @@ TEST(SignatureHelpTest, ActiveArg) {
 }
 
 TEST(SignatureHelpTest, OpeningParen) {
-  llvm::StringLiteral Tests[] = {// Recursive function call.
-                                 R"cpp(
-    int foo(int a, int b, int c);
-    int main() {
-      foo(foo $p^( foo(10, 10, 10), ^ )));
-    })cpp",
-                                 // Functional type cast.
-                                 R"cpp(
-    struct Foo {
-      Foo(int a, int b, int c);
-    };
-    int main() {
-      Foo $p^( 10, ^ );
-    })cpp",
-                                 // New expression.
-                                 R"cpp(
-    struct Foo {
-      Foo(int a, int b, int c);
-    };
-    int main() {
-      new Foo $p^( 10, ^ );
-    })cpp",
-                                 // Macro expansion.
-                                 R"cpp(
-    int foo(int a, int b, int c);
-    #define FOO foo(
-
-    int main() {
-      // Macro expansions.
-      $p^FOO 10, ^ );
-    })cpp",
-                                 // Macro arguments.
-                                 R"cpp(
-    int foo(int a, int b, int c);
-    int main() {
-    #define ID(X) X
-      // FIXME: figure out why ID(foo (foo(10), )) doesn't work when preserving
-      // the recovery expression.
-      ID(foo $p^( 10, ^ ))
-    })cpp"};
+  llvm::StringLiteral Tests[] = {
+      // Recursive function call.
+      R"cpp(
+        int foo(int a, int b, int c);
+        int main() {
+          foo(foo $p^( foo(10, 10, 10), ^ )));
+        })cpp",
+      // Functional type cast.
+      R"cpp(
+        struct Foo {
+          Foo(int a, int b, int c);
+        };
+        int main() {
+          Foo $p^( 10, ^ );
+        })cpp",
+      // New expression.
+      R"cpp(
+        struct Foo {
+          Foo(int a, int b, int c);
+        };
+        int main() {
+          new Foo $p^( 10, ^ );
+        })cpp",
+      // Macro expansion.
+      R"cpp(
+        int foo(int a, int b, int c);
+        #define FOO foo(
+
+        int main() {
+          // Macro expansions.
+          $p^FOO 10, ^ );
+        })cpp",
+      // Macro arguments.
+      R"cpp(
+        int foo(int a, int b, int c);
+        int main() {
+        #define ID(X) X
+          // FIXME: figure out why ID(foo (foo(10), )) doesn't work when preserving
+          // the recovery expression.
+          ID(foo $p^( 10, ^ ))
+        })cpp",
+      // Dependent args.
+      R"cpp(
+        int foo(int a, int b);
+        template <typename T> void bar(T t) {
+          foo$p^(t, ^t);
+        })cpp",
+      // Dependent args on templated func.
+      R"cpp(
+        template <typename T>
+        int foo(T, T);
+        template <typename T> void bar(T t) {
+          foo$p^(t, ^t);
+        })cpp",
+      // Dependent args on member.
+      R"cpp(
+        struct Foo { int foo(int, int); };
+        template <typename T> void bar(T t) {
+          Foo f;
+          f.foo$p^(t, ^t);
+        })cpp",
+      // Dependent args on templated member.
+      R"cpp(
+        struct Foo { template <typename T> int foo(T, T); };
+        template <typename T> void bar(T t) {
+          Foo f;
+          f.foo$p^(t, ^t);
+        })cpp",
+  };
 
   for (auto Test : Tests) {
     Annotations Code(Test);

diff  --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
index 0a8a27068ebf..cf38b1012e7d 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -5500,7 +5500,7 @@ typedef CodeCompleteConsumer::OverloadCandidate ResultCandidate;
 
 static void mergeCandidatesWithResults(
     Sema &SemaRef, SmallVectorImpl<ResultCandidate> &Results,
-    OverloadCandidateSet &CandidateSet, SourceLocation Loc) {
+    OverloadCandidateSet &CandidateSet, SourceLocation Loc, size_t ArgSize) {
   // Sort the overload candidate set by placing the best overloads first.
   llvm::stable_sort(CandidateSet, [&](const OverloadCandidate &X,
                                       const OverloadCandidate &Y) {
@@ -5510,8 +5510,19 @@ static void mergeCandidatesWithResults(
 
   // Add the remaining viable overload candidates as code-completion results.
   for (OverloadCandidate &Candidate : CandidateSet) {
-    if (Candidate.Function && Candidate.Function->isDeleted())
-      continue;
+    if (Candidate.Function) {
+      if (Candidate.Function->isDeleted())
+        continue;
+      if (!Candidate.Function->isVariadic() &&
+          Candidate.Function->getNumParams() <= ArgSize &&
+          // Having zero args is annoying, normally we don't surface a function
+          // with 2 params, if you already have 2 params, because you are
+          // inserting the 3rd now. But with zero, it helps the user to figure
+          // out there are no overloads that take any arguments. Hence we are
+          // keeping the overload.
+          ArgSize > 0)
+        continue;
+    }
     if (Candidate.Viable)
       Results.push_back(ResultCandidate(Candidate.Function));
   }
@@ -5562,22 +5573,25 @@ QualType Sema::ProduceCallSignatureHelp(Scope *S, Expr *Fn,
 
   // FIXME: Provide support for variadic template functions.
   // Ignore type-dependent call expressions entirely.
-  if (!Fn || Fn->isTypeDependent() || anyNullArguments(Args) ||
-      Expr::hasAnyTypeDependentArguments(Args)) {
+  if (!Fn || Fn->isTypeDependent() || anyNullArguments(Args))
     return QualType();
-  }
+  // In presence of dependent args we surface all possible signatures using the
+  // non-dependent args in the prefix. Afterwards we do a post filtering to make
+  // sure provided candidates satisfy parameter count restrictions.
+  auto ArgsWithoutDependentTypes =
+      Args.take_while([](Expr *Arg) { return !Arg->isTypeDependent(); });
 
+  SmallVector<ResultCandidate, 8> Results;
+
+  Expr *NakedFn = Fn->IgnoreParenCasts();
   // Build an overload candidate set based on the functions we find.
   SourceLocation Loc = Fn->getExprLoc();
   OverloadCandidateSet CandidateSet(Loc, OverloadCandidateSet::CSK_Normal);
 
-  SmallVector<ResultCandidate, 8> Results;
-
-  Expr *NakedFn = Fn->IgnoreParenCasts();
-  if (auto ULE = dyn_cast<UnresolvedLookupExpr>(NakedFn))
-    AddOverloadedCallCandidates(ULE, Args, CandidateSet,
+  if (auto ULE = dyn_cast<UnresolvedLookupExpr>(NakedFn)) {
+    AddOverloadedCallCandidates(ULE, ArgsWithoutDependentTypes, CandidateSet,
                                 /*PartialOverloading=*/true);
-  else if (auto UME = dyn_cast<UnresolvedMemberExpr>(NakedFn)) {
+  } else if (auto UME = dyn_cast<UnresolvedMemberExpr>(NakedFn)) {
     TemplateArgumentListInfo TemplateArgsBuffer, *TemplateArgs = nullptr;
     if (UME->hasExplicitTemplateArgs()) {
       UME->copyTemplateArgumentsInto(TemplateArgsBuffer);
@@ -5587,7 +5601,8 @@ QualType Sema::ProduceCallSignatureHelp(Scope *S, Expr *Fn,
     // Add the base as first argument (use a nullptr if the base is implicit).
     SmallVector<Expr *, 12> ArgExprs(
         1, UME->isImplicitAccess() ? nullptr : UME->getBase());
-    ArgExprs.append(Args.begin(), Args.end());
+    ArgExprs.append(ArgsWithoutDependentTypes.begin(),
+                    ArgsWithoutDependentTypes.end());
     UnresolvedSet<8> Decls;
     Decls.append(UME->decls_begin(), UME->decls_end());
     const bool FirstArgumentIsBase = !UME->isImplicitAccess() && UME->getBase();
@@ -5606,7 +5621,7 @@ QualType Sema::ProduceCallSignatureHelp(Scope *S, Expr *Fn,
         Results.push_back(ResultCandidate(FD));
       else
         AddOverloadCandidate(FD, DeclAccessPair::make(FD, FD->getAccess()),
-                             Args, CandidateSet,
+                             ArgsWithoutDependentTypes, CandidateSet,
                              /*SuppressUserConversions=*/false,
                              /*PartialOverloading=*/true);
 
@@ -5621,7 +5636,8 @@ QualType Sema::ProduceCallSignatureHelp(Scope *S, Expr *Fn,
         LookupQualifiedName(R, DC);
         R.suppressDiagnostics();
         SmallVector<Expr *, 12> ArgExprs(1, NakedFn);
-        ArgExprs.append(Args.begin(), Args.end());
+        ArgExprs.append(ArgsWithoutDependentTypes.begin(),
+                        ArgsWithoutDependentTypes.end());
         AddFunctionCandidates(R.asUnresolvedSet(), ArgExprs, CandidateSet,
                               /*ExplicitArgs=*/nullptr,
                               /*SuppressUserConversions=*/false,
@@ -5635,7 +5651,8 @@ QualType Sema::ProduceCallSignatureHelp(Scope *S, Expr *Fn,
         T = T->getPointeeType();
 
       if (auto FP = T->getAs<FunctionProtoType>()) {
-        if (!TooManyArguments(FP->getNumParams(), Args.size(),
+        if (!TooManyArguments(FP->getNumParams(),
+                              ArgsWithoutDependentTypes.size(),
                               /*PartialOverloading=*/true) ||
             FP->isVariadic())
           Results.push_back(ResultCandidate(FP));
@@ -5644,7 +5661,7 @@ QualType Sema::ProduceCallSignatureHelp(Scope *S, Expr *Fn,
         Results.push_back(ResultCandidate(FT));
     }
   }
-  mergeCandidatesWithResults(*this, Results, CandidateSet, Loc);
+  mergeCandidatesWithResults(*this, Results, CandidateSet, Loc, Args.size());
   QualType ParamType =
       ProduceSignatureHelp(*this, S, Results, Args.size(), OpenParLoc);
   return !CandidateSet.empty() ? ParamType : QualType();
@@ -5685,7 +5702,7 @@ QualType Sema::ProduceConstructorSignatureHelp(Scope *S, QualType Type,
   }
 
   SmallVector<ResultCandidate, 8> Results;
-  mergeCandidatesWithResults(*this, Results, CandidateSet, Loc);
+  mergeCandidatesWithResults(*this, Results, CandidateSet, Loc, Args.size());
   return ProduceSignatureHelp(*this, S, Results, Args.size(), OpenParLoc);
 }
 

diff  --git a/clang/test/CodeCompletion/call.cpp b/clang/test/CodeCompletion/call.cpp
index 9cba6ae100b3..95f66e6e0fb2 100644
--- a/clang/test/CodeCompletion/call.cpp
+++ b/clang/test/CodeCompletion/call.cpp
@@ -32,3 +32,23 @@ void test() {
   // CHECK-CC3-NEXT: OVERLOAD: [#void#]f(<#int i#>, int j, int k)
   // CHECK-CC3-NEXT: OVERLOAD: [#void#]f(<#float x#>, float y)
 }
+
+void f(int, int, int, int);
+template <typename T>
+void foo(T t) {
+  f(t, t, t);
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:39:5 %s -o - | FileCheck -check-prefix=CHECK-CC4 %s
+  // CHECK-CC4: f()
+  // CHECK-CC4-NEXT: f(<#X#>)
+  // CHECK-CC4-NEXT: f(<#int i#>, int j, int k)
+  // CHECK-CC4-NEXT: f(<#float x#>, float y)
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:39:8 %s -o - | FileCheck -check-prefix=CHECK-CC5 %s
+  // CHECK-CC5-NOT: f()
+  // CHECK-CC5: f(int i, <#int j#>, int k)
+  // CHECK-CC5-NEXT: f(float x, <#float y#>)
+  f(5, t, t);
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:49:11 %s -o - | FileCheck -check-prefix=CHECK-CC6 %s
+  // CHECK-CC6-NOT: f(float x, float y)
+  // CHECK-CC6: f(int, int, <#int#>, int)
+  // CHECK-CC6-NEXT: f(int i, int j, <#int k#>)
+}


        


More information about the cfe-commits mailing list