From 7ac42374abc01d89235c862cc44da0c6c930489c Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Thu, 5 Sep 2019 01:23:47 +0000 Subject: [PATCH] [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.) llvm-svn: 371004 --- clang/include/clang/AST/Mangle.h | 2 + clang/lib/AST/DeclBase.cpp | 7 ++ clang/lib/AST/ItaniumCXXABI.cpp | 70 ++++++++---- clang/lib/AST/ItaniumMangle.cpp | 103 +++++++++++++----- clang/lib/Sema/SemaLambda.cpp | 8 +- ...mangle-lambda-explicit-template-params.cpp | 59 ++++++++++ 6 files changed, 200 insertions(+), 49 deletions(-) diff --git a/clang/include/clang/AST/Mangle.h b/clang/include/clang/AST/Mangle.h index b1fbe936136a..1362756f55d2 100644 --- a/clang/include/clang/AST/Mangle.h +++ b/clang/include/clang/AST/Mangle.h @@ -170,6 +170,8 @@ class ItaniumMangleContext : public MangleContext { 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/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index fd80e1532eb5..aa2c21230c89 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/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/clang/lib/AST/ItaniumCXXABI.cpp b/clang/lib/AST/ItaniumCXXABI.cpp index 67f874b7b9f2..069add8464ae 100644 --- a/clang/lib/AST/ItaniumCXXABI.cpp +++ b/clang/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 @@ class ItaniumNumberingContext : public MangleNumberingContext { }; 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 @@ class ItaniumCXXABI : public CXXABI { std::unique_ptr createMangleNumberingContext() const override { - return std::make_unique(); + return std::make_unique( + cast(Mangler.get())); } }; } diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp index 08fd0adf1362..76781c24bab8 100644 --- a/clang/lib/AST/ItaniumMangle.cpp +++ b/clang/lib/AST/ItaniumMangle.cpp @@ -170,6 +170,8 @@ class ItaniumMangleContextImpl : public ItaniumMangleContext { 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 @@ class CXXNameMangler { void mangleName(const NamedDecl *ND); void mangleType(QualType T); void mangleNameOrStandardSubstitution(const NamedDecl *ND); + void mangleLambdaSig(const CXXRecordDecl *Lambda); private: @@ -550,7 +553,7 @@ class CXXNameMangler { 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 @@ void CXXNameMangler::mangleExpression(const Expr *E, unsigned Arity) { 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/clang/lib/Sema/SemaLambda.cpp b/clang/lib/Sema/SemaLambda.cpp index 703d3f212c33..a76a840f26ee 100644 --- a/clang/lib/Sema/SemaLambda.cpp +++ b/clang/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/clang/test/CodeGenCXX/mangle-lambda-explicit-template-params.cpp b/clang/test/CodeGenCXX/mangle-lambda-explicit-template-params.cpp index b88b0d56fd99..dba10fbf01db 100644 --- a/clang/test/CodeGenCXX/mangle-lambda-explicit-template-params.cpp +++ b/clang/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(); +}