From: Richard Smith Date: Thu, 5 Sep 2019 01:23:47 +0000 (+0000) Subject: [c++20] Fix some ambiguities in our mangling of lambdas with explicit X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6f53b4f252fdf2ca8fcc26286e1830d09e89cea2;p=clang [c++20] Fix some ambiguities in our mangling of lambdas with explicit template parameters. This finishes the implementation of the proposal described in https://github.com/itanium-cxx-abi/cxx-abi/issues/31. (We already implemented the extensions, but didn't take them into account when computing mangling numbers, and didn't deal properly with expanded parameter packs, and didn't disambiguate between different levels of template parameters in manglings.) git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@371004 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/Mangle.h b/include/clang/AST/Mangle.h index b1fbe93613..1362756f55 100644 --- a/include/clang/AST/Mangle.h +++ b/include/clang/AST/Mangle.h @@ -170,6 +170,8 @@ public: virtual void mangleCXXDtorComdat(const CXXDestructorDecl *D, raw_ostream &) = 0; + virtual void mangleLambdaSig(const CXXRecordDecl *Lambda, raw_ostream &) = 0; + static bool classof(const MangleContext *C) { return C->getKind() == MK_Itanium; } diff --git a/lib/AST/DeclBase.cpp b/lib/AST/DeclBase.cpp index fd80e1532e..aa2c21230c 100644 --- a/lib/AST/DeclBase.cpp +++ b/lib/AST/DeclBase.cpp @@ -12,6 +12,7 @@ #include "clang/AST/DeclBase.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/ASTLambda.h" #include "clang/AST/ASTMutationListener.h" #include "clang/AST/Attr.h" #include "clang/AST/AttrIterator.h" @@ -1043,6 +1044,12 @@ DeclContext *DeclContext::getLookupParent() { getLexicalParent()->getRedeclContext()->isRecord()) return getLexicalParent(); + // A lookup within the call operator of a lambda never looks in the lambda + // class; instead, skip to the context in which that closure type is + // declared. + if (isLambdaCallOperator(this)) + return getParent()->getParent(); + return getParent(); } diff --git a/lib/AST/ItaniumCXXABI.cpp b/lib/AST/ItaniumCXXABI.cpp index 67f874b7b9..069add8464 100644 --- a/lib/AST/ItaniumCXXABI.cpp +++ b/lib/AST/ItaniumCXXABI.cpp @@ -19,10 +19,12 @@ #include "CXXABI.h" #include "clang/AST/ASTContext.h" #include "clang/AST/DeclCXX.h" +#include "clang/AST/Mangle.h" #include "clang/AST/MangleNumberingContext.h" #include "clang/AST/RecordLayout.h" #include "clang/AST/Type.h" #include "clang/Basic/TargetInfo.h" +#include "llvm/ADT/FoldingSet.h" #include "llvm/ADT/iterator.h" using namespace clang; @@ -73,10 +75,33 @@ struct DecompositionDeclName { } namespace llvm { +template bool isDenseMapKeyEmpty(T V) { + return llvm::DenseMapInfo::isEqual( + V, llvm::DenseMapInfo::getEmptyKey()); +} +template bool isDenseMapKeyTombstone(T V) { + return llvm::DenseMapInfo::isEqual( + V, llvm::DenseMapInfo::getTombstoneKey()); +} + +template +Optional areDenseMapKeysEqualSpecialValues(T LHS, T RHS) { + bool LHSEmpty = isDenseMapKeyEmpty(LHS); + bool RHSEmpty = isDenseMapKeyEmpty(RHS); + if (LHSEmpty || RHSEmpty) + return LHSEmpty && RHSEmpty; + + bool LHSTombstone = isDenseMapKeyTombstone(LHS); + bool RHSTombstone = isDenseMapKeyTombstone(RHS); + if (LHSTombstone || RHSTombstone) + return LHSTombstone && RHSTombstone; + + return None; +} + template<> struct DenseMapInfo { using ArrayInfo = llvm::DenseMapInfo>; - using IdentInfo = llvm::DenseMapInfo; static DecompositionDeclName getEmptyKey() { return {ArrayInfo::getEmptyKey()}; } @@ -88,10 +113,10 @@ struct DenseMapInfo { return llvm::hash_combine_range(Key.begin(), Key.end()); } static bool isEqual(DecompositionDeclName LHS, DecompositionDeclName RHS) { - if (ArrayInfo::isEqual(LHS.Bindings, ArrayInfo::getEmptyKey())) - return ArrayInfo::isEqual(RHS.Bindings, ArrayInfo::getEmptyKey()); - if (ArrayInfo::isEqual(LHS.Bindings, ArrayInfo::getTombstoneKey())) - return ArrayInfo::isEqual(RHS.Bindings, ArrayInfo::getTombstoneKey()); + if (Optional Result = areDenseMapKeysEqualSpecialValues( + LHS.Bindings, RHS.Bindings)) + return *Result; + return LHS.Bindings.size() == RHS.Bindings.size() && std::equal(LHS.begin(), LHS.end(), RHS.begin()); } @@ -103,29 +128,32 @@ namespace { /// Keeps track of the mangled names of lambda expressions and block /// literals within a particular context. class ItaniumNumberingContext : public MangleNumberingContext { - llvm::DenseMap ManglingNumbers; + ItaniumMangleContext *Mangler; + llvm::StringMap LambdaManglingNumbers; + unsigned BlockManglingNumber = 0; llvm::DenseMap VarManglingNumbers; llvm::DenseMap TagManglingNumbers; llvm::DenseMap DecompsitionDeclManglingNumbers; public: + ItaniumNumberingContext(ItaniumMangleContext *Mangler) : Mangler(Mangler) {} + unsigned getManglingNumber(const CXXMethodDecl *CallOperator) override { - const FunctionProtoType *Proto = - CallOperator->getType()->getAs(); - ASTContext &Context = CallOperator->getASTContext(); + const CXXRecordDecl *Lambda = CallOperator->getParent(); + assert(Lambda->isLambda()); + + // Computation of the is non-trivial and subtle. Rather than + // duplicating it here, just mangle the directly. + llvm::SmallString<128> LambdaSig; + llvm::raw_svector_ostream Out(LambdaSig); + Mangler->mangleLambdaSig(Lambda, Out); - FunctionProtoType::ExtProtoInfo EPI; - EPI.Variadic = Proto->isVariadic(); - QualType Key = - Context.getFunctionType(Context.VoidTy, Proto->getParamTypes(), EPI); - Key = Context.getCanonicalType(Key); - return ++ManglingNumbers[Key->castAs()]; + return ++LambdaManglingNumbers[LambdaSig]; } unsigned getManglingNumber(const BlockDecl *BD) override { - const Type *Ty = nullptr; - return ++ManglingNumbers[Ty]; + return ++BlockManglingNumber; } unsigned getStaticLocalNumber(const VarDecl *VD) override { @@ -154,10 +182,13 @@ public: }; class ItaniumCXXABI : public CXXABI { +private: + std::unique_ptr Mangler; protected: ASTContext &Context; public: - ItaniumCXXABI(ASTContext &Ctx) : Context(Ctx) { } + ItaniumCXXABI(ASTContext &Ctx) + : Mangler(Ctx.createMangleContext()), Context(Ctx) {} MemberPointerInfo getMemberPointerInfo(const MemberPointerType *MPT) const override { @@ -218,7 +249,8 @@ public: std::unique_ptr createMangleNumberingContext() const override { - return std::make_unique(); + return std::make_unique( + cast(Mangler.get())); } }; } diff --git a/lib/AST/ItaniumMangle.cpp b/lib/AST/ItaniumMangle.cpp index 08fd0adf13..76781c24ba 100644 --- a/lib/AST/ItaniumMangle.cpp +++ b/lib/AST/ItaniumMangle.cpp @@ -170,6 +170,8 @@ public: void mangleStringLiteral(const StringLiteral *, raw_ostream &) override; + void mangleLambdaSig(const CXXRecordDecl *Lambda, raw_ostream &) override; + bool getNextDiscriminator(const NamedDecl *ND, unsigned &disc) { // Lambda closure types are already numbered. if (isLambda(ND)) @@ -424,6 +426,7 @@ public: void mangleName(const NamedDecl *ND); void mangleType(QualType T); void mangleNameOrStandardSubstitution(const NamedDecl *ND); + void mangleLambdaSig(const CXXRecordDecl *Lambda); private: @@ -550,7 +553,7 @@ private: void mangleTemplateArgs(const TemplateArgumentList &AL); void mangleTemplateArg(TemplateArgument A); - void mangleTemplateParameter(unsigned Index); + void mangleTemplateParameter(unsigned Depth, unsigned Index); void mangleFunctionParam(const ParmVarDecl *parm); @@ -965,7 +968,7 @@ void CXXNameMangler::mangleUnscopedTemplateName( if (const auto *TTP = dyn_cast(ND)) { assert(!AdditionalAbiTags && "template template param cannot have abi tags"); - mangleTemplateParameter(TTP->getIndex()); + mangleTemplateParameter(TTP->getDepth(), TTP->getIndex()); } else if (isa(ND)) { mangleUnscopedName(ND, AdditionalAbiTags); } else { @@ -1686,16 +1689,42 @@ void CXXNameMangler::mangleUnqualifiedBlock(const BlockDecl *Block) { // ::= Tn # template non-type parameter // ::= Tt * E # template template parameter void CXXNameMangler::mangleTemplateParamDecl(const NamedDecl *Decl) { - if (isa(Decl)) { + if (auto *Ty = dyn_cast(Decl)) { + if (Ty->isParameterPack()) + Out << "Tp"; Out << "Ty"; } else if (auto *Tn = dyn_cast(Decl)) { - Out << "Tn"; - mangleType(Tn->getType()); + if (Tn->isExpandedParameterPack()) { + for (unsigned I = 0, N = Tn->getNumExpansionTypes(); I != N; ++I) { + Out << "Tn"; + mangleType(Tn->getExpansionType(I)); + } + } else { + QualType T = Tn->getType(); + if (Tn->isParameterPack()) { + Out << "Tp"; + T = T->castAs()->getPattern(); + } + Out << "Tn"; + mangleType(T); + } } else if (auto *Tt = dyn_cast(Decl)) { - Out << "Tt"; - for (auto *Param : *Tt->getTemplateParameters()) - mangleTemplateParamDecl(Param); - Out << "E"; + if (Tt->isExpandedParameterPack()) { + for (unsigned I = 0, N = Tt->getNumExpansionTemplateParameters(); I != N; + ++I) { + Out << "Tt"; + for (auto *Param : *Tt->getExpansionTemplateParameters(I)) + mangleTemplateParamDecl(Param); + Out << "E"; + } + } else { + if (Tt->isParameterPack()) + Out << "Tp"; + Out << "Tt"; + for (auto *Param : *Tt->getTemplateParameters()) + mangleTemplateParamDecl(Param); + Out << "E"; + } } } @@ -1726,12 +1755,7 @@ void CXXNameMangler::mangleLambda(const CXXRecordDecl *Lambda) { } Out << "Ul"; - for (auto *D : Lambda->getLambdaExplicitTemplateParameters()) - mangleTemplateParamDecl(D); - const FunctionProtoType *Proto = Lambda->getLambdaTypeInfo()->getType()-> - getAs(); - mangleBareFunctionType(Proto, /*MangleReturnType=*/false, - Lambda->getLambdaStaticInvoker()); + mangleLambdaSig(Lambda); Out << "E"; // The number is omitted for the first closure type with a given @@ -1746,6 +1770,15 @@ void CXXNameMangler::mangleLambda(const CXXRecordDecl *Lambda) { Out << '_'; } +void CXXNameMangler::mangleLambdaSig(const CXXRecordDecl *Lambda) { + for (auto *D : Lambda->getLambdaExplicitTemplateParameters()) + mangleTemplateParamDecl(D); + const FunctionProtoType *Proto = Lambda->getLambdaTypeInfo()->getType()-> + getAs(); + mangleBareFunctionType(Proto, /*MangleReturnType=*/false, + Lambda->getLambdaStaticInvoker()); +} + void CXXNameMangler::manglePrefix(NestedNameSpecifier *qualifier) { switch (qualifier->getKind()) { case NestedNameSpecifier::Global: @@ -1852,7 +1885,7 @@ void CXXNameMangler::mangleTemplatePrefix(const TemplateDecl *ND, // ::= if (const auto *TTP = dyn_cast(ND)) { - mangleTemplateParameter(TTP->getIndex()); + mangleTemplateParameter(TTP->getDepth(), TTP->getIndex()); } else { manglePrefix(getEffectiveDeclContext(ND), NoFunction); if (isa(ND)) @@ -1885,8 +1918,8 @@ void CXXNameMangler::mangleType(TemplateName TN) { goto HaveDecl; HaveDecl: - if (isa(TD)) - mangleTemplateParameter(cast(TD)->getIndex()); + if (auto *TTP = dyn_cast(TD)) + mangleTemplateParameter(TTP->getDepth(), TTP->getIndex()); else mangleName(TD); break; @@ -2964,7 +2997,7 @@ void CXXNameMangler::mangleType(const MemberPointerType *T) { // ::= void CXXNameMangler::mangleType(const TemplateTypeParmType *T) { - mangleTemplateParameter(T->getIndex()); + mangleTemplateParameter(T->getDepth(), T->getIndex()); } // ::= @@ -3535,7 +3568,7 @@ void CXXNameMangler::mangleDeclRefExpr(const NamedDecl *D) { case Decl::NonTypeTemplateParm: const NonTypeTemplateParmDecl *PD = cast(D); - mangleTemplateParameter(PD->getIndex()); + mangleTemplateParameter(PD->getDepth(), PD->getIndex()); break; } } @@ -4264,13 +4297,13 @@ recurse: Out << "sZ"; const NamedDecl *Pack = SPE->getPack(); if (const TemplateTypeParmDecl *TTP = dyn_cast(Pack)) - mangleTemplateParameter(TTP->getIndex()); + mangleTemplateParameter(TTP->getDepth(), TTP->getIndex()); else if (const NonTypeTemplateParmDecl *NTTP = dyn_cast(Pack)) - mangleTemplateParameter(NTTP->getIndex()); + mangleTemplateParameter(NTTP->getDepth(), NTTP->getIndex()); else if (const TemplateTemplateParmDecl *TempTP = dyn_cast(Pack)) - mangleTemplateParameter(TempTP->getIndex()); + mangleTemplateParameter(TempTP->getDepth(), TempTP->getIndex()); else mangleFunctionParam(cast(Pack)); break; @@ -4557,13 +4590,21 @@ void CXXNameMangler::mangleTemplateArg(TemplateArgument A) { } } -void CXXNameMangler::mangleTemplateParameter(unsigned Index) { +void CXXNameMangler::mangleTemplateParameter(unsigned Depth, unsigned Index) { // ::= T_ # first template parameter // ::= T _ - if (Index == 0) - Out << "T_"; - else - Out << 'T' << (Index - 1) << '_'; + // ::= TL __ + // ::= TL _ + // _ + // + // The latter two manglings are from a proposal here: + // https://github.com/itanium-cxx-abi/cxx-abi/issues/31#issuecomment-528122117 + Out << 'T'; + if (Depth != 0) + Out << 'L' << (Depth - 1) << '_'; + if (Index != 0) + Out << (Index - 1); + Out << '_'; } void CXXNameMangler::mangleSeqID(unsigned SeqID) { @@ -5080,6 +5121,12 @@ void ItaniumMangleContextImpl::mangleStringLiteral(const StringLiteral *, raw_os llvm_unreachable("Can't mangle string literals"); } +void ItaniumMangleContextImpl::mangleLambdaSig(const CXXRecordDecl *Lambda, + raw_ostream &Out) { + CXXNameMangler Mangler(*this, Out); + Mangler.mangleLambdaSig(Lambda); +} + ItaniumMangleContext * ItaniumMangleContext::create(ASTContext &Context, DiagnosticsEngine &Diags) { return new ItaniumMangleContextImpl(Context, Diags); diff --git a/lib/Sema/SemaLambda.cpp b/lib/Sema/SemaLambda.cpp index 703d3f212c..a76a840f26 100644 --- a/lib/Sema/SemaLambda.cpp +++ b/lib/Sema/SemaLambda.cpp @@ -407,6 +407,8 @@ CXXMethodDecl *Sema::startLambdaDefinition( MethodType, MethodTypeInfo, SC_None, /*isInline=*/true, ConstexprKind, EndLoc); Method->setAccess(AS_public); + if (!TemplateParams) + Class->addDecl(Method); // Temporarily set the lexical declaration context to the current // context, so that the Scope stack matches the lexical nesting. @@ -418,9 +420,10 @@ CXXMethodDecl *Sema::startLambdaDefinition( TemplateParams, Method) : nullptr; if (TemplateMethod) { - TemplateMethod->setLexicalDeclContext(CurContext); TemplateMethod->setAccess(AS_public); Method->setDescribedFunctionTemplate(TemplateMethod); + Class->addDecl(TemplateMethod); + TemplateMethod->setLexicalDeclContext(CurContext); } // Add parameters. @@ -1641,8 +1644,9 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc, ? CallOperator->getDescribedFunctionTemplate() : cast(CallOperator); + // FIXME: Is this really the best choice? Keeping the lexical decl context + // set as CurContext seems more faithful to the source. TemplateOrNonTemplateCallOperatorDecl->setLexicalDeclContext(Class); - Class->addDecl(TemplateOrNonTemplateCallOperatorDecl); PopExpressionEvaluationContext(); diff --git a/test/CodeGenCXX/mangle-lambda-explicit-template-params.cpp b/test/CodeGenCXX/mangle-lambda-explicit-template-params.cpp index b88b0d56fd..dba10fbf01 100644 --- a/test/CodeGenCXX/mangle-lambda-explicit-template-params.cpp +++ b/test/CodeGenCXX/mangle-lambda-explicit-template-params.cpp @@ -33,9 +33,68 @@ void call_inline_func() { inline_func(); } +template struct X {}; + +inline auto pack = [](T (&...)[N]) {}; +int arr1[] = {1}; +int arr2[] = {1, 2}; +// CHECK: @_ZNK4packMUlTpTyTpTnT_DpRAT0__S_E_clIJiiEJLi1ELi2EEEEDaS2_( +void use_pack() { pack(arr1, arr2); } + +inline void collision() { + auto a = [] typename>{}; + auto b = [] typename>{}; + auto c = [] typename>{}; + a.operator()(); + // CHECK: @_ZZ9collisionvENKUlTyTtTyTnT_EvE_clIi1XEEDav + b.operator()(); + // CHECK: @_ZZ9collisionvENKUlTyTtTyTnTL0__EvE_clIi1XEEDav + c.operator()(); + // CHECK: @_ZZ9collisionvENKUlTyTtTyTnT_EvE0_clIi1XEEDav +} +void use_collision() { collision(); } + template void f() { // CHECK: define linkonce_odr {{.*}} @_ZZ1fIiEvvENKUlT_E_clIiEEDaS0_( auto x = [](auto){}; x(0); } void use_f() { f(); } + +template struct Y { + template struct Z {}; +}; + +template void expanded() { + auto x = [] typename...>{}; + auto y = [] typename>{}; + auto z = [] typename, template typename>{}; + // FIXME: Should we really require 'template' for y and z? + x.template operator()<(T())..., Y::template Z...>(); + y.template operator()<0, Y::Z>(); + y.template operator()<1, Y::Z>(); + z.template operator()<1, 2, Y::Z, Y::Z>(); +} +void use_expanded() { + // CHECK: @_ZZ8expandedIJEEvvENKUlvE_clIJEJEEEDav( + // CHECK: @_ZZ8expandedIJEEvvENKUlTniTtTniEvE_clILi0EN1YIiE1ZEEEDav( + // CHECK: @_ZZ8expandedIJEEvvENKUlTniTtTniEvE_clILi1EN1YIiE1ZEEEDav( + // CHECK: @_ZZ8expandedIJEEvvENKUlTniTniTtTniETtTniEvE_clILi1ELi2EN1YIiE1ZENS2_IfE1ZEEEDav( + expanded<>(); + + // FIXME: Should we really be using J...E for arguments corresponding to an + // expanded parameter pack? + // Note that the s of 'x' and 'y' collide here, after pack expansion. + // CHECK: @_ZZ8expandedIJiEEvvENKUlTniTtTniEvE_clIJLi0EEJN1YIiE1ZEEEEDav( + // CHECK: @_ZZ8expandedIJiEEvvENKUlTniTtTniEvE0_clILi0EN1YIiE1ZEEEDav( + // CHECK: @_ZZ8expandedIJiEEvvENKUlTniTtTniEvE0_clILi1EN1YIiE1ZEEEDav( + // CHECK: @_ZZ8expandedIJiEEvvENKUlTniTniTtTniETtTniEvE_clILi1ELi2EN1YIiE1ZENS2_IfE1ZEEEDav( + expanded(); + + // Note that the s of 'x' and 'z' collide here, after pack expansion. + // CHECK: @_ZZ8expandedIJiiEEvvENKUlTniTniTtTniETtTniEvE_clIJLi0ELi0EEJN1YIiE1ZES4_EEEDav( + // CHECK: @_ZZ8expandedIJiiEEvvENKUlTniTtTniEvE_clILi0EN1YIiE1ZEEEDav( + // CHECK: @_ZZ8expandedIJiiEEvvENKUlTniTtTniEvE_clILi1EN1YIiE1ZEEEDav( + // CHECK: @_ZZ8expandedIJiiEEvvENKUlTniTniTtTniETtTniEvE0_clILi1ELi2EN1YIiE1ZENS2_IfE1ZEEEDav( + expanded(); +}