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