llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-modules Author: cor3ntin (cor3ntin) <details> <summary>Changes</summary> When profiling a pack indexing that has been partially substituted, we should profile the expansions, rather than the pattern itseld This is a better approach to #<!-- -->139057 This mirrors the fix done for SizeOfPackExpr in #<!-- -->124533 Fixes #<!-- -->138255 --- Full diff: https://github.com/llvm/llvm-project/pull/139276.diff 8 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+1) - (modified) clang/include/clang/AST/ASTContext.h (+2-1) - (modified) clang/include/clang/AST/ExprCXX.h (+4) - (modified) clang/include/clang/AST/Type.h (+5-11) - (modified) clang/lib/AST/ASTContext.cpp (+4-4) - (modified) clang/lib/AST/StmtProfile.cpp (+8-2) - (modified) clang/lib/AST/Type.cpp (+20-6) - (modified) clang/test/SemaTemplate/concepts-out-of-line-def.cpp (+75-1) ``````````diff 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<ASTContext> { mutable llvm::ContextualFoldingSet<DependentDecltypeType, ASTContext &> DependentDecltypeTypes; - mutable llvm::FoldingSet<PackIndexingType> DependentPackIndexingTypes; + mutable llvm::ContextualFoldingSet<PackIndexingType, ASTContext &> + DependentPackIndexingTypes; mutable llvm::FoldingSet<TemplateTypeParmType> TemplateTypeParmTypes; mutable llvm::FoldingSet<ObjCTypeParamType> 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..242d7b72bd5b8 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<PackIndexingType, QualType> { 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<QualType> Expansions = {}); + PackIndexingType(QualType Canonical, QualType Pattern, Expr *IndexExpr, + bool FullySubstituted, ArrayRef<QualType> Expansions = {}); public: Expr *getIndexExpr() const { return IndexExpr; } @@ -6024,14 +6022,10 @@ 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); + QualType Pattern, Expr *E, bool FullySubstituted, + ArrayRef<QualType> Expansions); private: const QualType *getExpansionsPtr() const { diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 1ed16748dff1a..14f4994f1688f 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_()), @@ -6438,7 +6438,7 @@ QualType ASTContext::getPackIndexingType(QualType Pattern, Expr *IndexExpr, } else { llvm::FoldingSetNodeID ID; PackIndexingType::Profile(ID, *this, Pattern.getCanonicalType(), IndexExpr, - FullySubstituted); + FullySubstituted, Expansions); void *InsertPos = nullptr; PackIndexingType *Canon = DependentPackIndexingTypes.FindNodeOrInsertPos(ID, InsertPos); @@ -6447,7 +6447,7 @@ QualType ASTContext::getPackIndexingType(QualType Pattern, Expr *IndexExpr, PackIndexingType::totalSizeToAlloc<QualType>(Expansions.size()), TypeAlignment); Canon = new (Mem) - PackIndexingType(*this, QualType(), Pattern.getCanonicalType(), + PackIndexingType(QualType(), Pattern.getCanonicalType(), IndexExpr, FullySubstituted, Expansions); DependentPackIndexingTypes.InsertNode(Canon, InsertPos); } @@ -6457,7 +6457,7 @@ QualType ASTContext::getPackIndexingType(QualType Pattern, Expr *IndexExpr, void *Mem = Allocate(PackIndexingType::totalSizeToAlloc<QualType>(Expansions.size()), TypeAlignment); - auto *T = new (Mem) PackIndexingType(*this, Canonical, Pattern, IndexExpr, + auto *T = new (Mem) PackIndexingType(Canonical, Pattern, IndexExpr, FullySubstituted, Expansions); Types.push_back(T); return QualType(T, 0); 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..93da9d31083db 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<QualType> 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<QualType>()); } @@ -4174,12 +4174,26 @@ PackIndexingType::computeDependence(QualType Pattern, Expr *IndexExpr, return TD; } +void PackIndexingType::Profile(llvm::FoldingSetNodeID &ID, + const ASTContext &Context) { + Profile(ID, Context, getPattern(), getIndexExpr(), isFullySubstituted(), getExpansions()); +} + void PackIndexingType::Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Context, QualType Pattern, - Expr *E, bool FullySubstituted) { - Pattern.Profile(ID); + Expr *E, bool FullySubstituted, + ArrayRef<QualType> Expansions) { + E->Profile(ID, Context, true); ID.AddBoolean(FullySubstituted); + if(!Expansions.empty()) { + ID.AddInteger(Expansions.size()); + for(QualType T : Expansions) + T.getCanonicalType().Profile(ID); + } + else { + Pattern.Profile(ID); + } } UnaryTransformType::UnaryTransformType(QualType BaseType, 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 <typename T> consteval void S::mfn() requires (bool(&fn)) {} } + + +namespace GH138255 { + +template <typename... T> +concept C = true; + +struct Func { + template<typename... Ts> + requires C<Ts...[0]> + static auto buggy() -> void; + + template<typename... Ts> + requires C<Ts...[0]> + friend auto fr() -> void; + + template<typename... Ts> + requires C<Ts...[0]> + friend auto fr2() -> void{}; // expected-note{{previous definition is here}} +}; + +template<typename... Ts> +requires C<Ts...[0]> +auto Func::buggy() -> void {} + +template<typename... Ts> +requires C<Ts...[0]> +auto fr() -> void {} + +template<typename... Ts> +requires C<Ts...[0]> +auto fr2() -> void {} // expected-error{{redefinition of 'fr2'}} + + +template <typename... Ts> +requires C<Ts...[0]> +struct Class; + +template <typename... Ts> +requires C<Ts...[0]> +struct Class; + + +template <typename...> +struct TplClass { + template<typename... Ts> + requires C<Ts...[0]> + static auto buggy() -> void; +}; + +template<> +template<typename... Ts> +requires C<Ts...[0]> +auto TplClass<int>::buggy() -> void {} + +} + +namespace PackIndexExpr { +template <int... T> +concept C = true; + +template <typename...> struct TplClass { + template <int... Ts> + requires C<Ts...[0]> + static auto buggy() -> void; +}; + +template <> +template <int... Ts> +requires C<Ts...[0]> +auto TplClass<int>::buggy() -> void {} +} `````````` </details> https://github.com/llvm/llvm-project/pull/139276 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits