Author: rsmith Date: Thu Nov 12 21:52:13 2015 New Revision: 253010 URL: http://llvm.org/viewvc/llvm-project?rev=253010&view=rev Log: [modules] Follow the C++ standard's rule for linkage of enumerators: they have the linkage of the enumeration. For enumerators of unnamed enumerations, extend the -Wmodules-ambiguous-internal-linkage extension to allow selecting an arbitrary enumerator (but only if they all have the same value, otherwise it's ambiguous).
Modified: cfe/trunk/include/clang/Sema/Sema.h cfe/trunk/lib/AST/Decl.cpp cfe/trunk/lib/AST/DeclBase.cpp cfe/trunk/lib/Sema/SemaDecl.cpp cfe/trunk/lib/Sema/SemaOverload.cpp cfe/trunk/test/Index/linkage.c cfe/trunk/test/Modules/Inputs/no-linkage/decls.h cfe/trunk/test/Modules/no-linkage.cpp cfe/trunk/test/Modules/submodules-merge-defs.cpp Modified: cfe/trunk/include/clang/Sema/Sema.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=253010&r1=253009&r2=253010&view=diff ============================================================================== --- cfe/trunk/include/clang/Sema/Sema.h (original) +++ cfe/trunk/include/clang/Sema/Sema.h Thu Nov 12 21:52:13 2015 @@ -2121,7 +2121,8 @@ public: void mergeDeclAttributes(NamedDecl *New, Decl *Old, AvailabilityMergeKind AMK = AMK_Redeclaration); - void MergeTypedefNameDecl(TypedefNameDecl *New, LookupResult &OldDecls); + void MergeTypedefNameDecl(Scope *S, TypedefNameDecl *New, + LookupResult &OldDecls); bool MergeFunctionDecl(FunctionDecl *New, NamedDecl *&Old, Scope *S, bool MergeTypeWithOld); bool MergeCompatibleFunctionDecls(FunctionDecl *New, FunctionDecl *Old, Modified: cfe/trunk/lib/AST/Decl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=253010&r1=253009&r2=253010&view=diff ============================================================================== --- cfe/trunk/lib/AST/Decl.cpp (original) +++ cfe/trunk/lib/AST/Decl.cpp Thu Nov 12 21:52:13 2015 @@ -1239,7 +1239,6 @@ static LinkageInfo computeLVForDecl(cons // Note that the name of a typedef, namespace alias, using declaration, // and so on are not the name of the corresponding type, namespace, or // declaration, so they do *not* have linkage. - case Decl::EnumConstant: // FIXME: This has linkage, but that's dumb. case Decl::ImplicitParam: case Decl::Label: case Decl::NamespaceAlias: @@ -1249,6 +1248,10 @@ static LinkageInfo computeLVForDecl(cons case Decl::UsingDirective: return LinkageInfo::none(); + case Decl::EnumConstant: + // C++ [basic.link]p4: an enumerator has the linkage of its enumeration. + return getLVForDecl(cast<EnumDecl>(D->getDeclContext()), computation); + case Decl::Typedef: case Decl::TypeAlias: // A typedef declaration has linkage if it gives a type a name for Modified: cfe/trunk/lib/AST/DeclBase.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclBase.cpp?rev=253010&r1=253009&r2=253010&view=diff ============================================================================== --- cfe/trunk/lib/AST/DeclBase.cpp (original) +++ cfe/trunk/lib/AST/DeclBase.cpp Thu Nov 12 21:52:13 2015 @@ -1210,13 +1210,16 @@ void DeclContext::removeDecl(Decl *D) { // Remove only decls that have a name if (!ND->getDeclName()) return; - StoredDeclsMap *Map = getPrimaryContext()->LookupPtr; - if (!Map) return; - - StoredDeclsMap::iterator Pos = Map->find(ND->getDeclName()); - assert(Pos != Map->end() && "no lookup entry for decl"); - if (Pos->second.getAsVector() || Pos->second.getAsDecl() == ND) - Pos->second.remove(ND); + auto *DC = this; + do { + StoredDeclsMap *Map = DC->getPrimaryContext()->LookupPtr; + if (Map) { + StoredDeclsMap::iterator Pos = Map->find(ND->getDeclName()); + assert(Pos != Map->end() && "no lookup entry for decl"); + if (Pos->second.getAsVector() || Pos->second.getAsDecl() == ND) + Pos->second.remove(ND); + } + } while (DC->isTransparentContext() && (DC = DC->getParent())); } } Modified: cfe/trunk/lib/Sema/SemaDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=253010&r1=253009&r2=253010&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Nov 12 21:52:13 2015 @@ -1880,7 +1880,8 @@ bool Sema::isIncompatibleTypedef(TypeDec /// how to resolve this situation, merging decls or emitting /// diagnostics as appropriate. If there was an error, set New to be invalid. /// -void Sema::MergeTypedefNameDecl(TypedefNameDecl *New, LookupResult &OldDecls) { +void Sema::MergeTypedefNameDecl(Scope *S, TypedefNameDecl *New, + LookupResult &OldDecls) { // If the new decl is known invalid already, don't bother doing any // merging checks. if (New->isInvalidDecl()) return; @@ -1961,6 +1962,19 @@ void Sema::MergeTypedefNameDecl(TypedefN // Make the old tag definition visible. makeMergedDefinitionVisible(Hidden, NewTag->getLocation()); + + // If this was an unscoped enumeration, yank all of its enumerators + // out of the scope. + if (isa<EnumDecl>(NewTag)) { + Scope *EnumScope = getNonFieldDeclScope(S); + for (auto *D : NewTag->decls()) { + auto *ED = cast<EnumConstantDecl>(D); + assert(EnumScope->isDeclScope(ED)); + EnumScope->RemoveDecl(ED); + IdResolver.RemoveDecl(ED); + ED->getLexicalDeclContext()->removeDecl(ED); + } + } } } @@ -5212,7 +5226,7 @@ Sema::ActOnTypedefNameDecl(Scope *S, Dec filterNonConflictingPreviousTypedefDecls(*this, NewTD, Previous); if (!Previous.empty()) { Redeclaration = true; - MergeTypedefNameDecl(NewTD, Previous); + MergeTypedefNameDecl(S, NewTD, Previous); } // If this is the C FILE type, notify the AST context. @@ -13994,12 +14008,27 @@ Decl *Sema::ActOnEnumConstant(Scope *S, PrevDecl = nullptr; } + // C++ [class.mem]p15: + // If T is the name of a class, then each of the following shall have a name + // different from T: + // - every enumerator of every member of class T that is an unscoped + // enumerated type + if (!TheEnumDecl->isScoped()) + DiagnoseClassNameShadow(TheEnumDecl->getDeclContext(), + DeclarationNameInfo(Id, IdLoc)); + + EnumConstantDecl *New = + CheckEnumConstant(TheEnumDecl, LastEnumConst, IdLoc, Id, Val); + if (!New) + return nullptr; + if (PrevDecl) { // When in C++, we may get a TagDecl with the same name; in this case the // enum constant will 'hide' the tag. assert((getLangOpts().CPlusPlus || !isa<TagDecl>(PrevDecl)) && "Received TagDecl when not in C++!"); - if (!isa<TagDecl>(PrevDecl) && isDeclInScope(PrevDecl, CurContext, S)) { + if (!isa<TagDecl>(PrevDecl) && isDeclInScope(PrevDecl, CurContext, S) && + shouldLinkPossiblyHiddenDecl(PrevDecl, New)) { if (isa<EnumConstantDecl>(PrevDecl)) Diag(IdLoc, diag::err_redefinition_of_enumerator) << Id; else @@ -14009,26 +14038,12 @@ Decl *Sema::ActOnEnumConstant(Scope *S, } } - // C++ [class.mem]p15: - // If T is the name of a class, then each of the following shall have a name - // different from T: - // - every enumerator of every member of class T that is an unscoped - // enumerated type - if (!TheEnumDecl->isScoped()) - DiagnoseClassNameShadow(TheEnumDecl->getDeclContext(), - DeclarationNameInfo(Id, IdLoc)); - - EnumConstantDecl *New = - CheckEnumConstant(TheEnumDecl, LastEnumConst, IdLoc, Id, Val); + // Process attributes. + if (Attr) ProcessDeclAttributeList(S, New, Attr); - if (New) { - // Process attributes. - if (Attr) ProcessDeclAttributeList(S, New, Attr); - - // Register this decl in the current scope stack. - New->setAccess(TheEnumDecl->getAccess()); - PushOnScopeChains(New, S); - } + // Register this decl in the current scope stack. + New->setAccess(TheEnumDecl->getAccess()); + PushOnScopeChains(New, S); ActOnDocumentableDecl(New); Modified: cfe/trunk/lib/Sema/SemaOverload.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=253010&r1=253009&r2=253010&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaOverload.cpp (original) +++ cfe/trunk/lib/Sema/SemaOverload.cpp Thu Nov 12 21:52:13 2015 @@ -8576,22 +8576,55 @@ bool clang::isBetterOverloadCandidate(Se } /// Determine whether two declarations are "equivalent" for the purposes of -/// name lookup and overload resolution. This applies when the same internal -/// linkage variable or function is defined by two modules (textually including +/// name lookup and overload resolution. This applies when the same internal/no +/// linkage entity is defined by two modules (probably by textually including /// the same header). In such a case, we don't consider the declarations to /// declare the same entity, but we also don't want lookups with both /// declarations visible to be ambiguous in some cases (this happens when using /// a modularized libstdc++). bool Sema::isEquivalentInternalLinkageDeclaration(const NamedDecl *A, const NamedDecl *B) { - return A && B && isa<ValueDecl>(A) && isa<ValueDecl>(B) && - A->getDeclContext()->getRedeclContext()->Equals( - B->getDeclContext()->getRedeclContext()) && - getOwningModule(const_cast<NamedDecl *>(A)) != - getOwningModule(const_cast<NamedDecl *>(B)) && - !A->isExternallyVisible() && !B->isExternallyVisible() && - Context.hasSameType(cast<ValueDecl>(A)->getType(), - cast<ValueDecl>(B)->getType()); + auto *VA = dyn_cast_or_null<ValueDecl>(A); + auto *VB = dyn_cast_or_null<ValueDecl>(B); + if (!VA || !VB) + return false; + + // The declarations must be declaring the same name as an internal linkage + // entity in different modules. + if (!VA->getDeclContext()->getRedeclContext()->Equals( + VB->getDeclContext()->getRedeclContext()) || + getOwningModule(const_cast<ValueDecl *>(VA)) == + getOwningModule(const_cast<ValueDecl *>(VB)) || + VA->isExternallyVisible() || VB->isExternallyVisible()) + return false; + + // Check that the declarations appear to be equivalent. + // + // FIXME: Checking the type isn't really enough to resolve the ambiguity. + // For constants and functions, we should check the initializer or body is + // the same. For non-constant variables, we shouldn't allow it at all. + if (Context.hasSameType(VA->getType(), VB->getType())) + return true; + + // Enum constants within unnamed enumerations will have different types, but + // may still be similar enough to be interchangeable for our purposes. + if (auto *EA = dyn_cast<EnumConstantDecl>(VA)) { + if (auto *EB = dyn_cast<EnumConstantDecl>(VB)) { + // Only handle anonymous enums. If the enumerations were named and + // equivalent, they would have been merged to the same type. + auto *EnumA = cast<EnumDecl>(EA->getDeclContext()); + auto *EnumB = cast<EnumDecl>(EB->getDeclContext()); + if (EnumA->hasNameForLinkage() || EnumB->hasNameForLinkage() || + !Context.hasSameType(EnumA->getIntegerType(), + EnumB->getIntegerType())) + return false; + // Allow this only if the value is the same for both enumerators. + return llvm::APSInt::isSameValue(EA->getInitVal(), EB->getInitVal()); + } + } + + // Nothing else is sufficiently similar. + return false; } void Sema::diagnoseEquivalentInternalLinkageDeclarations( Modified: cfe/trunk/test/Index/linkage.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/linkage.c?rev=253010&r1=253009&r2=253010&view=diff ============================================================================== --- cfe/trunk/test/Index/linkage.c (original) +++ cfe/trunk/test/Index/linkage.c Thu Nov 12 21:52:13 2015 @@ -20,7 +20,7 @@ void f16(void) { // CHECK: EnumDecl=Baz:3:6 (Definition)linkage=External -// CHECK: EnumConstantDecl=Qux:3:12 (Definition)linkage=NoLinkage +// CHECK: EnumConstantDecl=Qux:3:12 (Definition)linkage=External // CHECK: VarDecl=x:4:5linkage=External // CHECK: FunctionDecl=foo:5:6linkage=External // CHECK: VarDecl=w:6:12linkage=Internal Modified: cfe/trunk/test/Modules/Inputs/no-linkage/decls.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/no-linkage/decls.h?rev=253010&r1=253009&r2=253010&view=diff ============================================================================== --- cfe/trunk/test/Modules/Inputs/no-linkage/decls.h (original) +++ cfe/trunk/test/Modules/Inputs/no-linkage/decls.h Thu Nov 12 21:52:13 2015 @@ -2,5 +2,4 @@ namespace RealNS { int UsingDecl; } namespace NS = RealNS; typedef int Typedef; using AliasDecl = int; -enum Enum { Enumerator }; using RealNS::UsingDecl; Modified: cfe/trunk/test/Modules/no-linkage.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/no-linkage.cpp?rev=253010&r1=253009&r2=253010&view=diff ============================================================================== --- cfe/trunk/test/Modules/no-linkage.cpp (original) +++ cfe/trunk/test/Modules/no-linkage.cpp Thu Nov 12 21:52:13 2015 @@ -6,21 +6,18 @@ namespace NS { int n; } // expected-note {{candidate}} struct Typedef { int n; }; // expected-note {{candidate}} int AliasDecl; // expected-note {{candidate}} -enum AlsoAnEnum { Enumerator }; // expected-note {{candidate}} int UsingDecl; // expected-note {{candidate}} // expected-note@decls.h:2 {{candidate}} // expected-note@decls.h:3 {{candidate}} // expected-note@decls.h:4 {{candidate}} // expected-note@decls.h:5 {{candidate}} -// expected-note@decls.h:6 {{candidate}} void use(int); void use_things() { use(Typedef().n); use(NS::n); use(AliasDecl); - use(Enumerator); use(UsingDecl); } @@ -30,6 +27,5 @@ void use_things_again() { use(Typedef().n); // expected-error {{ambiguous}} use(NS::n); // expected-error {{ambiguous}} use(AliasDecl); // expected-error {{ambiguous}} - use(Enumerator); // expected-error {{ambiguous}} use(UsingDecl); // expected-error {{ambiguous}} } Modified: cfe/trunk/test/Modules/submodules-merge-defs.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/submodules-merge-defs.cpp?rev=253010&r1=253009&r2=253010&view=diff ============================================================================== --- cfe/trunk/test/Modules/submodules-merge-defs.cpp (original) +++ cfe/trunk/test/Modules/submodules-merge-defs.cpp Thu Nov 12 21:52:13 2015 @@ -116,4 +116,12 @@ void use_static_inline() { StaticInline: // expected-note@defs.h:71 {{declared here in module 'redef'}} // expected-note@defs.h:71 {{declared here in module 'stuff.use'}} #endif +int use_anon_enum = G::g; +#ifdef EARLY_INDIRECT_INCLUDE +// expected-warning@-2 3{{ambiguous use of internal linkage declaration 'g' defined in multiple modules}} +// FIXME: These notes are produced, but -verify can't match them? +// FIXME-note@defs.h:51 3{{declared here in module 'redef'}} +// FIXME-note@defs.h:51 3{{declared here in module 'stuff.use'}} +#endif +int use_named_enum = G::i; #endif _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits