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

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.

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.

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;

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

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;
 

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() {

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;

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,8 +919,7 @@ 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)

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

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(); }
+}

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();
+  }
+}

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;

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: