Fix linkage computation for derived types in inline functions.

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.

llvm-svn: 182711

13 files changed, 142 insertions, 66 deletions

diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index aafd0bc5303..fbbb21c7ab4 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -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.
diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h
index 754facfb591..347c15dd936 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -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.
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index c3e86532c31..85aef1f6a6f 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -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;
diff --git a/clang/include/clang/Basic/Linkage.h b/clang/include/clang/Basic/Linkage.h
index 13d2d24d8ee..7f9ce6a9092 100644
--- a/clang/include/clang/Basic/Linkage.h
+++ b/clang/include/clang/Basic/Linkage.h
@@ -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
diff --git a/clang/include/clang/Basic/Visibility.h b/clang/include/clang/Basic/Visibility.h
index b623b94a678..604f05b0341 100644
--- a/clang/include/clang/Basic/Visibility.h
+++ b/clang/include/clang/Basic/Visibility.h
@@ -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;
 
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index 8a523d6a0de..fd0f7251376 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -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() {
diff --git a/clang/lib/CodeGen/CGRTTI.cpp b/clang/lib/CodeGen/CGRTTI.cpp
index 40dc6bfa3b0..3d414eaf450 100644
--- a/clang/lib/CodeGen/CGRTTI.cpp
+++ b/clang/lib/CodeGen/CGRTTI.cpp
@@ -332,6 +332,7 @@ getTypeInfoLinkage(CodeGenModule &CGM, QualType Ty) {
   
   switch (Ty->getLinkage()) {
   case NoLinkage:
+  case VisibleNoLinkage:
   case InternalLinkage:
   case UniqueExternalLinkage:
     return llvm::GlobalValue::InternalLinkage;
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 6eed40c9513..d2fed4637c8 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -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);
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index 7a22afaba71..bcbf0e3d1bd 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -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
diff --git a/clang/test/CodeGenCXX/linkage.cpp b/clang/test/CodeGenCXX/linkage.cpp
index ce93161cad5..c803a941614 100644
--- a/clang/test/CodeGenCXX/linkage.cpp
+++ b/clang/test/CodeGenCXX/linkage.cpp
@@ -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(); }
+}
diff --git a/clang/test/SemaCXX/linkage2.cpp b/clang/test/SemaCXX/linkage2.cpp
index 3cfa98138ba..ed6e9677d5a 100644
--- a/clang/test/SemaCXX/linkage2.cpp
+++ b/clang/test/SemaCXX/linkage2.cpp
@@ -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();
+  }
+}
diff --git a/clang/tools/libclang/CIndex.cpp b/clang/tools/libclang/CIndex.cpp
index dbef6209ec8..dacd46e96fc 100644
--- a/clang/tools/libclang/CIndex.cpp
+++ b/clang/tools/libclang/CIndex.cpp
@@ -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;
diff --git a/clang/tools/libclang/IndexingContext.cpp b/clang/tools/libclang/IndexingContext.cpp
index cb9b4925868..53561764991 100644
--- a/clang/tools/libclang/IndexingContext.cpp
+++ b/clang/tools/libclang/IndexingContext.cpp
@@ -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: