[CodeCompletion] Enable signature help when initializing class/struct/union members.

[kadircet]

Summary

Factors out member decleration gathering and uses it in parsing to call signature
help. Doesn't support signature help for base class constructors, the code was too
coupled with diagnostic handling, but still can be factored out but just needs
more afford.

[ilya-biryukov]

include/clang/Sema/Sema.h

10798 ↗

(On Diff #164825)

The name is very generic, but the helper is only applicable to our specific case (looking up ctor member initializer). Maybe choose a different name?
Something like tryLookupCtorInitMemberDecl would be closer to what it actually does.

lib/Parse/ParseDeclCXX.cpp

3471 ↗

(On Diff #164825)

We're calling completion in other instances, this gives us valuable information (preferred type).
Why not do the same in this case?

3473 ↗

(On Diff #164825)

That's a lot of code in parser.

We could probably extract this as a helper in Sema (similar to ProduceConstructorSignatureHelp) to follow the same pattern that we have for all the other signature help calls.
Would keep the parser simpler and, as an added benefit, would allow to make tryGetMember helper private (as it should be).
WDYT?

[kadircet]

• Move ValueDecl extraction to a helper.
• Call completion handlers as well.

[ilya-biryukov]

lib/Parse/ParseDeclCXX.cpp

3472 ↗

(On Diff #164844)

Let's always call signature help and code completion here to be consistent with other cases.
We don't want to ever block completion, even in obscure cases when it's called twice.

lib/Sema/SemaDeclCXX.cpp

3784 ↗

(On Diff #164844)

NIT: invert condition to reduce nesting and simplify the code, i.e.

if (SS.getScopeRep() || TemplateTypeTy)
  return nullptr;
// rest of the code

3785 ↗

(On Diff #164844)

This comment does not look useful without a context, maybe move it to the previous call site in sema?

[ilya-biryukov]

include/clang/Sema/Sema.h

4571 ↗

(On Diff #164844)

s/decleration/declaration.

Maybe even remove the comment? the name is self-descriptive

[kadircet]

• Resolve discussions.

[kadircet]

• Update tests.

[ilya-biryukov]

LGTM. Thanks for the fix!

cfe/trunk/include/clang/Sema/Sema.h

public:
   EnumDecl *getStdAlignValT() const;
 
 private:
   // A cache representing if we've fully checked the various comparison category
   // types stored in ASTContext. The bit-index corresponds to the integer value
   // of a ComparisonCategoryType enumerator.
   llvm::SmallBitVector FullyCheckedComparisonCategories;
 
+  ValueDecl *tryLookupCtorInitMemberDecl(CXXRecordDecl *ClassDecl,
+                                         CXXScopeSpec &SS,
+                                         ParsedType TemplateTypeTy,
+                                         IdentifierInfo *MemberOrBase);
+
 public:
   /// Lookup the specified comparison category types in the standard
   ///   library, an check the VarDecls possibly returned by the operator<=>
   ///   builtins for that type.
   ///
   /// \return The type of the comparison category type corresponding to the
   ///   specified Kind, or a null type if an error occurs
   QualType CheckComparisonCategoryType(ComparisonCategoryType Kind,
public:
   /// Reports signatures for a call to CodeCompleteConsumer and returns the
   /// preferred type for the current argument. Returned type can be null.
   QualType ProduceCallSignatureHelp(Scope *S, Expr *Fn, ArrayRef<Expr *> Args,
                                     SourceLocation OpenParLoc);
   QualType ProduceConstructorSignatureHelp(Scope *S, QualType Type,
                                            SourceLocation Loc,
                                            ArrayRef<Expr *> Args,
                                            SourceLocation OpenParLoc);
+  QualType ProduceCtorInitMemberSignatureHelp(Scope *S, Decl *ConstructorDecl,
+                                              CXXScopeSpec SS,
+                                              ParsedType TemplateTypeTy,
+                                              ArrayRef<Expr *> ArgExprs,
+                                              IdentifierInfo *II,
+                                              SourceLocation OpenParLoc);
   void CodeCompleteInitializer(Scope *S, Decl *D);
   void CodeCompleteReturn(Scope *S);
   void CodeCompleteAfterIf(Scope *S);
   void CodeCompleteAssignmentRHS(Scope *S, Expr *LHS);
 
   void CodeCompleteQualifiedId(Scope *S, CXXScopeSpec &SS,
                                bool EnteringContext);
   void CodeCompleteUsing(Scope *S);

 
 /// Contains a late templated function.
 /// Will be parsed at the end of the translation unit, used by Sema & Parser.
 struct LateParsedTemplate {
   CachedTokens Toks;
   /// The template function declaration to be late parsed.
   Decl *D;
 };
-
 } // end namespace clang
 
 namespace llvm {
 // Hash a FunctionDeclAndLoc by looking at both its FunctionDecl and its
 // SourceLocation.
 template <> struct DenseMapInfo<clang::Sema::FunctionDeclAndLoc> {
   using FunctionDeclAndLoc = clang::Sema::FunctionDeclAndLoc;
   using FDBaseInfo = DenseMapInfo<clang::CanonicalDeclPtr<clang::FunctionDecl>>;

cfe/trunk/lib/Parse/ParseDeclCXX.cpp

if (TemplateId && (TemplateId->Kind == TNK_Type_template ||
       return true;
     }
   }
 
   // Parse the '('.
   if (getLangOpts().CPlusPlus11 && Tok.is(tok::l_brace)) {
     Diag(Tok, diag::warn_cxx98_compat_generalized_initializer_lists);
 
+    // FIXME: Add support for signature help inside initializer lists.
     ExprResult InitList = ParseBraceInitializer();
     if (InitList.isInvalid())
       return true;
 
     SourceLocation EllipsisLoc;
     TryConsumeToken(tok::ellipsis, EllipsisLoc);
 
     return Actions.ActOnMemInitializer(ConstructorDecl, getCurScope(), SS, II,
                                        TemplateTypeTy, DS, IdLoc,
                                        InitList.get(), EllipsisLoc);
   } else if(Tok.is(tok::l_paren)) {
     BalancedDelimiterTracker T(*this, tok::l_paren);
     T.consumeOpen();
 
     // Parse the optional expression-list.
     ExprVector ArgExprs;
     CommaLocsTy CommaLocs;
-    if (Tok.isNot(tok::r_paren) && ParseExpressionList(ArgExprs, CommaLocs)) {
+    if (Tok.isNot(tok::r_paren) &&
+        ParseExpressionList(ArgExprs, CommaLocs, [&] {
+          QualType PreferredType = Actions.ProduceCtorInitMemberSignatureHelp(
+              getCurScope(), ConstructorDecl, SS, TemplateTypeTy, ArgExprs, II,
+              T.getOpenLocation());
+          CalledSignatureHelp = true;
+          Actions.CodeCompleteExpression(getCurScope(), PreferredType);
+        })) {
+      if (PP.isCodeCompletionReached() && !CalledSignatureHelp) {
+        Actions.ProduceCtorInitMemberSignatureHelp(
+            getCurScope(), ConstructorDecl, SS, TemplateTypeTy, ArgExprs, II,
+            T.getOpenLocation());
+        CalledSignatureHelp = true;
+      }
       SkipUntil(tok::r_paren, StopAtSemi);
       return true;
     }
 
     T.consumeClose();
 
     SourceLocation EllipsisLoc;
     TryConsumeToken(tok::ellipsis, EllipsisLoc);

cfe/trunk/lib/Sema/SemaCodeComplete.cpp

for (auto C : LookupConstructors(RD)) {
     }
   }
 
   SmallVector<ResultCandidate, 8> Results;
   mergeCandidatesWithResults(*this, Results, CandidateSet, Loc);
   return ProduceSignatureHelp(*this, S, Results, Args.size(), OpenParLoc);
 }
 
+QualType Sema::ProduceCtorInitMemberSignatureHelp(
+    Scope *S, Decl *ConstructorDecl, CXXScopeSpec SS, ParsedType TemplateTypeTy,
+    ArrayRef<Expr *> ArgExprs, IdentifierInfo *II, SourceLocation OpenParLoc) {
+  if (!CodeCompleter)
+    return QualType();
+
+  CXXConstructorDecl *Constructor =
+      dyn_cast<CXXConstructorDecl>(ConstructorDecl);
+  if (!Constructor)
+    return QualType();
+  // FIXME: Add support for Base class constructors as well.
+  if (ValueDecl *MemberDecl = tryLookupCtorInitMemberDecl(
+          Constructor->getParent(), SS, TemplateTypeTy, II))
+    return ProduceConstructorSignatureHelp(getCurScope(), MemberDecl->getType(),
+                                           MemberDecl->getLocation(), ArgExprs,
+                                           OpenParLoc);
+  return QualType();
+}
+
 void Sema::CodeCompleteInitializer(Scope *S, Decl *D) {
   ValueDecl *VD = dyn_cast_or_null<ValueDecl>(D);
   if (!VD) {
     CodeCompleteOrdinaryName(S, PCC_Expression);
     return;
   }
 
   CodeCompleteExpression(S, VD->getType());

cfe/trunk/lib/Sema/SemaDeclCXX.cpp

public:
   }
 
 private:
   CXXRecordDecl *ClassDecl;
 };
 
 }
 
+ValueDecl *Sema::tryLookupCtorInitMemberDecl(CXXRecordDecl *ClassDecl,
+                                             CXXScopeSpec &SS,
+                                             ParsedType TemplateTypeTy,
+                                             IdentifierInfo *MemberOrBase) {
+  if (SS.getScopeRep() || TemplateTypeTy)
+    return nullptr;
+  DeclContext::lookup_result Result = ClassDecl->lookup(MemberOrBase);
+  if (Result.empty())
+    return nullptr;
+  ValueDecl *Member;
+  if ((Member = dyn_cast<FieldDecl>(Result.front())) ||
+      (Member = dyn_cast<IndirectFieldDecl>(Result.front())))
+    return Member;
+  return nullptr;
+}
+
 /// Handle a C++ member initializer.
 MemInitResult
 Sema::BuildMemInitializer(Decl *ConstructorD,
                           Scope *S,
                           CXXScopeSpec &SS,
                           IdentifierInfo *MemberOrBase,
                           ParsedType TemplateTypeTy,
                           const DeclSpec &DS,
Sema::BuildMemInitializer(Decl *ConstructorD,
   //   constructor's class and, if not found in that scope, are looked
   //   up in the scope containing the constructor's definition.
   //   [Note: if the constructor's class contains a member with the
   //   same name as a direct or virtual base class of the class, a
   //   mem-initializer-id naming the member or base class and composed
   //   of a single identifier refers to the class member. A
   //   mem-initializer-id for the hidden base class may be specified
   //   using a qualified name. ]
-  if (!SS.getScopeRep() && !TemplateTypeTy) {
-    // Look for a member, first.
-    DeclContext::lookup_result Result = ClassDecl->lookup(MemberOrBase);
-    if (!Result.empty()) {
-      ValueDecl *Member;
-      if ((Member = dyn_cast<FieldDecl>(Result.front())) ||
-          (Member = dyn_cast<IndirectFieldDecl>(Result.front()))) {
-        if (EllipsisLoc.isValid())
-          Diag(EllipsisLoc, diag::err_pack_expansion_member_init)
-            << MemberOrBase
-            << SourceRange(IdLoc, Init->getSourceRange().getEnd());
+
+  if (ValueDecl *Member = tryLookupCtorInitMemberDecl(
+          ClassDecl, SS, TemplateTypeTy, MemberOrBase)) {
 
-        return BuildMemberInitializer(Member, Init, IdLoc);
-      }
-    }
-  }
   // It didn't name a member, so see if it names a class.
   QualType BaseType;
   TypeSourceInfo *TInfo = nullptr;
 
   if (TemplateTypeTy) {
     BaseType = GetTypeFromParser(TemplateTypeTy, &TInfo);
   } else if (DS.getTypeSpecType() == TST_decltype) {
     BaseType = BuildDecltypeType(DS.getRepAsExpr(), DS.getTypeSpecTypeLoc());

cfe/trunk/test/CodeCompletion/ctor-initializer.cpp

struct B {
   // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:57:9 %s -o - | FileCheck -check-prefix=CHECK-CC7 %s
   // CHECK-CC7: COMPLETION: Pattern : member1(<#args#>
   // Check in the middle and at the end of identifier too.
   // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:57:13 %s -o - | FileCheck -check-prefix=CHECK-CC8 %s
   // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:57:16 %s -o - | FileCheck -check-prefix=CHECK-CC8 %s
   // CHECK-CC8: COMPLETION: Pattern : member2(<#args#>
   int member1, member2;
 };
+
+struct Base2 {
+  Base2(int);
+};
+
+struct Composition1 {
+  Composition1() : b2_elem() {}
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:73:28 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:73:28 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // CHECK-CC9: OVERLOAD: Base2(<#int#>)
+  // CHECK-CC9: OVERLOAD: Base2(<#const Base2 &#>)
+  // CHECK-CC9-NOT: OVERLOAD: Composition1
+  Composition1(Base2);
+  Base2 b2_elem;
+};
+
+struct Composition2 {
+  Composition2() : c1_elem(Base2(1)) {}
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:84:34 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:84:34 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:84:35 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:84:35 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  Composition1 c1_elem;
+};