[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

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

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

3473

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

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

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

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

3785

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

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!

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;
 
 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,
                                        SourceLocation Loc);

 /// 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;
 };
 
+ValueDecl *tryGetMember(CXXRecordDecl *ClassDecl, CXXScopeSpec &SS,
+                        ParsedType TemplateTypeTy,
+                        IdentifierInfo *MemberOrBase);
 } // 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>>;

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)) {
+    auto SignatureHelpCaller = [&] {
+      if (CalledSignatureHelp)
+        return;
+      CXXConstructorDecl *Constructor =
+          dyn_cast<CXXConstructorDecl>(ConstructorDecl);
+      if (!Constructor)
+        return;
+      // FIXME: Add support for Base class constructors as well.
+      if (ValueDecl *MemberDecl =
+              tryGetMember(Constructor->getParent(), SS, TemplateTypeTy, II)) {
+        Actions.ProduceConstructorSignatureHelp(
+            getCurScope(), MemberDecl->getType(), MemberDecl->getLocation(),
+            ArgExprs, T.getOpenLocation());
+        CalledSignatureHelp = true;
+      }
+    };
+    if (Tok.isNot(tok::r_paren) &&
+        ParseExpressionList(ArgExprs, CommaLocs, SignatureHelpCaller)) {
+      if (PP.isCodeCompletionReached())
+        SignatureHelpCaller();
       SkipUntil(tok::r_paren, StopAtSemi);
       return true;
     }
 
     T.consumeClose();
 
     SourceLocation EllipsisLoc;
     TryConsumeToken(tok::ellipsis, EllipsisLoc);

lib/Sema/SemaDeclCXX.cpp

public:
   }
 
 private:
   CXXRecordDecl *ClassDecl;
 };
 
 }
 
+ValueDecl *clang::tryGetMember(CXXRecordDecl *ClassDecl, CXXScopeSpec &SS,
+                        ParsedType TemplateTypeTy,
+                        IdentifierInfo *MemberOrBase) {
+  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()))) {
+        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 =
+          tryGetMember(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());

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 &#>)
+  Composition1(Base2);
+  Base2 b2_elem;
+};
+
+struct Composition2 {
+  Composition2() : c1_elem(Base2(1)) {}
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:83:34 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:83:34 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++98 -code-completion-at=%s:83:35 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  // RUN: %clang_cc1 -fsyntax-only -std=c++14 -code-completion-at=%s:83:35 %s -o - | FileCheck -check-prefix=CHECK-CC9 %s
+  Composition1 c1_elem;
+};