diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index c52e285bde627..938d9578bae61 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -667,6 +667,7 @@ Bug Fixes to C++ Support whose type depends on itself. (#GH51347), (#GH55872) - Improved parser recovery of invalid requirement expressions. In turn, this fixes crashes from follow-on processing of the invalid requirement. (#GH138820) +- Fixed the handling of pack indexing types in the constraints of a member function redeclaration. (#GH138255) Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index e107db458742e..1fdc488a76507 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -221,7 +221,8 @@ class ASTContext : public RefCountedBase { mutable llvm::ContextualFoldingSet DependentDecltypeTypes; - mutable llvm::FoldingSet DependentPackIndexingTypes; + mutable llvm::ContextualFoldingSet + DependentPackIndexingTypes; mutable llvm::FoldingSet TemplateTypeParmTypes; mutable llvm::FoldingSet ObjCTypeParamTypes; diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h index 8710f252a0c5c..e4b2e3486ec27 100644 --- a/clang/include/clang/AST/ExprCXX.h +++ b/clang/include/clang/AST/ExprCXX.h @@ -4549,6 +4549,10 @@ class PackIndexingExpr final bool isFullySubstituted() const { return FullySubstituted; } + bool isPartiallySubstituted() const { + return isValueDependent() && TransformedExpressions; + }; + /// Determine if the expression was expanded to empty. bool expandsToEmptyPack() const { return isFullySubstituted() && TransformedExpressions == 0; diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index 773796a55eaa1..ba723e7e5b5b1 100644 --- a/clang/include/clang/AST/Type.h +++ b/clang/include/clang/AST/Type.h @@ -5976,7 +5976,6 @@ class PackIndexingType final private llvm::TrailingObjects { friend TrailingObjects; - const ASTContext &Context; QualType Pattern; Expr *IndexExpr; @@ -5987,9 +5986,8 @@ class PackIndexingType final protected: friend class ASTContext; // ASTContext creates these. - PackIndexingType(const ASTContext &Context, QualType Canonical, - QualType Pattern, Expr *IndexExpr, bool FullySubstituted, - ArrayRef Expansions = {}); + PackIndexingType(QualType Canonical, QualType Pattern, Expr *IndexExpr, + bool FullySubstituted, ArrayRef Expansions = {}); public: Expr *getIndexExpr() const { return IndexExpr; } @@ -6024,12 +6022,7 @@ class PackIndexingType final return T->getTypeClass() == PackIndexing; } - void Profile(llvm::FoldingSetNodeID &ID) { - if (hasSelectedType()) - getSelectedType().Profile(ID); - else - Profile(ID, Context, getPattern(), getIndexExpr(), isFullySubstituted()); - } + void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Context); static void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Context, QualType Pattern, Expr *E, bool FullySubstituted); diff --git a/clang/include/clang/Sema/Template.h b/clang/include/clang/Sema/Template.h index f9a10cfafb1f7..4fb3b5df6eed0 100644 --- a/clang/include/clang/Sema/Template.h +++ b/clang/include/clang/Sema/Template.h @@ -24,6 +24,7 @@ #include "llvm/ADT/SmallVector.h" #include #include +#include #include namespace clang { @@ -76,9 +77,17 @@ enum class TemplateSubstitutionKind : char { class MultiLevelTemplateArgumentList { /// The template argument list at a certain template depth + enum ListProperties { + /// A 'Final' substitution. + IsFinal = 0x1, + /// Track if the arguments are injected template parameters. + /// Injected template parameters should not be expanded. + ArgumentsAreInjectedTemplateParams = 0x02, + }; + using ArgList = ArrayRef; struct ArgumentListLevel { - llvm::PointerIntPair AssociatedDeclAndFinal; + llvm::PointerIntPair AssociatedDeclAndProperties; ArgList Args; }; using ContainerType = SmallVector; @@ -161,11 +170,19 @@ enum class TemplateSubstitutionKind : char { /// A template-like entity which owns the whole pattern being substituted. /// This will usually own a set of template parameters, or in some /// cases might even be a template parameter itself. - std::pair getAssociatedDecl(unsigned Depth) const { + std::tuple getAssociatedDecl(unsigned Depth) const { + assert(NumRetainedOuterLevels <= Depth && Depth < getNumLevels()); + auto AD = TemplateArgumentLists[getNumLevels() - Depth - 1] + .AssociatedDeclAndProperties; + return {AD.getPointer(), AD.getInt() & IsFinal, + AD.getInt() & ArgumentsAreInjectedTemplateParams}; + } + + bool ArgumentsAreInjectedParameters(unsigned Depth) const { assert(NumRetainedOuterLevels <= Depth && Depth < getNumLevels()); auto AD = TemplateArgumentLists[getNumLevels() - Depth - 1] - .AssociatedDeclAndFinal; - return {AD.getPointer(), AD.getInt()}; + .AssociatedDeclAndProperties; + return AD.getInt() & ArgumentsAreInjectedTemplateParams; } /// Determine whether there is a non-NULL template argument at the @@ -205,16 +222,15 @@ enum class TemplateSubstitutionKind : char { /// Add a new outmost level to the multi-level template argument /// list. - /// A 'Final' substitution means that Subst* nodes won't be built - /// for the replacements. - void addOuterTemplateArguments(Decl *AssociatedDecl, ArgList Args, - bool Final) { + void + addOuterTemplateArguments(Decl *AssociatedDecl, ArgList Args, bool Final, + bool ArgumentsAreInjectedTemplateParams = false) { assert(!NumRetainedOuterLevels && "substituted args outside retained args?"); assert(getKind() == TemplateSubstitutionKind::Specialization); TemplateArgumentLists.push_back( {{AssociatedDecl ? AssociatedDecl->getCanonicalDecl() : nullptr, - Final}, + getProperties(Final, ArgumentsAreInjectedTemplateParams)}, Args}); } @@ -239,15 +255,17 @@ enum class TemplateSubstitutionKind : char { "Replacing in an empty list?"); if (!TemplateArgumentLists.empty()) { - assert((TemplateArgumentLists[0].AssociatedDeclAndFinal.getPointer() || - TemplateArgumentLists[0].AssociatedDeclAndFinal.getPointer() == + assert((TemplateArgumentLists[0] + .AssociatedDeclAndProperties.getPointer() || + TemplateArgumentLists[0] + .AssociatedDeclAndProperties.getPointer() == AssociatedDecl) && "Trying to change incorrect declaration?"); TemplateArgumentLists[0].Args = Args; } else { --NumRetainedOuterLevels; TemplateArgumentLists.push_back( - {{AssociatedDecl, /*Final=*/false}, Args}); + {{AssociatedDecl, /*Properties=*/0}, Args}); } } @@ -292,6 +310,16 @@ enum class TemplateSubstitutionKind : char { llvm::errs() << "\n"; } } + + static unsigned getProperties(bool IsFinal, bool IsInjected) { + unsigned Props = 0; + if (IsFinal) + Props |= MultiLevelTemplateArgumentList::IsFinal; + if (IsInjected) + Props |= + MultiLevelTemplateArgumentList::ArgumentsAreInjectedTemplateParams; + return Props; + } }; /// The context in which partial ordering of function templates occurs. diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 1ed16748dff1a..6ebef70b2d44a 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -940,7 +940,7 @@ ASTContext::ASTContext(LangOptions &LOpts, SourceManager &SM, DependentSizedMatrixTypes(this_()), FunctionProtoTypes(this_(), FunctionProtoTypesLog2InitSize), DependentTypeOfExprTypes(this_()), DependentDecltypeTypes(this_()), - TemplateSpecializationTypes(this_()), + DependentPackIndexingTypes(this_()), TemplateSpecializationTypes(this_()), DependentTemplateSpecializationTypes(this_()), DependentBitIntTypes(this_()), SubstTemplateTemplateParmPacks(this_()), DeducedTemplates(this_()), ArrayParameterTypes(this_()), @@ -6433,34 +6433,29 @@ QualType ASTContext::getPackIndexingType(QualType Pattern, Expr *IndexExpr, ArrayRef Expansions, UnsignedOrNone Index) const { QualType Canonical; - if (FullySubstituted && Index) { + if (FullySubstituted && Index) Canonical = getCanonicalType(Expansions[*Index]); - } else { - llvm::FoldingSetNodeID ID; - PackIndexingType::Profile(ID, *this, Pattern.getCanonicalType(), IndexExpr, - FullySubstituted); - void *InsertPos = nullptr; - PackIndexingType *Canon = - DependentPackIndexingTypes.FindNodeOrInsertPos(ID, InsertPos); - if (!Canon) { - void *Mem = Allocate( - PackIndexingType::totalSizeToAlloc(Expansions.size()), - TypeAlignment); - Canon = new (Mem) - PackIndexingType(*this, QualType(), Pattern.getCanonicalType(), - IndexExpr, FullySubstituted, Expansions); - DependentPackIndexingTypes.InsertNode(Canon, InsertPos); - } - Canonical = QualType(Canon, 0); - } + else if (!Pattern.isCanonical()) + Canonical = getPackIndexingType(Pattern.getCanonicalType(), IndexExpr, + FullySubstituted, Expansions, Index); + + llvm::FoldingSetNodeID ID; + PackIndexingType::Profile(ID, *this, Pattern, IndexExpr, FullySubstituted); + void *InsertPos = nullptr; + PackIndexingType *Canon = + DependentPackIndexingTypes.FindNodeOrInsertPos(ID, InsertPos); + if (Canon) + return QualType(Canon, 0); void *Mem = Allocate(PackIndexingType::totalSizeToAlloc(Expansions.size()), TypeAlignment); - auto *T = new (Mem) PackIndexingType(*this, Canonical, Pattern, IndexExpr, - FullySubstituted, Expansions); - Types.push_back(T); - return QualType(T, 0); + + Canon = new (Mem) PackIndexingType(Canonical, Pattern, IndexExpr, + FullySubstituted, Expansions); + DependentPackIndexingTypes.InsertNode(Canon, InsertPos); + Types.push_back(Canon); + return QualType(Canon, 0); } /// getUnaryTransformationType - We don't unique these, since the memory diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp index 83d54da9be7e5..2483ca36b77ea 100644 --- a/clang/lib/AST/StmtProfile.cpp +++ b/clang/lib/AST/StmtProfile.cpp @@ -2291,9 +2291,15 @@ void StmtProfiler::VisitSizeOfPackExpr(const SizeOfPackExpr *S) { } void StmtProfiler::VisitPackIndexingExpr(const PackIndexingExpr *E) { - VisitExpr(E); - VisitExpr(E->getPackIdExpression()); VisitExpr(E->getIndexExpr()); + + if (E->isPartiallySubstituted()) { + ID.AddInteger(E->getExpressions().size()); + for (const Expr *Sub : E->getExpressions()) + Visit(Sub); + } else { + VisitExpr(E->getPackIdExpression()); + } } void StmtProfiler::VisitSubstNonTypeTemplateParmPackExpr( diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp index 31e4bcd7535ea..bb4fe24cf94d1 100644 --- a/clang/lib/AST/Type.cpp +++ b/clang/lib/AST/Type.cpp @@ -4126,14 +4126,14 @@ void DependentDecltypeType::Profile(llvm::FoldingSetNodeID &ID, E->Profile(ID, Context, true); } -PackIndexingType::PackIndexingType(const ASTContext &Context, - QualType Canonical, QualType Pattern, +PackIndexingType::PackIndexingType(QualType Canonical, QualType Pattern, Expr *IndexExpr, bool FullySubstituted, ArrayRef Expansions) : Type(PackIndexing, Canonical, computeDependence(Pattern, IndexExpr, Expansions)), - Context(Context), Pattern(Pattern), IndexExpr(IndexExpr), - Size(Expansions.size()), FullySubstituted(FullySubstituted) { + Pattern(Pattern), IndexExpr(IndexExpr), Size(Expansions.size()), + FullySubstituted(FullySubstituted) { + llvm::uninitialized_copy(Expansions, getTrailingObjects()); } @@ -4174,6 +4174,14 @@ PackIndexingType::computeDependence(QualType Pattern, Expr *IndexExpr, return TD; } +void PackIndexingType::Profile(llvm::FoldingSetNodeID &ID, + const ASTContext &Context) { + if (hasSelectedType() && isFullySubstituted()) + getSelectedType().Profile(ID); + else + Profile(ID, Context, getPattern(), getIndexExpr(), isFullySubstituted()); +} + void PackIndexingType::Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Context, QualType Pattern, Expr *E, bool FullySubstituted) { diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index fb490bcac6e91..3b99c490eb664 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -338,7 +338,7 @@ Response HandleFunctionTemplateDecl(Sema &SemaRef, const_cast(FTD), const_cast(FTD)->getInjectedTemplateArgs( SemaRef.Context), - /*Final=*/false); + /*Final=*/false, /*InjectedTemplateParams=*/true); NestedNameSpecifier *NNS = FTD->getTemplatedDecl()->getQualifier(); @@ -363,18 +363,20 @@ Response HandleFunctionTemplateDecl(Sema &SemaRef, // This meets the contract in // TreeTransform::TryExpandParameterPacks that the template arguments // for unexpanded parameters should be of a Pack kind. + bool Injected = false; if (TSTy->isCurrentInstantiation()) { auto *RD = TSTy->getCanonicalTypeInternal()->getAsCXXRecordDecl(); - if (ClassTemplateDecl *CTD = RD->getDescribedClassTemplate()) + if (ClassTemplateDecl *CTD = RD->getDescribedClassTemplate()) { Arguments = CTD->getInjectedTemplateArgs(SemaRef.Context); - else if (auto *Specialization = - dyn_cast(RD)) + Injected = true; + } else if (auto *Specialization = + dyn_cast(RD)) Arguments = Specialization->getTemplateInstantiationArgs().asArray(); } Result.addOuterTemplateArguments( TSTy->getTemplateName().getAsTemplateDecl(), Arguments, - /*Final=*/false); + /*Final=*/false, /*InjectedTemplateParams=*/Injected); } } @@ -399,7 +401,7 @@ Response HandleRecordDecl(Sema &SemaRef, const CXXRecordDecl *Rec, Result.addOuterTemplateArguments( const_cast(Rec), ClassTemplate->getInjectedTemplateArgs(SemaRef.Context), - /*Final=*/false); + /*Final=*/false, /*InjectedTemplateParams=*/true); } if (const MemberSpecializationInfo *MSInfo = @@ -1476,8 +1478,8 @@ namespace { } } - TemplateArgument - getTemplateArgumentPackPatternForRewrite(const TemplateArgument &TA) { + TemplateArgument getTemplateArgumentOrUnsubstitutedExpansionPattern( + const TemplateArgument &TA) { if (TA.getKind() != TemplateArgument::Pack) return TA; if (SemaRef.ArgPackSubstIndex) @@ -1902,7 +1904,9 @@ Decl *TemplateInstantiator::TransformDecl(SourceLocation Loc, Decl *D) { TemplateArgument Arg = TemplateArgs(TTP->getDepth(), TTP->getPosition()); - if (TTP->isParameterPack()) { + if (TemplateArgs.ArgumentsAreInjectedParameters(TTP->getDepth())) { + Arg = getTemplateArgumentOrUnsubstitutedExpansionPattern(Arg); + } else if (TTP->isParameterPack()) { assert(Arg.getKind() == TemplateArgument::Pack && "Missing argument pack"); Arg = getPackSubstitutedTemplateArgument(getSema(), Arg); @@ -2054,20 +2058,23 @@ TemplateName TemplateInstantiator::TransformTemplateName( if (TemplateArgs.isRewrite()) { // We're rewriting the template parameter as a reference to another // template parameter. - Arg = getTemplateArgumentPackPatternForRewrite(Arg); + Arg = getTemplateArgumentOrUnsubstitutedExpansionPattern(Arg); assert(Arg.getKind() == TemplateArgument::Template && "unexpected nontype template argument kind in template rewrite"); return Arg.getAsTemplate(); } - auto [AssociatedDecl, Final] = + auto [AssociatedDecl, Final, ArgumentsAreInjectedTemplateParams] = TemplateArgs.getAssociatedDecl(TTP->getDepth()); UnsignedOrNone PackIndex = std::nullopt; if (TTP->isParameterPack()) { assert(Arg.getKind() == TemplateArgument::Pack && "Missing argument pack"); - if (!getSema().ArgPackSubstIndex) { + if (ArgumentsAreInjectedTemplateParams) + Arg = getTemplateArgumentOrUnsubstitutedExpansionPattern(Arg); + + else if (!getSema().ArgPackSubstIndex) { // We have the template argument pack to substitute, but we're not // actually expanding the enclosing pack expansion yet. So, just // keep the entire argument pack. @@ -2131,7 +2138,7 @@ TemplateInstantiator::TransformTemplateParmRefExpr(DeclRefExpr *E, if (TemplateArgs.isRewrite()) { // We're rewriting the template parameter as a reference to another // template parameter. - Arg = getTemplateArgumentPackPatternForRewrite(Arg); + Arg = getTemplateArgumentOrUnsubstitutedExpansionPattern(Arg); assert(Arg.getKind() == TemplateArgument::Expression && "unexpected nontype template argument kind in template rewrite"); // FIXME: This can lead to the same subexpression appearing multiple times @@ -2139,7 +2146,7 @@ TemplateInstantiator::TransformTemplateParmRefExpr(DeclRefExpr *E, return Arg.getAsExpr(); } - auto [AssociatedDecl, Final] = + auto [AssociatedDecl, Final, ArgumentsAreInjectedTemplateParams] = TemplateArgs.getAssociatedDecl(NTTP->getDepth()); UnsignedOrNone PackIndex = std::nullopt; if (NTTP->isParameterPack()) { @@ -2584,7 +2591,7 @@ TemplateInstantiator::TransformTemplateTypeParmType(TypeLocBuilder &TLB, if (TemplateArgs.isRewrite()) { // We're rewriting the template parameter as a reference to another // template parameter. - Arg = getTemplateArgumentPackPatternForRewrite(Arg); + Arg = getTemplateArgumentOrUnsubstitutedExpansionPattern(Arg); assert(Arg.getKind() == TemplateArgument::Type && "unexpected nontype template argument kind in template rewrite"); QualType NewT = Arg.getAsType(); @@ -2592,7 +2599,7 @@ TemplateInstantiator::TransformTemplateTypeParmType(TypeLocBuilder &TLB, return NewT; } - auto [AssociatedDecl, Final] = + auto [AssociatedDecl, Final, ArgumentsAreInjectedTemplateParams] = TemplateArgs.getAssociatedDecl(T->getDepth()); UnsignedOrNone PackIndex = std::nullopt; if (T->isParameterPack()) { @@ -2600,6 +2607,14 @@ TemplateInstantiator::TransformTemplateTypeParmType(TypeLocBuilder &TLB, "Missing argument pack"); if (!getSema().ArgPackSubstIndex) { + + if (ArgumentsAreInjectedTemplateParams) { + TemplateTypeParmTypeLoc NewTL = + TLB.push(TL.getType()); + NewTL.setNameLoc(TL.getNameLoc()); + return TL.getType(); + } + // We have the template argument pack, but we're not expanding the // enclosing pack expansion yet. Just save the template argument // pack for later substitution. diff --git a/clang/lib/Sema/SemaTemplateVariadic.cpp b/clang/lib/Sema/SemaTemplateVariadic.cpp index 5f0e968ff18c4..ff2f6f94d1b54 100644 --- a/clang/lib/Sema/SemaTemplateVariadic.cpp +++ b/clang/lib/Sema/SemaTemplateVariadic.cpp @@ -823,6 +823,13 @@ bool Sema::CheckParameterPacksForExpansion( continue; } + // Template arguments that are injected template parameters + // Cannot be expanded, even if they constitute a pack expansion. + if (TemplateArgs.ArgumentsAreInjectedParameters(Depth)) { + ShouldExpand = false; + continue; + } + // Determine the size of the argument pack. ArrayRef Pack = TemplateArgs(Depth, Index).getPackAsArray(); diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h index 8b4b79c6ec039..317e8351f1d50 100644 --- a/clang/lib/Sema/TreeTransform.h +++ b/clang/lib/Sema/TreeTransform.h @@ -6884,7 +6884,7 @@ TreeTransform::TransformPackIndexingType(TypeLocBuilder &TLB, assert(!Unexpanded.empty() && "Pack expansion without parameter packs?"); // Determine whether the set of unexpanded parameter packs can and should // be expanded. - bool ShouldExpand = true; + bool ShouldExpand = false; bool RetainExpansion = false; UnsignedOrNone NumExpansions = std::nullopt; if (getDerived().TryExpandParameterPacks(TL.getEllipsisLoc(), SourceRange(), diff --git a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp index 5af4ec75cae90..e5d00491d3fb8 100644 --- a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp +++ b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp @@ -1,4 +1,6 @@ -// RUN: %clang_cc1 -std=c++20 -verify %s +// RUN: %clang_cc1 -std=c++20 -Wno-c++26-extensions -verify %s +// RUN: %clang_cc1 -std=c++2c -Wno-c++26-extensions -verify %s + static constexpr int PRIMARY = 0; static constexpr int SPECIALIZATION_CONCEPT = 1; @@ -779,3 +781,75 @@ template consteval void S::mfn() requires (bool(&fn)) {} } + + +namespace GH138255 { + +template +concept C = true; + +struct Func { + template + requires C + static auto buggy() -> void; + + template + requires C + friend auto fr() -> void; + + template + requires C + friend auto fr2() -> void{}; // expected-note{{previous definition is here}} +}; + +template +requires C +auto Func::buggy() -> void {} + +template +requires C +auto fr() -> void {} + +template +requires C +auto fr2() -> void {} // expected-error{{redefinition of 'fr2'}} + + +template +requires C +struct Class; + +template +requires C +struct Class; + + +template +struct TplClass { + template + requires C + static auto buggy() -> void; +}; + +template<> +template +requires C +auto TplClass::buggy() -> void {} + +} + +namespace PackIndexExpr { +template +concept C = true; + +template struct TplClass { + template + requires C + static auto buggy() -> void; +}; + +template <> +template +requires C +auto TplClass::buggy() -> void {} +}