Make typo-correction of inheriting constructors work a bit better. Limit

correction to direct base class members, and recover properly after we apply
such a correction.

llvm-svn: 207731

clang/include/clang/Sema/Sema.h

@@ -3785,7 +3785,7 @@ public:
   NamedDecl *BuildUsingDeclaration(Scope *S, AccessSpecifier AS,
                                    SourceLocation UsingLoc,
                                    CXXScopeSpec &SS,
-                                   const DeclarationNameInfo &NameInfo,
+                                   DeclarationNameInfo NameInfo,
                                    AttributeList *AttrList,
                                    bool IsInstantiation,
                                    bool HasTypenameKeyword,

clang/lib/Sema/SemaDeclCXX.cpp

@@ -7314,13 +7314,30 @@ void Sema::HideUsingShadowDecl(Scope *S, UsingShadowDecl *Shadow) {
   // be possible for this to happen, because...?
 }
 
+/// Find the base specifier for a base class with the given type.
+static CXXBaseSpecifier *findDirectBaseWithType(CXXRecordDecl *Derived,
+                                                QualType DesiredBase,
+                                                bool &AnyDependentBases) {
+  // Check whether the named type is a direct base class.
+  CanQualType CanonicalDesiredBase = DesiredBase->getCanonicalTypeUnqualified();
+  for (auto &Base : Derived->bases()) {
+    CanQualType BaseType = Base.getType()->getCanonicalTypeUnqualified();
+    if (CanonicalDesiredBase == BaseType)
+      return &Base;
+    if (BaseType->isDependentType())
+      AnyDependentBases = true;
+  }
+  return 0;
+}
+
 namespace {
 class UsingValidatorCCC : public CorrectionCandidateCallback {
 public:
   UsingValidatorCCC(bool HasTypenameKeyword, bool IsInstantiation,
-                    CXXRecordDecl *RequireMemberOf)
+                    NestedNameSpecifier *NNS, CXXRecordDecl *RequireMemberOf)
       : HasTypenameKeyword(HasTypenameKeyword),
-        IsInstantiation(IsInstantiation), RequireMemberOf(RequireMemberOf) {}
+        IsInstantiation(IsInstantiation), OldNNS(NNS),
+        RequireMemberOf(RequireMemberOf) {}
 
   bool ValidateCandidate(const TypoCorrection &Candidate) override {
     NamedDecl *ND = Candidate.getCorrectionDecl();
@@ -7329,17 +7346,48 @@ public:
     if (!ND || isa<NamespaceDecl>(ND))
       return false;
 
-    if (RequireMemberOf) {
-      auto *RD = dyn_cast<CXXRecordDecl>(ND->getDeclContext());
-      if (!RD || RequireMemberOf->isProvablyNotDerivedFrom(RD))
-        return false;
-      // FIXME: Check that the base class member is accessible?
-    }
-
     // Completely unqualified names are invalid for a 'using' declaration.
     if (Candidate.WillReplaceSpecifier() && !Candidate.getCorrectionSpecifier())
       return false;
 
+    if (RequireMemberOf) {
+      auto *FoundRecord = dyn_cast<CXXRecordDecl>(ND);
+      if (FoundRecord && FoundRecord->isInjectedClassName()) {
+        // No-one ever wants a using-declaration to name an injected-class-name
+        // of a base class, unless they're declaring an inheriting constructor.
+        ASTContext &Ctx = ND->getASTContext();
+        if (!Ctx.getLangOpts().CPlusPlus11)
+          return false;
+        QualType FoundType = Ctx.getRecordType(FoundRecord);
+
+        // Check that the injected-class-name is named as a member of its own
+        // type; we don't want to suggest 'using Derived::Base;', since that
+        // means something else.
+        NestedNameSpecifier *Specifier =
+            Candidate.WillReplaceSpecifier()
+                ? Candidate.getCorrectionSpecifier()
+                : OldNNS;
+        if (!Specifier->getAsType() ||
+            !Ctx.hasSameType(QualType(Specifier->getAsType(), 0), FoundType))
+          return false;
+
+        // Check that this inheriting constructor declaration actually names a
+        // direct base class of the current class.
+        bool AnyDependentBases = false;
+        if (!findDirectBaseWithType(RequireMemberOf,
+                                    Ctx.getRecordType(FoundRecord),
+                                    AnyDependentBases) &&
+            !AnyDependentBases)
+          return false;
+      } else {
+        auto *RD = dyn_cast<CXXRecordDecl>(ND->getDeclContext());
+        if (!RD || RequireMemberOf->isProvablyNotDerivedFrom(RD))
+          return false;
+
+        // FIXME: Check that the base class member is accessible?
+      }
+    }
+
     if (isa<TypeDecl>(ND))
       return HasTypenameKeyword || !IsInstantiation;
 
@@ -7349,6 +7397,7 @@ public:
 private:
   bool HasTypenameKeyword;
   bool IsInstantiation;
+  NestedNameSpecifier *OldNNS;
   CXXRecordDecl *RequireMemberOf;
 };
 } // end anonymous namespace
@@ -7361,7 +7410,7 @@ private:
 NamedDecl *Sema::BuildUsingDeclaration(Scope *S, AccessSpecifier AS,
                                        SourceLocation UsingLoc,
                                        CXXScopeSpec &SS,
-                                       const DeclarationNameInfo &NameInfo,
+                                       DeclarationNameInfo NameInfo,
                                        AttributeList *AttrList,
                                        bool IsInstantiation,
                                        bool HasTypenameKeyword,
@@ -7428,25 +7477,30 @@ NamedDecl *Sema::BuildUsingDeclaration(Scope *S, AccessSpecifier AS,
       D = UnresolvedUsingValueDecl::Create(Context, CurContext, UsingLoc, 
                                            QualifierLoc, NameInfo);
     }
-  } else {
+    D->setAccess(AS);
-    D = UsingDecl::Create(Context, CurContext, UsingLoc, QualifierLoc,
+    CurContext->addDecl(D);
-                          NameInfo, HasTypenameKeyword);
+    return D;
   }
-  D->setAccess(AS);
-  CurContext->addDecl(D);
 
-  if (!LookupContext) return D;
+  auto Build = [&](bool Invalid) {
-  UsingDecl *UD = cast<UsingDecl>(D);
+    UsingDecl *UD =
-
+        UsingDecl::Create(Context, CurContext, UsingLoc, QualifierLoc, NameInfo,
-  if (RequireCompleteDeclContext(SS, LookupContext)) {
+                          HasTypenameKeyword);
-    UD->setInvalidDecl();
+    UD->setAccess(AS);
+    CurContext->addDecl(UD);
+    UD->setInvalidDecl(Invalid);
     return UD;
-  }
+  };
+  auto BuildInvalid = [&]{ return Build(true); };
+  auto BuildValid = [&]{ return Build(false); };
+
+  if (RequireCompleteDeclContext(SS, LookupContext))
+    return BuildInvalid();
 
   // The normal rules do not apply to inheriting constructor declarations.
   if (NameInfo.getName().getNameKind() == DeclarationName::CXXConstructorName) {
-    if (CheckInheritingConstructorUsingDecl(UD))
+    UsingDecl *UD = BuildValid();
-      UD->setInvalidDecl();
+    CheckInheritingConstructorUsingDecl(UD);
     return UD;
   }
 
@@ -7472,32 +7526,53 @@ NamedDecl *Sema::BuildUsingDeclaration(Scope *S, AccessSpecifier AS,
 
   // Try to correct typos if possible.
   if (R.empty()) {
-    UsingValidatorCCC CCC(HasTypenameKeyword, IsInstantiation,
+    UsingValidatorCCC CCC(HasTypenameKeyword, IsInstantiation, SS.getScopeRep(),
                           dyn_cast<CXXRecordDecl>(CurContext));
     if (TypoCorrection Corrected = CorrectTypo(R.getLookupNameInfo(),
                                                R.getLookupKind(), S, &SS, CCC,
                                                CTK_ErrorRecovery)){
       // We reject any correction for which ND would be NULL.
       NamedDecl *ND = Corrected.getCorrectionDecl();
-      R.setLookupName(Corrected.getCorrection());
+
-      R.addDecl(ND);
       // We reject candidates where DroppedSpecifier == true, hence the
       // literal '0' below.
       diagnoseTypo(Corrected, PDiag(diag::err_no_member_suggest)
                                 << NameInfo.getName() << LookupContext << 0
                                 << SS.getRange());
+
+      // If we corrected to an inheriting constructor, handle it as one.
+      auto *RD = dyn_cast<CXXRecordDecl>(ND);
+      if (RD && RD->isInjectedClassName()) {
+        // Fix up the information we'll use to build the using declaration.
+        if (Corrected.WillReplaceSpecifier()) {
+          NestedNameSpecifierLocBuilder Builder;
+          Builder.MakeTrivial(Context, Corrected.getCorrectionSpecifier(),
+                              QualifierLoc.getSourceRange());
+          QualifierLoc = Builder.getWithLocInContext(Context);
+        }
+
+        NameInfo.setName(Context.DeclarationNames.getCXXConstructorName(
+            Context.getCanonicalType(Context.getRecordType(RD))));
+        NameInfo.setNamedTypeInfo(0);
+
+        // Build it and process it as an inheriting constructor.
+        UsingDecl *UD = BuildValid();
+        CheckInheritingConstructorUsingDecl(UD);
+        return UD;
+      }
+
+      // FIXME: Pick up all the declarations if we found an overloaded function.
+      R.setLookupName(Corrected.getCorrection());
+      R.addDecl(ND);
     } else {
       Diag(IdentLoc, diag::err_no_member)
         << NameInfo.getName() << LookupContext << SS.getRange();
-      UD->setInvalidDecl();
+      return BuildInvalid();
-      return UD;
     }
   }
 
-  if (R.isAmbiguous()) {
+  if (R.isAmbiguous())
-    UD->setInvalidDecl();
+    return BuildInvalid();
-    return UD;
-  }
 
   if (HasTypenameKeyword) {
     // If we asked for a typename and got a non-type decl, error out.
@@ -7506,8 +7581,7 @@ NamedDecl *Sema::BuildUsingDeclaration(Scope *S, AccessSpecifier AS,
       for (LookupResult::iterator I = R.begin(), E = R.end(); I != E; ++I)
         Diag((*I)->getUnderlyingDecl()->getLocation(),
              diag::note_using_decl_target);
-      UD->setInvalidDecl();
+      return BuildInvalid();
-      return UD;
     }
   } else {
     // If we asked for a non-typename and we got a type, error out,
@@ -7516,8 +7590,7 @@ NamedDecl *Sema::BuildUsingDeclaration(Scope *S, AccessSpecifier AS,
     if (IsInstantiation && R.getAsSingle<TypeDecl>()) {
       Diag(IdentLoc, diag::err_using_dependent_value_is_type);
       Diag(R.getFoundDecl()->getLocation(), diag::note_using_decl_target);
-      UD->setInvalidDecl();
+      return BuildInvalid();
-      return UD;
     }
   }
 
@@ -7526,10 +7599,10 @@ NamedDecl *Sema::BuildUsingDeclaration(Scope *S, AccessSpecifier AS,
   if (R.getAsSingle<NamespaceDecl>()) {
     Diag(IdentLoc, diag::err_using_decl_can_not_refer_to_namespace)
       << SS.getRange();
-    UD->setInvalidDecl();
+    return BuildInvalid();
-    return UD;
   }
 
+  UsingDecl *UD = BuildValid();
   for (LookupResult::iterator I = R.begin(), E = R.end(); I != E; ++I) {
     UsingShadowDecl *PrevDecl = 0;
     if (!CheckUsingShadowDecl(UD, *I, Previous, PrevDecl))
@@ -7549,28 +7622,20 @@ bool Sema::CheckInheritingConstructorUsingDecl(UsingDecl *UD) {
   CXXRecordDecl *TargetClass = cast<CXXRecordDecl>(CurContext);
 
   // Check whether the named type is a direct base class.
-  CanQualType CanonicalSourceType = SourceType->getCanonicalTypeUnqualified();
+  bool AnyDependentBases = false;
-  CXXRecordDecl::base_class_iterator BaseIt, BaseE;
+  auto *Base = findDirectBaseWithType(TargetClass, QualType(SourceType, 0),
-  for (BaseIt = TargetClass->bases_begin(), BaseE = TargetClass->bases_end();
+                                      AnyDependentBases);
-       BaseIt != BaseE; ++BaseIt) {
+  if (!Base && !AnyDependentBases) {
-    CanQualType BaseType = BaseIt->getType()->getCanonicalTypeUnqualified();
-    if (CanonicalSourceType == BaseType)
-      break;
-    if (BaseIt->getType()->isDependentType())
-      break;
-  }
-
-  if (BaseIt == BaseE) {
-    // Did not find SourceType in the bases.
     Diag(UD->getUsingLoc(),
          diag::err_using_decl_constructor_not_in_direct_base)
       << UD->getNameInfo().getSourceRange()
       << QualType(SourceType, 0) << TargetClass;
+    UD->setInvalidDecl();
     return true;
   }
 
-  if (!CurContext->isDependentContext())
+  if (Base)
-    BaseIt->setInheritConstructors();
+    Base->setInheritConstructors();
 
   return false;
 }

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

@@ -2182,8 +2182,7 @@ Decl *TemplateDeclInstantiator::VisitUsingDecl(UsingDecl *D) {
     return NewUD;
 
   if (NameInfo.getName().getNameKind() == DeclarationName::CXXConstructorName) {
-    if (SemaRef.CheckInheritingConstructorUsingDecl(NewUD))
+    SemaRef.CheckInheritingConstructorUsingDecl(NewUD);
-      NewUD->setInvalidDecl();
     return NewUD;
   }
 

clang/test/SemaCXX/cxx11-inheriting-ctors.cpp

@@ -26,3 +26,11 @@ namespace PR15757 {
     return 0;
   }
 }
+
+namespace WrongIdent {
+  struct A {};
+  struct B : A {};
+  struct C : B {
+    using B::A;
+  };
+}

clang/test/SemaCXX/using-decl-1.cpp

@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++98 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
 
 extern "C" { void f(bool); }
 
@@ -205,7 +206,10 @@ namespace PR19171 {
   } S;
 
   struct Y : S {
-    using S::S; // expected-error {{no member named 'S' in 'PR19171::S'}}
+    using S::S;
+#if __cplusplus < 201103L
+    // expected-error@-2 {{no member named 'S' in 'PR19171::S'}}
+#endif
   };
 
   // [namespace.udecl]p3: In a using-declaration used as a member-declaration,
@@ -213,12 +217,28 @@ namespace PR19171 {
   // If such a using-declaration names a constructor, the nested-name-specifier
   // shall name a direct base class of the class being defined;
 
-  // FIXME: For c++11, Typo correction should only consider constructor of direct base class
+  struct B_blah { };
-  struct B_blah { }; // expected-note {{'B_blah' declared here}}
+  struct C_blah : B_blah { C_blah(int); }; // expected-note 0-1{{declared here}}
-  struct C_blah : B_blah { };
+  struct D1 : C_blah {
-  struct D : C_blah {
+    // FIXME: We should be able to correct this in C++11 mode.
-    using B_blah::C_blah; // expected-error {{no member named 'C_blah' in 'PR19171::B_blah'; did you mean 'B_blah'?}}
+    using B_blah::C_blah; // expected-error-re {{no member named 'C_blah' in 'PR19171::B_blah'{{$}}}}
   };
+  struct D2 : C_blah {
+    // Somewhat bizarrely, this names the injected-class-name of B_blah within
+    // C_blah, and is valid.
+    using C_blah::B_blah;
+  };
+  struct D3 : C_blah {
+    using C_blah::D_blah;
+#if __cplusplus < 201103L
+    // expected-error-re@-2 {{no member named 'D_blah' in 'PR19171::C_blah'{{$}}}}
+#else
+    // expected-error@-4 {{no member named 'D_blah' in 'PR19171::C_blah'; did you mean 'C_blah'?}}
+#endif
+  };
+#if __cplusplus >= 201103L
+  D3 d3(0); // ok
+#endif
 
   struct E { };
   struct EE { int EE; };