[PATCH] Implementation for completion in call context for C++

[Francisco Lopes da Silva]

Thanks for the detailed feedback Manuel.


================
Comment at: include/clang/Sema/Sema.h:8775-8776
@@ -8766,1 +8774,4 @@
 
+bool TooManyArguments(size_t NumParams, size_t NumArgs,
+                      bool PartialOverloading = false);
+
----------------
klimek wrote:
> Ok, after thinking about it for a bit, I believe we should put this into Sema (and add a comment).
Ok.

================
Comment at: lib/Sema/SemaCodeComplete.cpp:3843-3844
@@ +3842,4 @@
+
+QualType getThisParamType(Sema &S, ArrayRef<ResultCandidate> Results,
+                          ArrayRef<Expr *> Args) {
+  QualType ParamType;
----------------
klimek wrote:
> I think the names are a bit off, we need a comment, and slightly modify the function (the Args array is not used for anything but the size).
> How about:
> 
> // Get the type of the Nth parameter from a given candidate set.
> // Given the overloads 'Candidates' for a function call matching all arguments up to
> // N, return the type of the Nth parameter if it is the same for all overload candidates.
> // Otherwise return a default-constructed QualType.
> QualType getParamType(Sema &S, ArrayRef<ResultCandidate> Candidates, int N)
Ok, thanks for suggestion. I've took freedom to change the brief a bit since the commentary is long and with multiple sentences. Let me know whether it's good the way I've left it.

================
Comment at: lib/Sema/SemaCodeComplete.cpp:3893-3896
@@ +3892,6 @@
+  else if (auto UME = dyn_cast<UnresolvedMemberExpr>(NakedFn)) {
+    TemplateArgumentListInfo TemplateArgsBuffer, *TemplateArgs = nullptr;
+    if (UME->hasExplicitTemplateArgs()) {
+      UME->copyTemplateArgumentsInto(TemplateArgsBuffer);
+      TemplateArgs = &TemplateArgsBuffer;
+    }
----------------
klimek wrote:
> I now understand the need for copying, but not why you need an extra variable for the pointer instead of just passing &TemplateArgsBuffer below.
There's a convention in the codebase that a parameter of type `TemplateArgumentListInfo *`, for explicit template arguments, may be checked for `nullptr` by the callee. Also, this snippet follows a pattern used elsewhere in the codebase.

================
Comment at: lib/Sema/SemaCodeComplete.cpp:3942-3953
@@ -3868,51 +3941,14 @@
+
   QualType ParamType;
-  
-  if (!CandidateSet.empty()) {
-    // Sort the overload candidate set by placing the best overloads first.
-    std::stable_sort(
-        CandidateSet.begin(), CandidateSet.end(),
-        [&](const OverloadCandidate &X, const OverloadCandidate &Y) {
-          return isBetterOverloadCandidate(*this, X, Y, Loc);
-        });
+  if (!CandidateSet.empty())
+    ParamType = getThisParamType(*this, Results, Args);
 
-    // Add the remaining viable overload candidates as code-completion reslults.
-    for (OverloadCandidateSet::iterator Cand = CandidateSet.begin(),
-                                     CandEnd = CandidateSet.end();
-         Cand != CandEnd; ++Cand) {
-      if (Cand->Viable)
-        Results.push_back(ResultCandidate(Cand->Function));
-    }
-
-    // From the viable candidates, try to determine the type of this parameter.
-    for (unsigned I = 0, N = Results.size(); I != N; ++I) {
-      if (const FunctionType *FType = Results[I].getFunctionType())
-        if (const FunctionProtoType *Proto = dyn_cast<FunctionProtoType>(FType))
-          if (Args.size() < Proto->getNumParams()) {
-            if (ParamType.isNull())
-              ParamType = Proto->getParamType(Args.size());
-            else if (!Context.hasSameUnqualifiedType(
-                          ParamType.getNonReferenceType(),
-                          Proto->getParamType(Args.size())
-                              .getNonReferenceType())) {
-              ParamType = QualType();
-              break;
-            }
-          }
-    }
-  } else {
-    // Try to determine the parameter type from the type of the expression
-    // being called.
-    QualType FunctionType = Fn->getType();
-    if (const PointerType *Ptr = FunctionType->getAs<PointerType>())
-      FunctionType = Ptr->getPointeeType();
-    else if (const BlockPointerType *BlockPtr
-                                    = FunctionType->getAs<BlockPointerType>())
-      FunctionType = BlockPtr->getPointeeType();
-    else if (const MemberPointerType *MemPtr
-                                    = FunctionType->getAs<MemberPointerType>())
-      FunctionType = MemPtr->getPointeeType();
-    
-    if (const FunctionProtoType *Proto
-                                  = FunctionType->getAs<FunctionProtoType>()) {
-      if (Args.size() < Proto->getNumParams())
-        ParamType = Proto->getParamType(Args.size());
+  if (ParamType.isNull())
+    CodeCompleteOrdinaryName(S, PCC_Expression);
+  else
+    CodeCompleteExpression(S, ParamType);
+
+  if (!Results.empty())
+    CodeCompleter->ProcessOverloadCandidates(*this, Args.size(), Results.data(),
+                                             Results.size());
+}
----------------
klimek wrote:
> This seems to also be pretty much the same as in the function below; I'd pull out a function for that, too.
Ok.

================
Comment at: lib/Sema/SemaCodeComplete.cpp:3961-3963
@@ +3960,5 @@
+
+  // A complete type is needed to lookup for constructors
+  if(RequireCompleteType(Loc, Type, 0))
+    return;
+
----------------
klimek wrote:
> Space after 'if'; generally, use clang-format with -style=LLVM
Thanks, I'm sorry for that, I've fixed this `if` and the surrounding `for`. I'm following formatting by hand since using clang-format over the file changes a lot of unrelated stuff. Any tips to pass it over the file? Only option I see is using clang-format over the particular lines I'm editing.

================
Comment at: lib/Sema/SemaCodeComplete.cpp:3966
@@ +3965,3 @@
+  // FIXME: Provide support for variadic template constructors.
+  // FIXME: Provide support for highlighting what parameters are optional.
+
----------------
klimek wrote:
> perhaps "highlighting optional parameters"?
Ok.

http://reviews.llvm.org/D6880

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/