From: Richard Smith Date: Thu, 1 May 2014 00:35:04 +0000 (+0000) Subject: Make typo-correction of inheriting constructors work a bit better. Limit X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=fc666be85c1653c29ba0c4a4799c8d6bdc9fe456;p=clang 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@207731 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 4d4fd360a3..b4278b590e 100644 --- a/include/clang/Sema/Sema.h +++ b/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, diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index c4f6ee13ea..6416b6277c 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/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(ND)) return false; - if (RequireMemberOf) { - auto *RD = dyn_cast(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(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(ND->getDeclContext()); + if (!RD || RequireMemberOf->isProvablyNotDerivedFrom(RD)) + return false; + + // FIXME: Check that the base class member is accessible? + } + } + if (isa(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 = UsingDecl::Create(Context, CurContext, UsingLoc, QualifierLoc, - NameInfo, HasTypenameKeyword); + D->setAccess(AS); + CurContext->addDecl(D); + return D; } - D->setAccess(AS); - CurContext->addDecl(D); - if (!LookupContext) return D; - UsingDecl *UD = cast(D); - - if (RequireCompleteDeclContext(SS, LookupContext)) { - UD->setInvalidDecl(); + auto Build = [&](bool Invalid) { + UsingDecl *UD = + UsingDecl::Create(Context, CurContext, UsingLoc, QualifierLoc, NameInfo, + HasTypenameKeyword); + 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)) - UD->setInvalidDecl(); + UsingDecl *UD = BuildValid(); + 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(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(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 UD; + return BuildInvalid(); } } - if (R.isAmbiguous()) { - UD->setInvalidDecl(); - return UD; - } + if (R.isAmbiguous()) + return BuildInvalid(); 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 UD; + return BuildInvalid(); } } 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()) { Diag(IdentLoc, diag::err_using_dependent_value_is_type); Diag(R.getFoundDecl()->getLocation(), diag::note_using_decl_target); - UD->setInvalidDecl(); - return UD; + return BuildInvalid(); } } @@ -7526,10 +7599,10 @@ NamedDecl *Sema::BuildUsingDeclaration(Scope *S, AccessSpecifier AS, if (R.getAsSingle()) { Diag(IdentLoc, diag::err_using_decl_can_not_refer_to_namespace) << SS.getRange(); - UD->setInvalidDecl(); - return UD; + return BuildInvalid(); } + 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(CurContext); // Check whether the named type is a direct base class. - CanQualType CanonicalSourceType = SourceType->getCanonicalTypeUnqualified(); - CXXRecordDecl::base_class_iterator BaseIt, BaseE; - for (BaseIt = TargetClass->bases_begin(), BaseE = TargetClass->bases_end(); - BaseIt != BaseE; ++BaseIt) { - CanQualType BaseType = BaseIt->getType()->getCanonicalTypeUnqualified(); - if (CanonicalSourceType == BaseType) - break; - if (BaseIt->getType()->isDependentType()) - break; - } - - if (BaseIt == BaseE) { - // Did not find SourceType in the bases. + bool AnyDependentBases = false; + auto *Base = findDirectBaseWithType(TargetClass, QualType(SourceType, 0), + AnyDependentBases); + if (!Base && !AnyDependentBases) { 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()) - BaseIt->setInheritConstructors(); + if (Base) + Base->setInheritConstructors(); return false; } diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp index 316b574fdf..8abe2fe9a0 100644 --- a/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/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)) - NewUD->setInvalidDecl(); + SemaRef.CheckInheritingConstructorUsingDecl(NewUD); return NewUD; } diff --git a/test/SemaCXX/cxx11-inheriting-ctors.cpp b/test/SemaCXX/cxx11-inheriting-ctors.cpp index 67d55213a0..04aa117b29 100644 --- a/test/SemaCXX/cxx11-inheriting-ctors.cpp +++ b/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; + }; +} diff --git a/test/SemaCXX/using-decl-1.cpp b/test/SemaCXX/using-decl-1.cpp index a91d20b0f3..40f80a70ef 100644 --- a/test/SemaCXX/using-decl-1.cpp +++ b/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 { }; // expected-note {{'B_blah' declared here}} - struct C_blah : B_blah { }; - struct D : C_blah { - using B_blah::C_blah; // expected-error {{no member named 'C_blah' in 'PR19171::B_blah'; did you mean 'B_blah'?}} + struct B_blah { }; + struct C_blah : B_blah { C_blah(int); }; // expected-note 0-1{{declared here}} + struct D1 : C_blah { + // FIXME: We should be able to correct this in C++11 mode. + 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; };