From 604e7f14d672af80ca5b9044f30f3dc23d37ddd5 Mon Sep 17 00:00:00 2001 From: John McCall Date: Tue, 8 Dec 2009 07:46:18 +0000 Subject: [PATCH] Correctly implement the C++03 and 0x restrictions on class-member using declarations. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@90843 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticSemaKinds.td | 6 +- lib/Sema/Sema.h | 3 + lib/Sema/SemaDeclCXX.cpp | 231 ++++++++++++++---- lib/Sema/SemaTemplateInstantiateDecl.cpp | 13 +- .../namespace.udecl/p3-cxx0x.cpp | 28 ++- .../basic.namespace/namespace.udecl/p4.cpp | 37 ++- test/SemaCXX/using-decl-templates.cpp | 2 +- 7 files changed, 260 insertions(+), 60 deletions(-) diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 4543ee07f7..6fadb00e1c 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -105,7 +105,11 @@ def err_using_typename_non_type : Error< "'typename' keyword used on a non-type">; def err_using_dependent_value_is_type : Error< "dependent using declaration resolved to type without 'typename'">; -def err_using_decl_nested_name_specifier_is_not_a_base_class : Error< +def err_using_decl_nested_name_specifier_is_not_class : Error< + "using declaration in class refers into '%0', which is not a class">; +def err_using_decl_nested_name_specifier_is_current_class : Error< + "using declaration refers to its own class">; +def err_using_decl_nested_name_specifier_is_not_base_class : Error< "using declaration refers into '%0', which is not a base class of %1">; def err_using_decl_can_not_refer_to_class_member : Error< "using declaration can not refer to class member">; diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h index fbf1c41410..205ceb7e5b 100644 --- a/lib/Sema/Sema.h +++ b/lib/Sema/Sema.h @@ -1730,6 +1730,9 @@ public: SourceLocation IdentLoc, IdentifierInfo *Ident); + UsingShadowDecl *BuildUsingShadowDecl(Scope *S, AccessSpecifier AS, + UsingDecl *UD, NamedDecl *Target); + bool CheckUsingDeclQualifier(SourceLocation UsingLoc, const CXXScopeSpec &SS, SourceLocation NameLoc); diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index bd191ab8c9..3a3bf3e9c1 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -2889,6 +2889,9 @@ Sema::DeclPtrTy Sema::ActOnUsingDeclaration(Scope *S, break; case UnqualifiedId::IK_ConstructorName: + // C++0x inherited constructors. + if (getLangOptions().CPlusPlus0x) break; + Diag(Name.getSourceRange().getBegin(), diag::err_using_decl_constructor) << SS.getRange(); return DeclPtrTy(); @@ -2905,6 +2908,9 @@ Sema::DeclPtrTy Sema::ActOnUsingDeclaration(Scope *S, } DeclarationName TargetName = GetNameFromUnqualifiedId(Name); + if (!TargetName) + return DeclPtrTy(); + NamedDecl *UD = BuildUsingDeclaration(S, AS, UsingLoc, SS, Name.getSourceRange().getBegin(), TargetName, AttrList, @@ -2917,28 +2923,66 @@ Sema::DeclPtrTy Sema::ActOnUsingDeclaration(Scope *S, } /// Builds a shadow declaration corresponding to a 'using' declaration. -static UsingShadowDecl *BuildUsingShadowDecl(Sema &SemaRef, Scope *S, - AccessSpecifier AS, - UsingDecl *UD, NamedDecl *Orig) { +UsingShadowDecl *Sema::BuildUsingShadowDecl(Scope *S, + AccessSpecifier AS, + UsingDecl *UD, + NamedDecl *Orig) { // FIXME: diagnose hiding, collisions // If we resolved to another shadow declaration, just coalesce them. - if (isa(Orig)) { - Orig = cast(Orig)->getTargetDecl(); - assert(!isa(Orig) && "nested shadow declaration"); + NamedDecl *Target = Orig; + if (isa(Target)) { + Target = cast(Target)->getTargetDecl(); + assert(!isa(Target) && "nested shadow declaration"); } UsingShadowDecl *Shadow - = UsingShadowDecl::Create(SemaRef.Context, SemaRef.CurContext, - UD->getLocation(), UD, Orig); + = UsingShadowDecl::Create(Context, CurContext, + UD->getLocation(), UD, Target); UD->addShadowDecl(Shadow); if (S) - SemaRef.PushOnScopeChains(Shadow, S); + PushOnScopeChains(Shadow, S); else - SemaRef.CurContext->addDecl(Shadow); + CurContext->addDecl(Shadow); Shadow->setAccess(AS); + if (Orig->isInvalidDecl() || UD->isInvalidDecl()) + Shadow->setInvalidDecl(); + + // If we haven't already declared the shadow decl invalid, check + // whether the decl comes from a base class of the current class. + // We don't have to do this in C++0x because we do the check once on + // the qualifier. + else if (!getLangOptions().CPlusPlus0x && CurContext->isRecord()) { + DeclContext *OrigDC = Orig->getDeclContext(); + + // Handle enums and anonymous structs. + if (isa(OrigDC)) OrigDC = OrigDC->getParent(); + CXXRecordDecl *OrigRec = cast(OrigDC); + while (OrigRec->isAnonymousStructOrUnion()) + OrigRec = cast(OrigRec->getDeclContext()); + + if (cast(CurContext)->isProvablyNotDerivedFrom(OrigRec)) { + if (OrigDC == CurContext) { + Diag(UD->getLocation(), + diag::err_using_decl_nested_name_specifier_is_current_class) + << UD->getNestedNameRange(); + Diag(Orig->getLocation(), diag::note_using_decl_target); + Shadow->setInvalidDecl(); + return Shadow; + } + + Diag(UD->getNestedNameRange().getBegin(), + diag::err_using_decl_nested_name_specifier_is_not_base_class) + << UD->getTargetNestedNameDecl() + << cast(CurContext) + << UD->getNestedNameRange(); + Diag(Orig->getLocation(), diag::note_using_decl_target); + return Shadow; + } + } + return Shadow; } @@ -2998,41 +3042,19 @@ NamedDecl *Sema::BuildUsingDeclaration(Scope *S, AccessSpecifier AS, if (!LookupContext) return D; UsingDecl *UD = cast(D); - if (const CXXRecordDecl *RD = dyn_cast(CurContext)) { - // C++0x N2914 [namespace.udecl]p3: - // A using-declaration used as a member-declaration shall refer to a member - // of a base class of the class being defined, shall refer to a member of an - // anonymous union that is a member of a base class of the class being - // defined, or shall refer to an enumerator for an enumeration type that is - // a member of a base class of the class being defined. - - CXXRecordDecl *LookupRD = dyn_cast(LookupContext); - if (!LookupRD || !RD->isDerivedFrom(LookupRD)) { - Diag(SS.getRange().getBegin(), - diag::err_using_decl_nested_name_specifier_is_not_a_base_class) - << NNS << RD->getDeclName(); - UD->setInvalidDecl(); - return UD; - } - } else { - // C++0x N2914 [namespace.udecl]p8: - // A using-declaration for a class member shall be a member-declaration. - if (isa(LookupContext)) { - Diag(IdentLoc, diag::err_using_decl_can_not_refer_to_class_member) - << SS.getRange(); - UD->setInvalidDecl(); - return UD; - } + if (RequireCompleteDeclContext(SS)) { + UD->setInvalidDecl(); + return UD; } - // Look up the target name. Unlike most lookups, we do not want to - // hide tag declarations: tag names are visible through the using - // declaration even if hidden by ordinary names. + // Look up the target name. + LookupResult R(*this, Name, IdentLoc, LookupOrdinaryName); - // We don't hide tags behind ordinary decls if we're in a - // non-dependent context, but in a dependent context, this is - // important for the stability of two-phase lookup. + // Unlike most lookups, we don't always want to hide tag + // declarations: tag names are visible through the using declaration + // even if hidden by ordinary names, *except* in a dependent context + // where it's important for the sanity of two-phase lookup. if (!IsInstantiation) R.setHideTags(false); @@ -3082,20 +3104,141 @@ NamedDecl *Sema::BuildUsingDeclaration(Scope *S, AccessSpecifier AS, } for (LookupResult::iterator I = R.begin(), E = R.end(); I != E; ++I) - BuildUsingShadowDecl(*this, S, AS, UD, *I); + BuildUsingShadowDecl(S, AS, UD, *I); return UD; } + /// Checks that the given nested-name qualifier used in a using decl /// in the current context is appropriately related to the current /// scope. If an error is found, diagnoses it and returns true. bool Sema::CheckUsingDeclQualifier(SourceLocation UsingLoc, const CXXScopeSpec &SS, SourceLocation NameLoc) { - // FIXME: implement + DeclContext *NamedContext = computeDeclContext(SS); - return false; + if (!CurContext->isRecord()) { + // C++03 [namespace.udecl]p3: + // C++0x [namespace.udecl]p8: + // A using-declaration for a class member shall be a member-declaration. + + // If we weren't able to compute a valid scope, it must be a + // dependent class scope. + if (!NamedContext || NamedContext->isRecord()) { + Diag(NameLoc, diag::err_using_decl_can_not_refer_to_class_member) + << SS.getRange(); + return true; + } + + // Otherwise, everything is known to be fine. + return false; + } + + // The current scope is a record. + + // If the named context is dependent, we can't decide much. + if (!NamedContext) { + // FIXME: in C++0x, we can diagnose if we can prove that the + // nested-name-specifier does not refer to a base class, which is + // still possible in some cases. + + // Otherwise we have to conservatively report that things might be + // okay. + return false; + } + + if (!NamedContext->isRecord()) { + // Ideally this would point at the last name in the specifier, + // but we don't have that level of source info. + Diag(SS.getRange().getBegin(), + diag::err_using_decl_nested_name_specifier_is_not_class) + << (NestedNameSpecifier*) SS.getScopeRep() << SS.getRange(); + return true; + } + + if (getLangOptions().CPlusPlus0x) { + // C++0x [namespace.udecl]p3: + // In a using-declaration used as a member-declaration, the + // nested-name-specifier shall name a base class of the class + // being defined. + + if (cast(CurContext)->isProvablyNotDerivedFrom( + cast(NamedContext))) { + if (CurContext == NamedContext) { + Diag(NameLoc, + diag::err_using_decl_nested_name_specifier_is_current_class) + << SS.getRange(); + return true; + } + + Diag(SS.getRange().getBegin(), + diag::err_using_decl_nested_name_specifier_is_not_base_class) + << (NestedNameSpecifier*) SS.getScopeRep() + << cast(CurContext) + << SS.getRange(); + return true; + } + + return false; + } + + // C++03 [namespace.udecl]p4: + // A using-declaration used as a member-declaration shall refer + // to a member of a base class of the class being defined [etc.]. + + // Salient point: SS doesn't have to name a base class as long as + // lookup only finds members from base classes. Therefore we can + // diagnose here only if we can prove that that can't happen, + // i.e. if the class hierarchies provably don't intersect. + + // TODO: it would be nice if "definitely valid" results were cached + // in the UsingDecl and UsingShadowDecl so that these checks didn't + // need to be repeated. + + struct UserData { + llvm::DenseSet Bases; + + static bool collect(const CXXRecordDecl *Base, void *OpaqueData) { + UserData *Data = reinterpret_cast(OpaqueData); + Data->Bases.insert(Base); + return true; + } + + bool hasDependentBases(const CXXRecordDecl *Class) { + return !Class->forallBases(collect, this); + } + + /// Returns true if the base is dependent or is one of the + /// accumulated base classes. + static bool doesNotContain(const CXXRecordDecl *Base, void *OpaqueData) { + UserData *Data = reinterpret_cast(OpaqueData); + return !Data->Bases.count(Base); + } + + bool mightShareBases(const CXXRecordDecl *Class) { + return Bases.count(Class) || !Class->forallBases(doesNotContain, this); + } + }; + + UserData Data; + + // Returns false if we find a dependent base. + if (Data.hasDependentBases(cast(CurContext))) + return false; + + // Returns false if the class has a dependent base or if it or one + // of its bases is present in the base set of the current context. + if (Data.mightShareBases(cast(NamedContext))) + return false; + + Diag(SS.getRange().getBegin(), + diag::err_using_decl_nested_name_specifier_is_not_base_class) + << (NestedNameSpecifier*) SS.getScopeRep() + << cast(CurContext) + << SS.getRange(); + + return true; } Sema::DeclPtrTy Sema::ActOnNamespaceAliasDef(Scope *S, diff --git a/lib/Sema/SemaTemplateInstantiateDecl.cpp b/lib/Sema/SemaTemplateInstantiateDecl.cpp index 432217d3c4..c5173bbca7 100644 --- a/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -1071,17 +1071,12 @@ Decl *TemplateDeclInstantiator::VisitUsingShadowDecl(UsingShadowDecl *D) { cast(SemaRef.FindInstantiatedDecl(D->getTargetDecl(), TemplateArgs)); - UsingShadowDecl *InstD = UsingShadowDecl::Create(SemaRef.Context, Owner, - InstUsing->getLocation(), - InstUsing, InstTarget); - InstUsing->addShadowDecl(InstD); - - if (InstTarget->isInvalidDecl() || InstUsing->isInvalidDecl()) - InstD->setInvalidDecl(); + UsingShadowDecl *InstD = SemaRef.BuildUsingShadowDecl(/*Scope*/ 0, + D->getAccess(), + InstUsing, + InstTarget); SemaRef.Context.setInstantiatedFromUsingShadowDecl(InstD, D); - InstD->setAccess(D->getAccess()); - Owner->addDecl(InstD); return InstD; } diff --git a/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p3-cxx0x.cpp b/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p3-cxx0x.cpp index d701f885fb..8257330dcf 100644 --- a/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p3-cxx0x.cpp +++ b/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p3-cxx0x.cpp @@ -1,4 +1,4 @@ -// RUN: clang-cc -fsyntax-only -verify %s +// RUN: clang-cc -std=c++0x -fsyntax-only -verify %s // C++0x N2914. struct B { @@ -18,3 +18,29 @@ class D2 : public B { using B::x; using C::g; // expected-error{{using declaration refers into 'C::', which is not a base class of 'D2'}} }; + +namespace test1 { + struct Base { + int foo(); + }; + + struct Unrelated { + int foo(); + }; + + struct Subclass : Base { + }; + + namespace InnerNS { + int foo(); + } + + // We should be able to diagnose these without instantiation. + template struct C : Base { + using InnerNS::foo; // expected-error {{not a class}} + using Base::bar; // expected-error {{no member named 'bar'}} + using Unrelated::foo; // expected-error {{not a base class}} + using C::foo; // expected-error {{refers to its own class}} + using Subclass::foo; // expected-error {{not a base class}} + }; +} diff --git a/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp b/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp index deed96b4a9..bf314c41b5 100644 --- a/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp +++ b/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p4.cpp @@ -21,10 +21,10 @@ namespace test0 { } class Test0 { - using NonClass::type; // expected-error {{not a base class}} - using NonClass::hiding; // expected-error {{not a base class}} - using NonClass::union_member; // expected-error {{not a base class}} - using NonClass::enumerator; // expected-error {{not a base class}} + using NonClass::type; // expected-error {{not a class}} + using NonClass::hiding; // expected-error {{not a class}} + using NonClass::union_member; // expected-error {{not a class}} + using NonClass::enumerator; // expected-error {{not a class}} }; } @@ -181,3 +181,32 @@ namespace test3 { template struct C; // expected-note {{in instantiation}} } + +namespace test4 { + struct Base { + int foo(); + }; + + struct Unrelated { + int foo(); + }; + + struct Subclass : Base { + }; + + namespace InnerNS { + int foo(); + } + + // We should be able to diagnose these without instantiation. + template struct C : Base { + using InnerNS::foo; // expected-error {{not a class}} + using Base::bar; // expected-error {{no member named 'bar'}} + using Unrelated::foo; // expected-error {{not a base class}} + using C::foo; // legal in C++03 + using Subclass::foo; // legal in C++03 + + int bar(); //expected-note {{target of using declaration}} + using C::bar; // expected-error {{refers to its own class}} + }; +} diff --git a/test/SemaCXX/using-decl-templates.cpp b/test/SemaCXX/using-decl-templates.cpp index 63fa4610cb..a19223d479 100644 --- a/test/SemaCXX/using-decl-templates.cpp +++ b/test/SemaCXX/using-decl-templates.cpp @@ -10,7 +10,7 @@ template struct B : A { using A::N; // expected-error{{dependent using declaration resolved to type without 'typename'}} using A::foo; // expected-error{{no member named 'foo'}} - using A::f; // expected-error{{using declaration refers into 'A::', which is not a base class of 'B'}} + using A::f; // expected-error{{using declaration refers into 'A::', which is not a base class of 'B'}} }; B a; // expected-note{{in instantiation of template class 'struct B' requested here}} -- 2.40.0