From a99ecbcc4c431d52df0b01539035ab5281d54656 Mon Sep 17 00:00:00 2001 From: Rafael Espindola Date: Sat, 25 May 2013 17:16:20 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@182711 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/Decl.h | 5 ++- include/clang/AST/DeclBase.h | 29 ++++++++----- include/clang/AST/Type.h | 2 +- include/clang/Basic/Linkage.h | 23 ++++++++++- include/clang/Basic/Visibility.h | 2 +- lib/AST/Decl.cpp | 64 +++++++++++------------------ lib/CodeGen/CGRTTI.cpp | 1 + lib/Serialization/ASTReaderDecl.cpp | 8 ++-- lib/Serialization/ASTWriterDecl.cpp | 2 +- test/CodeGenCXX/linkage.cpp | 40 ++++++++++++++++++ test/SemaCXX/linkage2.cpp | 28 +++++++++++-- tools/libclang/CIndex.cpp | 3 +- tools/libclang/IndexingContext.cpp | 1 + 13 files changed, 142 insertions(+), 66 deletions(-) diff --git a/include/clang/AST/Decl.h b/include/clang/AST/Decl.h index aafd0bc530..fbbb21c7ab 100644 --- a/include/clang/AST/Decl.h +++ b/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/include/clang/AST/DeclBase.h b/include/clang/AST/DeclBase.h index 754facfb59..347c15dd93 100644 --- a/include/clang/AST/DeclBase.h +++ b/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/include/clang/AST/Type.h b/include/clang/AST/Type.h index c3e86532c3..85aef1f6a6 100644 --- a/include/clang/AST/Type.h +++ b/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/include/clang/Basic/Linkage.h b/include/clang/Basic/Linkage.h index 13d2d24d8e..7f9ce6a909 100644 --- a/include/clang/Basic/Linkage.h +++ b/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/include/clang/Basic/Visibility.h b/include/clang/Basic/Visibility.h index b623b94a67..604f05b034 100644 --- a/include/clang/Basic/Visibility.h +++ b/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/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 8a523d6a0d..fd0f725137 100644 --- a/lib/AST/Decl.cpp +++ b/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(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 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(*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 @@ -1093,7 +1067,17 @@ static LinkageInfo getLVForLocalDecl(const NamedDecl *D, } } - return LinkageInfo::none(); + if (!isa(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/lib/CodeGen/CGRTTI.cpp b/lib/CodeGen/CGRTTI.cpp index 40dc6bfa3b..3d414eaf45 100644 --- a/lib/CodeGen/CGRTTI.cpp +++ b/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/lib/Serialization/ASTReaderDecl.cpp b/lib/Serialization/ASTReaderDecl.cpp index 6eed40c951..d2fed4637c 100644 --- a/lib/Serialization/ASTReaderDecl.cpp +++ b/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/lib/Serialization/ASTWriterDecl.cpp b/lib/Serialization/ASTWriterDecl.cpp index 7a22afaba7..bcbf0e3d1b 100644 --- a/lib/Serialization/ASTWriterDecl.cpp +++ b/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/test/CodeGenCXX/linkage.cpp b/test/CodeGenCXX/linkage.cpp index ce93161cad..c803a94161 100644 --- a/test/CodeGenCXX/linkage.cpp +++ b/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 void f(T) {} + inline void *g() { + struct S { + } s; + return reinterpret_cast(f); + } + void *h() { return g(); } +} + +namespace test10 { + // CHECK-DAG: define linkonce_odr void @_ZN6test101fIPFZNS_1gEvE1S_6vEEEvT_( + template void f(T) {} + inline void *g() { + struct S { + } s; + typedef S(*ftype)(); + return reinterpret_cast(f); + } + void *h() { return g(); } +} + +namespace test11 { + // CHECK-DAG: define internal void @_ZN6test111fIPFZNS_1gEvE1S_7PNS_12_GLOBAL__N_11IEEEEvT_( + namespace { + struct I { + }; + } + + template void f(T) {} + inline void *g() { + struct S { + }; + typedef S(*ftype)(I * x); + return reinterpret_cast(f); + } + void *h() { return g(); } +} diff --git a/test/SemaCXX/linkage2.cpp b/test/SemaCXX/linkage2.cpp index 3cfa98138b..ed6e9677d5 100644 --- a/test/SemaCXX/linkage2.cpp +++ b/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 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 void foo() {} + template void bar() {} // expected-note {{candidate function}} + inline void *g() { + struct L { + }; + // foo'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 as a template + // argument should fail. + return reinterpret_cast(bar), foo >); // expected-error {{reinterpret_cast cannot resolve overloaded function 'bar' to type 'void *}} + } + void h() { + g(); + } +} diff --git a/tools/libclang/CIndex.cpp b/tools/libclang/CIndex.cpp index dbef6209ec..dacd46e96f 100644 --- a/tools/libclang/CIndex.cpp +++ b/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(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/tools/libclang/IndexingContext.cpp b/tools/libclang/IndexingContext.cpp index cb9b492586..5356176499 100644 --- a/tools/libclang/IndexingContext.cpp +++ b/tools/libclang/IndexingContext.cpp @@ -212,6 +212,7 @@ bool IndexingContext::isFunctionLocalDecl(const Decl *D) { if (const NamedDecl *ND = dyn_cast(D)) { switch (ND->getFormalLinkage()) { case NoLinkage: + case VisibleNoLinkage: case InternalLinkage: return true; case UniqueExternalLinkage: -- 2.40.0