[CodeComplete] Complete a lambda when preferred type is a function

[ilya-biryukov]

Summary

Uses a heuristic to detect std::function and friends.

[kadircet]

clang/lib/Sema/SemaCodeComplete.cpp

4135 ↗

(On Diff #200677)

This looks cheesy, do we really want to perform this operation only for templates with "function" in their names?

As discussed offline maybe perform this for any template with a single(non-defaulted?) argument, or maybe even better perform a type-check as suggested by you. But I believe there would be too many false positives with the current state.

4154 ↗

(On Diff #200677)

maybe also add a placeholder for captures?

[ilya-biryukov]

• Add placeholder for captures.
• Only accept classes that have exactly one template argument.

[ilya-biryukov]

clang/lib/Sema/SemaCodeComplete.cpp

4135 ↗

(On Diff #200677)

WDYT about "function" + only template with a single arg?
FWIW, I think function in template arguments are very rare, so I'm more optimistic about false-positives even in the current form.

We have an option of making it super-restrictive and only firing on whitelisted things like std::function, boost::function, etc.
But we don't have a mechanism to configure those from the outside, so that will probably turn out too restrictive.

4154 ↗

(On Diff #200677)

Done. With = as a default.
I'd personally rather have less placeholders, but I guess that might be useful.

[kadircet]

clang/lib/Sema/SemaCodeComplete.cpp

4125 ↗

(On Diff #200717)

I thought you decided to get rid of this branch after the offline discussions, sorry for not mentioning in earlier revision.

4142 ↗

(On Diff #200717)

what about referencetype?

void test();
void (&y)() = test;

4135 ↗

(On Diff #200677)

I don't think adding "function" as a restriction is helping much on reducing false positives. I would rather get rid of that check to increase usability.

The real problem is rather checking constructors to make sure there is at least one for which we can provide a callable with the signature we found. Anything else just seems cheesy and might result in people adding more and more cases over time.

It is up to you, I am OK if you choose to keep this restriction as well.

[ilya-biryukov]

• Only work on sugared types
• Do not check for 'function' in the name

clang/lib/Sema/SemaCodeComplete.cpp

4125 ↗

(On Diff #200717)

Sorry, my bad, forgot about it.

4142 ↗

(On Diff #200717)

Lambdas are r-values, so suggesting them for l-value references would lead to an error.
R-value references to functions are so rare that I feel handling those is not worth it.

4135 ↗

(On Diff #200677)

Removed the restriction on the function way.
I don't think there's an easy way to check for conversions, though, I hope the false-positive rate would super-small.
If not, I'll find a way to check conversions or whitelist only known function names.

[kadircet]

LGTM, thanks!

clang/lib/Sema/SemaCodeComplete.cpp

4142 ↗

(On Diff #200717)

ah right

[ilya-biryukov]

Thanks for a quick review!

[ilya-biryukov]

• Remove a leftover from name check.

cfe/trunk/lib/Sema/SemaCodeComplete.cpp

if (Enumerators.Seen.count(E))
       continue;
 
     CodeCompletionResult R(E, CCP_EnumInCase, Qualifier);
     Results.AddResult(R, CurContext, nullptr, false);
   }
   Results.ExitScope();
 }
 
+/// Try to find a corresponding FunctionProtoType for function-like types (e.g.
+/// function pointers, std::function, etc).
+static const FunctionProtoType *TryDeconstructFunctionLike(QualType T) {
+  assert(!T.isNull());
+  // Try to extract first template argument from std::function<> and similar.
+  // Note we only handle the sugared types, they closely match what users wrote.
+  // We explicitly choose to not handle ClassTemplateSpecializationDecl.
+  if (auto *Specialization = T->getAs<TemplateSpecializationType>()) {
+    if (Specialization->getNumArgs() != 1)
+      return nullptr;
+    const TemplateArgument &Argument = Specialization->getArg(0);
+    if (Argument.getKind() != TemplateArgument::Type)
+      return nullptr;
+    return Argument.getAsType()->getAs<FunctionProtoType>();
+  }
+  // Handle other cases.
+  if (T->isPointerType())
+    T = T->getPointeeType();
+  return T->getAs<FunctionProtoType>();
+}
+
+/// Adds a pattern completion for a lambda expression with the specified
+/// parameter types and placeholders for parameter names.
+static void AddLambdaCompletion(ResultBuilder &Results,
+                                llvm::ArrayRef<QualType> Parameters,
+                                const LangOptions &LangOpts) {
+  CodeCompletionBuilder Completion(Results.getAllocator(),
+                                   Results.getCodeCompletionTUInfo());
+  // [](<parameters>) {}
+  Completion.AddChunk(CodeCompletionString::CK_LeftBracket);
+  Completion.AddPlaceholderChunk("=");
+  Completion.AddChunk(CodeCompletionString::CK_RightBracket);
+  if (!Parameters.empty()) {
+    Completion.AddChunk(CodeCompletionString::CK_LeftParen);
+    bool First = true;
+    for (auto Parameter : Parameters) {
+      if (!First)
+        Completion.AddChunk(CodeCompletionString::ChunkKind::CK_Comma);
+      else
+        First = false;
+
+      constexpr llvm::StringLiteral NamePlaceholder = "!#!NAME_GOES_HERE!#!";
+      std::string Type = NamePlaceholder;
+      Parameter.getAsStringInternal(Type, PrintingPolicy(LangOpts));
+      llvm::StringRef Prefix, Suffix;
+      std::tie(Prefix, Suffix) = llvm::StringRef(Type).split(NamePlaceholder);
+      Prefix = Prefix.rtrim();
+      Suffix = Suffix.ltrim();
+
+      Completion.AddTextChunk(Completion.getAllocator().CopyString(Prefix));
+      Completion.AddChunk(CodeCompletionString::CK_HorizontalSpace);
+      Completion.AddPlaceholderChunk("parameter");
+      Completion.AddTextChunk(Completion.getAllocator().CopyString(Suffix));
+    };
+    Completion.AddChunk(CodeCompletionString::CK_RightParen);
+  }
+  Completion.AddChunk(CodeCompletionString::CK_LeftBrace);
+  Completion.AddPlaceholderChunk("body");
+  Completion.AddChunk(CodeCompletionString::CK_RightBrace);
+
+  Results.AddResult(Completion.TakeString());
+}
+
 /// Perform code-completion in an expression context when we know what
 /// type we're looking for.
 void Sema::CodeCompleteExpression(Scope *S,
                                   const CodeCompleteExpressionData &Data) {
   ResultBuilder Results(
       *this, CodeCompleter->getAllocator(),
       CodeCompleter->getCodeCompletionTUInfo(),
       CodeCompletionContext(
void Sema::CodeCompleteExpression(Scope *S,
 
   if (S->getFnParent() && !Data.ObjCCollection &&
       !Data.IntegralConstantExpression)
     AddPrettyFunctionResults(getLangOpts(), Results);
 
   if (CodeCompleter->includeMacros())
     AddMacroResults(PP, Results, CodeCompleter->loadExternal(), false,
                     PreferredTypeIsPointer);
+
+  // Complete a lambda expression when preferred type is a function.
+  if (!Data.PreferredType.isNull() && getLangOpts().CPlusPlus11) {
+    if (const FunctionProtoType *F =
+            TryDeconstructFunctionLike(Data.PreferredType))
+      AddLambdaCompletion(Results, F->getParamTypes(), getLangOpts());
+  }
+
   HandleCodeCompleteResults(this, CodeCompleter, Results.getCompletionContext(),
                             Results.data(), Results.size());
 }
 
 void Sema::CodeCompleteExpression(Scope *S, QualType PreferredType,
                                   bool IsParenthesized) {
   return CodeCompleteExpression(
       S, CodeCompleteExpressionData(PreferredType, IsParenthesized));

cfe/trunk/test/CodeCompletion/lambdas.cpp

+template <class T>
+struct function {
+};
+
+
+void test() {
+  void (*x)(int, double) = nullptr;
+
+  function<void(int, double)> y = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:7:28 %s -o - | FileCheck -check-prefix=CHECK-1 %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:9:35 %s -o - | FileCheck -check-prefix=CHECK-1 %s
+  // CHECK-1: COMPLETION: Pattern : [<#=#>](int <#parameter#>, double <#parameter#>){<#body#>}
+
+  // == Placeholders for suffix types must be placed properly.
+  function<void(void(*)(int))> z = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:15:36 %s -o - | FileCheck -check-prefix=CHECK-2 %s
+  // CHECK-2: COMPLETION: Pattern : [<#=#>](void (* <#parameter#>)(int)){<#body#>}
+
+  // == No need for a parameter list if function has no parameters.
+  function<void()> a = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:20:24 %s -o - | FileCheck -check-prefix=CHECK-3 %s
+  // CHECK-3: COMPLETION: Pattern : [<#=#>]{<#body#>}
+}
+
+template <class T, class Allocator = int>
+struct vector {};
+
+void test2() {
+  // == Try to preserve types as written.
+  function<void(vector<int>)> a = {};
+
+  using function_typedef = function<void(vector<int>)>;
+  function_typedef b = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:30:35 %s -o - | FileCheck -check-prefix=CHECK-4 %s
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:33:24 %s -o - | FileCheck -check-prefix=CHECK-4 %s
+  // CHECK-4: COMPLETION: Pattern : [<#=#>](vector<int> <#parameter#>){<#body#>}
+}
+
+// Check another common function wrapper name.
+template <class T> struct unique_function {};
+
+void test3() {
+  unique_function<void()> a = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:43:31 %s -o - | FileCheck -check-prefix=CHECK-5 %s
+  // CHECK-5: COMPLETION: Pattern : [<#=#>]{<#body#>}
+}
+
+template <class T, class U> struct weird_function {};
+void test4() {
+  weird_function<void(), int> b = {};
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-patterns -code-completion-at=%s:50:35 %s -o - | FileCheck -check-prefix=CHECK-6 %s
+  // CHECK-6-NOT: COMPLETION: Pattern : [<#=
+}