r207731 - Make typo-correction of inheriting constructors work a bit better. Limit
Richard Smith
richard-llvm at metafoo.co.uk
Wed Apr 30 17:35:04 PDT 2014
Author: rsmith
Date: Wed Apr 30 19:35:04 2014
New Revision: 207731
URL: http://llvm.org/viewvc/llvm-project?rev=207731&view=rev
Log:
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.
Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/test/SemaCXX/cxx11-inheriting-ctors.cpp
cfe/trunk/test/SemaCXX/using-decl-1.cpp
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=207731&r1=207730&r2=207731&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Apr 30 19:35:04 2014
@@ -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,
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=207731&r1=207730&r2=207731&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed Apr 30 19:35:04 2014
@@ -7314,13 +7314,30 @@ void Sema::HideUsingShadowDecl(Scope *S,
// 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(S
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<UsingDecl>(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(S
// 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 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(S
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(S
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 UD;
+ return BuildInvalid();
}
}
@@ -7526,10 +7599,10 @@ NamedDecl *Sema::BuildUsingDeclaration(S
if (R.getAsSingle<NamespaceDecl>()) {
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::CheckInheritingConstructorUsi
CXXRecordDecl *TargetClass = cast<CXXRecordDecl>(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;
}
Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=207731&r1=207730&r2=207731&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Apr 30 19:35:04 2014
@@ -2182,8 +2182,7 @@ Decl *TemplateDeclInstantiator::VisitUsi
return NewUD;
if (NameInfo.getName().getNameKind() == DeclarationName::CXXConstructorName) {
- if (SemaRef.CheckInheritingConstructorUsingDecl(NewUD))
- NewUD->setInvalidDecl();
+ SemaRef.CheckInheritingConstructorUsingDecl(NewUD);
return NewUD;
}
Modified: cfe/trunk/test/SemaCXX/cxx11-inheriting-ctors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx11-inheriting-ctors.cpp?rev=207731&r1=207730&r2=207731&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/cxx11-inheriting-ctors.cpp (original)
+++ cfe/trunk/test/SemaCXX/cxx11-inheriting-ctors.cpp Wed Apr 30 19:35:04 2014
@@ -26,3 +26,11 @@ namespace PR15757 {
return 0;
}
}
+
+namespace WrongIdent {
+ struct A {};
+ struct B : A {};
+ struct C : B {
+ using B::A;
+ };
+}
Modified: cfe/trunk/test/SemaCXX/using-decl-1.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/using-decl-1.cpp?rev=207731&r1=207730&r2=207731&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/using-decl-1.cpp (original)
+++ cfe/trunk/test/SemaCXX/using-decl-1.cpp Wed Apr 30 19:35:04 2014
@@ -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 at -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 at -2 {{no member named 'D_blah' in 'PR19171::C_blah'{{$}}}}
+#else
+ // expected-error at -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; };
More information about the cfe-commits
mailing list