]> granicus.if.org Git - clang/commitdiff
Fix linkage computation for derived types in inline functions.
authorRafael Espindola <rafael.espindola@gmail.com>
Sat, 25 May 2013 17:16:20 +0000 (17:16 +0000)
committerRafael Espindola <rafael.espindola@gmail.com>
Sat, 25 May 2013 17:16:20 +0000 (17:16 +0000)
John noticed that the fix for pr15930 (r181981) didn't handle indirect
uses of local types. For example, a pointer to local struct, or a
function that returns it.

One way to implement this would be to recursively look for local
types. This would look a lot like the linkage computation itself for
types.

To avoid code duplication and utilize the existing linkage cache, this
patch just makes the computation of "type with no linkage but
externally visible because it is from an inline function"  part of the
linkage computation itself.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@182711 91177308-0d34-0410-b5e6-96231b3b80d8

13 files changed:
include/clang/AST/Decl.h
include/clang/AST/DeclBase.h
include/clang/AST/Type.h
include/clang/Basic/Linkage.h
include/clang/Basic/Visibility.h
lib/AST/Decl.cpp
lib/CodeGen/CGRTTI.cpp
lib/Serialization/ASTReaderDecl.cpp
lib/Serialization/ASTWriterDecl.cpp
test/CodeGenCXX/linkage.cpp
test/SemaCXX/linkage2.cpp
tools/libclang/CIndex.cpp
tools/libclang/IndexingContext.cpp

index aafd0bc5303e8d38b47be9ea690a5b59b46146e5..fbbb21c7ab41ccfa0c1072a9c75a23d3359341ce 100644 (file)
@@ -224,6 +224,8 @@ public:
     Linkage L = getLinkageInternal();
     if (L == UniqueExternalLinkage)
       return ExternalLinkage;
+    if (L == VisibleNoLinkage)
+      return NoLinkage;
     return L;
   }
 
@@ -232,7 +234,8 @@ public:
     return getFormalLinkage() == ExternalLinkage;
   }
   bool isExternallyVisible() const {
-    return getLinkageInternal() == ExternalLinkage;
+    Linkage L = getLinkageInternal();
+    return L == ExternalLinkage || L == VisibleNoLinkage;
   }
 
   /// \brief Determines the visibility of this entity.
index 754facfb591d2ae7596edf486e5234733d915823..347c15dd936d49ead7712bda822bf65519d5839a 100644 (file)
@@ -16,6 +16,7 @@
 
 #include "clang/AST/AttrIterator.h"
 #include "clang/AST/DeclarationName.h"
+#include "clang/Basic/Linkage.h"
 #include "clang/Basic/Specifiers.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/Support/Compiler.h"
@@ -284,15 +285,9 @@ protected:
   /// IdentifierNamespace - This specifies what IDNS_* namespace this lives in.
   unsigned IdentifierNamespace : 12;
 
-  /// \brief Whether the \c CachedLinkage field is active.
-  ///
-  /// This field is only valid for NamedDecls subclasses.
-  mutable unsigned HasCachedLinkage : 1;
-
-  /// \brief If \c HasCachedLinkage, the linkage of this declaration.
-  ///
-  /// This field is only valid for NamedDecls subclasses.
-  mutable unsigned CachedLinkage : 2;
+  /// \brief If 0, we have not computed the linkage of this declaration.
+  /// Otherwise, it is the linkage + 1.
+  mutable unsigned CacheValidAndLinkage : 3;
 
   friend class ASTDeclWriter;
   friend class ASTDeclReader;
@@ -309,7 +304,7 @@ protected:
       HasAttrs(false), Implicit(false), Used(false), Referenced(false),
       Access(AS_none), FromASTFile(0), Hidden(0),
       IdentifierNamespace(getIdentifierNamespaceForKind(DK)),
-      HasCachedLinkage(0)
+      CacheValidAndLinkage(0)
   {
     if (StatisticsEnabled) add(DK);
   }
@@ -319,7 +314,7 @@ protected:
       HasAttrs(false), Implicit(false), Used(false), Referenced(false),
       Access(AS_none), FromASTFile(0), Hidden(0),
       IdentifierNamespace(getIdentifierNamespaceForKind(DK)),
-      HasCachedLinkage(0)
+      CacheValidAndLinkage(0)
   {
     if (StatisticsEnabled) add(DK);
   }
@@ -341,6 +336,18 @@ protected:
   /// \brief Update a potentially out-of-date declaration.
   void updateOutOfDate(IdentifierInfo &II) const;
 
+  Linkage getCachedLinkage() const {
+    return Linkage(CacheValidAndLinkage - 1);
+  }
+
+  void setCachedLinkage(Linkage L) const {
+    CacheValidAndLinkage = L + 1;
+  }
+
+  bool hasCachedLinkage() const {
+    return CacheValidAndLinkage;
+  }
+
 public:
 
   /// \brief Source range that this declaration covers.
index c3e86532c31f75584aef1fa86390462381591e85..85aef1f6a6f828541c14193e86d89cb051a4ff28 100644 (file)
@@ -1194,7 +1194,7 @@ private:
     mutable unsigned CacheValid : 1;
 
     /// \brief Linkage of this type.
-    mutable unsigned CachedLinkage : 2;
+    mutable unsigned CachedLinkage : 3;
 
     /// \brief Whether this type involves and local or unnamed types.
     mutable unsigned CachedLocalOrUnnamed : 1;
index 13d2d24d8ee9ccabeb46ccf83ed295caec2533fa..7f9ce6a9092a8677a5ec58576563ed70f1345c44 100644 (file)
@@ -37,6 +37,10 @@ enum Linkage {
   /// point of view.
   UniqueExternalLinkage,
 
+  /// \brief No linkage according to the standard, but is visible from other
+  /// translation units because of types defined in a inline function.
+  VisibleNoLinkage,
+
   /// \brief External linkage, which indicates that the entity can
   /// be referred to from other translation units.
   ExternalLinkage
@@ -62,9 +66,24 @@ enum GVALinkage {
   GVA_ExplicitTemplateInstantiation
 };
 
-/// \brief Compute the minimum linkage given two linages.
+/// \brief Compute the minimum linkage given two linkages.
+///
+/// The linkage can be interpreted as a pair formed by the formal linkage and
+/// a boolean for external visibility. This is just what getFormalLinkage and
+/// isExternallyVisible return. We want the minimum of both components. The
+/// Linkage enum is defined in an order that makes this simple, we just need
+/// special cases for when VisibleNoLinkage would lose the visible bit and
+/// become NoLinkage.
 inline Linkage minLinkage(Linkage L1, Linkage L2) {
-  return L1 < L2? L1 : L2;
+  if (L2 == VisibleNoLinkage)
+    std::swap(L1, L2);
+  if (L1 == VisibleNoLinkage) {
+    if (L2 == InternalLinkage)
+      return NoLinkage;
+    if (L2 == UniqueExternalLinkage)
+      return NoLinkage;
+  }
+  return L1 < L2 ? L1 : L2;
 }
 
 } // end namespace clang
index b623b94a678760a702b0b53e98792dfe5789ba40..604f05b03414427908808720c00dd64c993fbe50 100644 (file)
@@ -49,7 +49,7 @@ inline Visibility minVisibility(Visibility L, Visibility R) {
 }
 
 class LinkageInfo {
-  uint8_t linkage_    : 2;
+  uint8_t linkage_    : 3;
   uint8_t visibility_ : 2;
   uint8_t explicit_   : 1;
 
index 8a523d6a0deacb61c2ddf82191d95600c4cf6a8a..fd0f72513767088cd9c3ee7502648a701dc57758 100644 (file)
@@ -285,30 +285,6 @@ static const FunctionDecl *getOutermostFunctionContext(const Decl *D) {
   return Ret;
 }
 
-/// Get the linkage and visibility to be used when this type is a template
-/// argument. This is normally just the linkage and visibility of the type,
-/// but for function local types we need to check the linkage and visibility
-/// of the function.
-static LinkageInfo getLIForTemplateTypeArgument(QualType T) {
-  LinkageInfo LI = T->getLinkageAndVisibility();
-  if (LI.getLinkage() != NoLinkage)
-    return LI;
-
-  const TagType *TT = dyn_cast<TagType>(T);
-  if (!TT)
-    return LI;
-
-  const Decl *D = TT->getDecl();
-  const FunctionDecl *FD = getOutermostFunctionContext(D);
-  if (!FD)
-    return LI;
-
-  if (!FD->isInlined())
-    return LI;
-
-  return FD->getLinkageAndVisibility();
-}
-
 /// \brief Get the most restrictive linkage for the types and
 /// declarations in the given template argument list.
 ///
@@ -327,7 +303,7 @@ getLVForTemplateArgumentList(ArrayRef<TemplateArgument> args) {
       continue;
 
     case TemplateArgument::Type:
-      LV.merge(getLIForTemplateTypeArgument(arg.getAsType()));
+      LV.merge(arg.getAsType()->getLinkageAndVisibility());
       continue;
 
     case TemplateArgument::Declaration:
@@ -918,39 +894,37 @@ static LinkageInfo getLVForClassMember(const NamedDecl *D,
 void NamedDecl::anchor() { }
 
 bool NamedDecl::isLinkageValid() const {
-  if (!HasCachedLinkage)
+  if (!hasCachedLinkage())
     return true;
 
   return getLVForDecl(this, LVForExplicitValue).getLinkage() ==
-    Linkage(CachedLinkage);
+         getCachedLinkage();
 }
 
 Linkage NamedDecl::getLinkageInternal() const {
-  if (HasCachedLinkage)
-    return Linkage(CachedLinkage);
+  if (hasCachedLinkage())
+    return getCachedLinkage();
 
   // We don't care about visibility here, so ask for the cheapest
   // possible visibility analysis.
-  CachedLinkage = getLVForDecl(this, LVForExplicitValue).getLinkage();
-  HasCachedLinkage = 1;
+  setCachedLinkage(getLVForDecl(this, LVForExplicitValue).getLinkage());
 
 #ifndef NDEBUG
   verifyLinkage();
 #endif
 
-  return Linkage(CachedLinkage);
+  return getCachedLinkage();
 }
 
 LinkageInfo NamedDecl::getLinkageAndVisibility() const {
   LVComputationKind computation =
     (usesTypeVisibility(this) ? LVForType : LVForValue);
   LinkageInfo LI = getLVForDecl(this, computation);
-  if (HasCachedLinkage) {
-    assert(Linkage(CachedLinkage) == LI.getLinkage());
+  if (hasCachedLinkage()) {
+    assert(getCachedLinkage() == LI.getLinkage());
     return LI;
   }
-  HasCachedLinkage = 1;
-  CachedLinkage = LI.getLinkage();
+  setCachedLinkage(LI.getLinkage());
 
 #ifndef NDEBUG
   verifyLinkage();
@@ -975,12 +949,12 @@ void NamedDecl::verifyLinkage() const {
     NamedDecl *T = cast<NamedDecl>(*I);
     if (T == this)
       continue;
-    if (T->HasCachedLinkage != 0) {
+    if (T->hasCachedLinkage()) {
       D = T;
       break;
     }
   }
-  assert(!D || D->CachedLinkage == CachedLinkage);
+  assert(!D || D->getCachedLinkage() == getCachedLinkage());
 }
 
 Optional<Visibility>
@@ -1093,7 +1067,17 @@ static LinkageInfo getLVForLocalDecl(const NamedDecl *D,
     }
   }
 
-  return LinkageInfo::none();
+  if (!isa<TagDecl>(D))
+    return LinkageInfo::none();
+
+  const FunctionDecl *FD = getOutermostFunctionContext(D);
+  if (!FD || !FD->isInlined())
+    return LinkageInfo::none();
+  LinkageInfo LV = FD->getLinkageAndVisibility();
+  if (LV.getLinkage() != ExternalLinkage)
+    return LinkageInfo::none();
+  return LinkageInfo(VisibleNoLinkage, LV.getVisibility(),
+                     LV.isVisibilityExplicit());
 }
 
 static LinkageInfo getLVForDecl(const NamedDecl *D,
@@ -1329,7 +1313,7 @@ bool NamedDecl::declarationReplaces(NamedDecl *OldD) const {
 }
 
 bool NamedDecl::hasLinkage() const {
-  return getLinkageInternal() != NoLinkage;
+  return getFormalLinkage() != NoLinkage;
 }
 
 NamedDecl *NamedDecl::getUnderlyingDeclImpl() {
index 40dc6bfa3b0e917eaf398ffddece401e368768dd..3d414eaf45004a51f166dd31b90ebcc6680c2876 100644 (file)
@@ -332,6 +332,7 @@ getTypeInfoLinkage(CodeGenModule &CGM, QualType Ty) {
   
   switch (Ty->getLinkage()) {
   case NoLinkage:
+  case VisibleNoLinkage:
   case InternalLinkage:
   case UniqueExternalLinkage:
     return llvm::GlobalValue::InternalLinkage;
index 6eed40c9513197e059446d325ceaa8296521e818..d2fed4637c8b3dba8d4ec9de580354978f69c11e 100644 (file)
@@ -530,8 +530,7 @@ void ASTDeclReader::VisitFunctionDecl(FunctionDecl *FD) {
   FD->HasImplicitReturnZero = Record[Idx++];
   FD->IsConstexpr = Record[Idx++];
   FD->HasSkippedBody = Record[Idx++];
-  FD->HasCachedLinkage = true;
-  FD->CachedLinkage = Record[Idx++];
+  FD->setCachedLinkage(Linkage(Record[Idx++]));
   FD->EndRangeLoc = ReadSourceLocation(Record, Idx);
 
   switch ((FunctionDecl::TemplatedKind)Record[Idx++]) {
@@ -920,9 +919,8 @@ void ASTDeclReader::VisitVarDecl(VarDecl *VD) {
   VD->VarDeclBits.CXXForRangeDecl = Record[Idx++];
   VD->VarDeclBits.ARCPseudoStrong = Record[Idx++];
   VD->VarDeclBits.IsConstexpr = Record[Idx++];
-  VD->HasCachedLinkage = true;
-  VD->CachedLinkage = Record[Idx++];
-  
+  VD->setCachedLinkage(Linkage(Record[Idx++]));
+
   // Only true variables (not parameters or implicit parameters) can be merged.
   if (VD->getKind() == Decl::Var)
     mergeRedeclarable(VD, Redecl);
index 7a22afaba71ce045f41e81d9ef1e01e40f1a8a95..bcbf0e3d1bd494151977dca09590396df45352c8 100644 (file)
@@ -1620,7 +1620,7 @@ void ASTWriter::WriteDeclsBlockAbbrevs() {
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isCXXForRangeDecl
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isARCPseudoStrong
   Abv->Add(BitCodeAbbrevOp(0));                         // isConstexpr
-  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 2)); // Linkage
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 3)); // Linkage
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // HasInit
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // HasMemberSpecInfo
   // Type Source Info
index ce93161cad55f46eb3d42ba41f0e29cf122c3fe9..c803a941614a9ecba6945649f9cf7a7a15620fea 100644 (file)
@@ -103,3 +103,43 @@ namespace test8 {
   }
   void *h() { return g(); }
 }
+
+namespace test9 {
+  // CHECK-DAG: define linkonce_odr void @_ZN5test91fIPZNS_1gEvE1S_5EEvT_(
+  template <typename T> void f(T) {}
+  inline void *g() {
+    struct S {
+    } s;
+    return reinterpret_cast<void *>(f<S*>);
+  }
+  void *h() { return g(); }
+}
+
+namespace test10 {
+  // CHECK-DAG: define linkonce_odr void @_ZN6test101fIPFZNS_1gEvE1S_6vEEEvT_(
+  template <typename T> void f(T) {}
+  inline void *g() {
+    struct S {
+    } s;
+    typedef S(*ftype)();
+    return reinterpret_cast<void *>(f<ftype>);
+  }
+  void *h() { return g(); }
+}
+
+namespace test11 {
+  // CHECK-DAG: define internal void @_ZN6test111fIPFZNS_1gEvE1S_7PNS_12_GLOBAL__N_11IEEEEvT_(
+  namespace {
+    struct I {
+    };
+  }
+
+  template <typename T> void f(T) {}
+  inline void *g() {
+    struct S {
+    };
+    typedef S(*ftype)(I * x);
+    return reinterpret_cast<void *>(f<ftype>);
+  }
+  void *h() { return g(); }
+}
index 3cfa98138babb3e4e0875cbadfd4617524d05b6d..ed6e9677d5a4e081b4af3c8bc08da5c4a8d6a580 100644 (file)
@@ -1,5 +1,6 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -fsyntax-only -verify -fmodules %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=gnu++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++11-extensions -Wno-local-type-template-args %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-c++11-extensions -Wno-local-type-template-args -fmodules %s
 
 namespace test1 {
   int x; // expected-note {{previous definition is here}}
@@ -65,7 +66,7 @@ namespace test6 {
   get_future();
   template <class _Rp>
   struct shared_future<_Rp&> {
-    shared_future(future<_Rp&>&& __f); // expected-warning {{rvalue references are a C++11 extension}}
+    shared_future(future<_Rp&>&& __f);
   };
   void f() {
     typedef int T;
@@ -164,3 +165,24 @@ namespace test16 {
     }
   }
 }
+
+namespace test17 {
+  namespace {
+    struct I {
+    };
+  }
+  template <typename T1, typename T2> void foo() {}
+  template <typename T, T x> void bar() {} // expected-note {{candidate function}}
+  inline void *g() {
+    struct L {
+    };
+    // foo<L, I>'s linkage should be the merge of UniqueExternalLinkage (or
+    // InternalLinkage in c++11) and VisibleNoLinkage. The correct answer is
+    // NoLinkage in both cases. This means that using foo<L, I> as a template
+    // argument should fail.
+    return reinterpret_cast<void*>(bar<typeof(foo<L, I>), foo<L, I> >); // expected-error {{reinterpret_cast cannot resolve overloaded function 'bar' to type 'void *}}
+  }
+  void h() {
+    g();
+  }
+}
index dbef6209ec8ed843ebd39a1a5cfb87684886ac82..dacd46e96fc42ac72ea78e093783d2e041100193 100644 (file)
@@ -5687,7 +5687,8 @@ CXLinkageKind clang_getCursorLinkage(CXCursor cursor) {
   const Decl *D = cxcursor::getCursorDecl(cursor);
   if (const NamedDecl *ND = dyn_cast_or_null<NamedDecl>(D))
     switch (ND->getLinkageInternal()) {
-      case NoLinkage: return CXLinkage_NoLinkage;
+      case NoLinkage:
+      case VisibleNoLinkage: return CXLinkage_NoLinkage;
       case InternalLinkage: return CXLinkage_Internal;
       case UniqueExternalLinkage: return CXLinkage_UniqueExternal;
       case ExternalLinkage: return CXLinkage_External;
index cb9b4925868351dc180e95bd7edbfe4eea528579..53561764991865ed4edbd283648c7db1fc3cfc94 100644 (file)
@@ -212,6 +212,7 @@ bool IndexingContext::isFunctionLocalDecl(const Decl *D) {
   if (const NamedDecl *ND = dyn_cast<NamedDecl>(D)) {
     switch (ND->getFormalLinkage()) {
     case NoLinkage:
+    case VisibleNoLinkage:
     case InternalLinkage:
       return true;
     case UniqueExternalLinkage: