r253010 - [modules] Follow the C++ standard's rule for linkage of enumerators: they have
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 12 19:52:14 PST 2015
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 at decls.h:2 {{candidate}}
// expected-note at decls.h:3 {{candidate}}
// expected-note at decls.h:4 {{candidate}}
// expected-note at decls.h:5 {{candidate}}
-// expected-note at 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 at defs.h:71 {{declared here in module 'redef'}}
// expected-note at defs.h:71 {{declared here in module 'stuff.use'}}
#endif
+int use_anon_enum = G::g;
+#ifdef EARLY_INDIRECT_INCLUDE
+// expected-warning at -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 at defs.h:51 3{{declared here in module 'redef'}}
+// FIXME-note at defs.h:51 3{{declared here in module 'stuff.use'}}
+#endif
+int use_named_enum = G::i;
#endif
More information about the cfe-commits
mailing list