From: Douglas Gregor Date: Wed, 28 Mar 2012 16:01:27 +0000 (+0000) Subject: Unify and fix our checking of C++ [dcl.meaning]p1's requirements X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6960587df0bd1b421c11715807a4d2302a3aae3c;p=clang Unify and fix our checking of C++ [dcl.meaning]p1's requirements concerning qualified declarator-ids. We now diagnose extraneous qualification at namespace scope (which we had previously missed) and diagnose these qualification errors for all kinds of declarations; it was rather uneven before. Fixes . git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@153577 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 85ded4f06c..cd8b1e96d9 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -1091,7 +1091,7 @@ public: const LookupResult &Previous, Scope *S); bool DiagnoseClassNameShadow(DeclContext *DC, DeclarationNameInfo Info); - bool diagnoseQualifiedDeclInClass(CXXScopeSpec &SS, DeclContext *DC, + bool diagnoseQualifiedDeclaration(CXXScopeSpec &SS, DeclContext *DC, DeclarationName Name, SourceLocation Loc); void DiagnoseFunctionSpecifiers(Declarator& D); diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index d4159bf189..75f7f99efb 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -3236,40 +3236,96 @@ bool Sema::DiagnoseClassNameShadow(DeclContext *DC, return false; } -/// \brief Diagnose a declaration that has a qualified name within a class, -/// which is ill-formed but often recoverable. +/// \brief Diagnose a declaration whose declarator-id has the given +/// nested-name-specifier. +/// +/// \param SS The nested-name-specifier of the declarator-id. +/// +/// \param DC The declaration context to which the nested-name-specifier +/// resolves. +/// +/// \param Name The name of the entity being declared. +/// +/// \param Loc The location of the name of the entity being declared. /// /// \returns true if we cannot safely recover from this error, false otherwise. -bool Sema::diagnoseQualifiedDeclInClass(CXXScopeSpec &SS, DeclContext *DC, +bool Sema::diagnoseQualifiedDeclaration(CXXScopeSpec &SS, DeclContext *DC, DeclarationName Name, - SourceLocation Loc) { - // The user provided a superfluous scope specifier inside a class - // definition: + SourceLocation Loc) { + DeclContext *Cur = CurContext; + while (isa(Cur)) + Cur = Cur->getParent(); + + // C++ [dcl.meaning]p1: + // A declarator-id shall not be qualified except for the definition + // of a member function (9.3) or static data member (9.4) outside of + // its class, the definition or explicit instantiation of a function + // or variable member of a namespace outside of its namespace, or the + // definition of an explicit specialization outside of its namespace, + // or the declaration of a friend function that is a member of + // another class or namespace (11.3). [...] + + // The user provided a superfluous scope specifier that refers back to the + // class or namespaces in which the entity is already declared. // // class X { // void X::f(); // }; - if (CurContext->Equals(DC)) { + if (Cur->Equals(DC)) { Diag(Loc, diag::warn_member_extra_qualification) << Name << FixItHint::CreateRemoval(SS.getRange()); SS.clear(); return false; } - - Diag(Loc, diag::err_member_qualification) - << Name << SS.getRange(); - SS.clear(); - - // C++ constructors and destructors with incorrect scopes can break - // our AST invariants by having the wrong underlying types. If - // that's the case, then drop this declaration entirely. - if ((Name.getNameKind() == DeclarationName::CXXConstructorName || - Name.getNameKind() == DeclarationName::CXXDestructorName) && - !Context.hasSameType(Name.getCXXNameType(), - Context.getTypeDeclType( - cast(CurContext)))) + + // Check whether the qualifying scope encloses the scope of the original + // declaration. + if (!Cur->Encloses(DC)) { + if (Cur->isRecord()) + Diag(Loc, diag::err_member_qualification) + << Name << SS.getRange(); + else if (isa(DC)) + Diag(Loc, diag::err_invalid_declarator_global_scope) + << Name << SS.getRange(); + else if (isa(Cur)) + Diag(Loc, diag::err_invalid_declarator_in_function) + << Name << SS.getRange(); + else + Diag(Loc, diag::err_invalid_declarator_scope) + << Name << cast(DC) << SS.getRange(); + return true; + } + + if (Cur->isRecord()) { + // Cannot qualify members within a class. + Diag(Loc, diag::err_member_qualification) + << Name << SS.getRange(); + SS.clear(); + + // C++ constructors and destructors with incorrect scopes can break + // our AST invariants by having the wrong underlying types. If + // that's the case, then drop this declaration entirely. + if ((Name.getNameKind() == DeclarationName::CXXConstructorName || + Name.getNameKind() == DeclarationName::CXXDestructorName) && + !Context.hasSameType(Name.getCXXNameType(), + Context.getTypeDeclType(cast(Cur)))) + return true; + + return false; + } + // C++11 [dcl.meaning]p1: + // [...] "The nested-name-specifier of the qualified declarator-id shall + // not begin with a decltype-specifer" + NestedNameSpecifierLoc SpecLoc(SS.getScopeRep(), SS.location_data()); + while (SpecLoc.getPrefix()) + SpecLoc = SpecLoc.getPrefix(); + if (dyn_cast_or_null( + SpecLoc.getNestedNameSpecifier()->getAsType())) + Diag(Loc, diag::err_decltype_in_declarator) + << SpecLoc.getTypeLoc().getSourceRange(); + return false; } @@ -3323,17 +3379,18 @@ Decl *Sema::HandleDeclarator(Scope *S, Declarator &D, RequireCompleteDeclContext(D.getCXXScopeSpec(), DC)) return 0; - if (isa(DC)) { - if (!cast(DC)->hasDefinition()) { - Diag(D.getIdentifierLoc(), - diag::err_member_def_undefined_record) - << Name << DC << D.getCXXScopeSpec().getRange(); - D.setInvalidType(); - } else if (isa(CurContext) && - !D.getDeclSpec().isFriendSpecified()) { - if (diagnoseQualifiedDeclInClass(D.getCXXScopeSpec(), DC, - Name, D.getIdentifierLoc())) + if (isa(DC) && !cast(DC)->hasDefinition()) { + Diag(D.getIdentifierLoc(), + diag::err_member_def_undefined_record) + << Name << DC << D.getCXXScopeSpec().getRange(); + D.setInvalidType(); + } else if (!D.getDeclSpec().isFriendSpecified()) { + if (diagnoseQualifiedDeclaration(D.getCXXScopeSpec(), DC, + Name, D.getIdentifierLoc())) { + if (DC->isRecord()) return 0; + + D.setInvalidType(); } } @@ -3391,21 +3448,16 @@ Decl *Sema::HandleDeclarator(Scope *S, Declarator &D, } else { // Something like "int foo::x;" LookupQualifiedName(Previous, DC); - // Don't consider using declarations as previous declarations for - // out-of-line members. - RemoveUsingDecls(Previous); - - // C++ 7.3.1.2p2: - // Members (including explicit specializations of templates) of a named - // namespace can also be defined outside that namespace by explicit - // qualification of the name being defined, provided that the entity being - // defined was already declared in the namespace and the definition appears - // after the point of declaration in a namespace that encloses the - // declarations namespace. + // C++ [dcl.meaning]p1: + // When the declarator-id is qualified, the declaration shall refer to a + // previously declared member of the class or namespace to which the + // qualifier refers (or, in the case of a namespace, of an element of the + // inline namespace set of that namespace (7.3.1)) or to a specialization + // thereof; [...] // - // Note that we only check the context at this point. We don't yet - // have enough information to make sure that PrevDecl is actually - // the declaration we want to match. For example, given: + // Note that we already checked the context above, and that we do not have + // enough information to make sure that Previous contains the declaration + // we want to match. For example, given: // // class X { // void f(); @@ -3414,45 +3466,15 @@ Decl *Sema::HandleDeclarator(Scope *S, Declarator &D, // // void X::f(int) { } // ill-formed // - // In this case, PrevDecl will point to the overload set + // In this case, Previous will point to the overload set // containing the two f's declared in X, but neither of them // matches. - - // First check whether we named the global scope. - if (isa(DC)) { - Diag(D.getIdentifierLoc(), diag::err_invalid_declarator_global_scope) - << Name << D.getCXXScopeSpec().getRange(); - } else { - DeclContext *Cur = CurContext; - while (isa(Cur)) - Cur = Cur->getParent(); - if (!Cur->Encloses(DC)) { - // The qualifying scope doesn't enclose the original declaration. - // Emit diagnostic based on current scope. - SourceLocation L = D.getIdentifierLoc(); - SourceRange R = D.getCXXScopeSpec().getRange(); - if (isa(Cur)) - Diag(L, diag::err_invalid_declarator_in_function) << Name << R; - else - Diag(L, diag::err_invalid_declarator_scope) - << Name << cast(DC) << R; - D.setInvalidType(); - } - - // C++11 8.3p1: - // ... "The nested-name-specifier of the qualified declarator-id shall - // not begin with a decltype-specifer" - NestedNameSpecifierLoc SpecLoc = - D.getCXXScopeSpec().getWithLocInContext(Context); - assert(SpecLoc && "A non-empty CXXScopeSpec should have a non-empty " - "NestedNameSpecifierLoc"); - while (SpecLoc.getPrefix()) - SpecLoc = SpecLoc.getPrefix(); - if (dyn_cast_or_null( - SpecLoc.getNestedNameSpecifier()->getAsType())) - Diag(SpecLoc.getBeginLoc(), diag::err_decltype_in_declarator) - << SpecLoc.getTypeLoc().getSourceRange(); - } + + // C++ [dcl.meaning]p1: + // [...] the member shall not merely have been introduced by a + // using-declaration in the scope of the class or namespace nominated by + // the nested-name-specifier of the declarator-id. + RemoveUsingDecls(Previous); } if (Previous.isSingleResult() && @@ -7916,6 +7938,7 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK, bool ScopedEnumUsesClassTag, TypeResult UnderlyingType) { // If this is not a definition, it must have a name. + IdentifierInfo *OrigName = Name; assert((Name != 0 || TUK == TUK_Definition) && "Nameless record must be a definition!"); assert(TemplateParameterLists.size() == 0 || TUK != TUK_Reference); @@ -8028,9 +8051,6 @@ Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK, << SS.getRange(); return 0; } - - if (isa(CurContext)) - diagnoseQualifiedDeclInClass(SS, DC, Name, NameLoc); } if (RequireCompleteDeclContext(SS, DC)) @@ -8491,6 +8511,16 @@ CreateNewDecl: // Maybe add qualifier info. if (SS.isNotEmpty()) { if (SS.isSet()) { + // If this is either a declaration or a definition, check the + // nested-name-specifier against the current context. We don't do this + // for explicit specializations, because they have similar checking + // (with more specific diagnostics) in the call to + // CheckMemberSpecialization, below. + if (!isExplicitSpecialization && + (TUK == TUK_Definition || TUK == TUK_Declaration) && + diagnoseQualifiedDeclaration(SS, DC, OrigName, NameLoc)) + Invalid = true; + New->setQualifierInfo(SS.getWithLocInContext(Context)); if (TemplateParameterLists.size() > 0) { New->setTemplateParameterListsInfo(Context, @@ -8533,7 +8563,7 @@ CreateNewDecl: // check the specialization. if (isExplicitSpecialization && CheckMemberSpecialization(New, Previous)) Invalid = true; - + if (Invalid) New->setInvalidDecl(); diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index c8c3af3cd5..703af2b6ab 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -1538,10 +1538,8 @@ Sema::ActOnCXXMemberDeclarator(Scope *S, AccessSpecifier AS, Declarator &D, // class X { // int X::member; // }; - DeclContext *DC = 0; - if ((DC = computeDeclContext(SS, false)) && DC->Equals(CurContext)) - Diag(D.getIdentifierLoc(), diag::warn_member_extra_qualification) - << Name << FixItHint::CreateRemoval(SS.getRange()); + if (DeclContext *DC = computeDeclContext(SS, false)) + diagnoseQualifiedDeclaration(SS, DC, Name, D.getIdentifierLoc()); else Diag(D.getIdentifierLoc(), diag::err_member_qualification) << Name << SS.getRange(); diff --git a/lib/Sema/SemaTemplate.cpp b/lib/Sema/SemaTemplate.cpp index 3a4a6f5312..34e5d722ca 100644 --- a/lib/Sema/SemaTemplate.cpp +++ b/lib/Sema/SemaTemplate.cpp @@ -887,9 +887,8 @@ Sema::CheckClassTemplate(Scope *S, unsigned TagSpec, TagUseKind TUK, ContextRAII SavedContext(*this, SemanticContext); if (RebuildTemplateParamsInCurrentInstantiation(TemplateParams)) Invalid = true; - } else if (CurContext->isRecord() && TUK != TUK_Friend && - TUK != TUK_Reference) - diagnoseQualifiedDeclInClass(SS, SemanticContext, Name, NameLoc); + } else if (TUK != TUK_Friend && TUK != TUK_Reference) + diagnoseQualifiedDeclaration(SS, SemanticContext, Name, NameLoc); LookupQualifiedName(Previous, SemanticContext); } else { diff --git a/test/CXX/dcl.decl/dcl.meaning/p1.cpp b/test/CXX/dcl.decl/dcl.meaning/p1.cpp index 9aa3a20bcd..3672ea0ea0 100644 --- a/test/CXX/dcl.decl/dcl.meaning/p1.cpp +++ b/test/CXX/dcl.decl/dcl.meaning/p1.cpp @@ -20,3 +20,18 @@ namespace PR8019 { }; } + +namespace NS { + void foo(); + extern int bar; + struct X; + template struct Y; + template void wibble(T); +} +namespace NS { + void NS::foo() {} // expected-warning{{extra qualification on member 'foo'}} + int NS::bar; // expected-warning{{extra qualification on member 'bar'}} + struct NS::X { }; // expected-warning{{extra qualification on member 'X'}} + template struct NS::Y; // expected-warning{{extra qualification on member 'Y'}} + template void NS::wibble(T) { } // expected-warning{{extra qualification on member 'wibble'}} +} diff --git a/test/CXX/temp/p3.cpp b/test/CXX/temp/p3.cpp index 16ebb381fd..c146bc4faa 100644 --- a/test/CXX/temp/p3.cpp +++ b/test/CXX/temp/p3.cpp @@ -6,11 +6,9 @@ template struct S { template int S::a, S::b; // expected-error {{can only declare a single entity}} -// FIXME: the last two diagnostics here are terrible. template struct A { static A a; } A::a; // expected-error {{expected ';' after struct}} \ expected-error {{use of undeclared identifier 'T'}} \ - expected-error {{cannot name the global scope}} \ - expected-error {{no member named 'a' in the global namespace}} + expected-warning{{extra qualification}} template struct B { } f(); // expected-error {{expected ';' after struct}} \ expected-error {{requires a type specifier}} diff --git a/test/SemaCXX/nested-name-spec.cpp b/test/SemaCXX/nested-name-spec.cpp index 6b3cb23e2a..a60b115695 100644 --- a/test/SemaCXX/nested-name-spec.cpp +++ b/test/SemaCXX/nested-name-spec.cpp @@ -160,7 +160,7 @@ namespace N { void f(); // FIXME: if we move this to a separate definition of N, things break! } -void ::global_func2(int) { } // expected-error{{definition or redeclaration of 'global_func2' cannot name the global scope}} +void ::global_func2(int) { } // expected-warning{{extra qualification on member 'global_func2'}} void N::f() { } // okay