Author: Younan Zhang Date: 2024-10-01T12:28:30+08:00 New Revision: 463a4f15044c04279583d6d0da73ae49f4c242ec
URL: https://github.com/llvm/llvm-project/commit/463a4f15044c04279583d6d0da73ae49f4c242ec DIFF: https://github.com/llvm/llvm-project/commit/463a4f15044c04279583d6d0da73ae49f4c242ec.diff LOG: [Clang][Concepts] Normalize SizeOfPackExpr's pack declaration (#110238) SizeOfPackExpr has a pointer to the referenced pack declaration, which is left as-is during the transformation process. The situation could be subtle when a friend class template declaration comes into play. The declaration per se would be instantiated into its parent declaration context, and consequently, the template parameter list would have a depth adjustment; however, as we don't evaluate constraints during instantiation, those constraints would still reference the original template parameters, which is fine for constraint evaluation because we have handled friend cases in the template argument collection. However, things are different when we want to profile the constraint expression with dependent template arguments. The hash algorithm of SizeOfPackExpr takes its pack declaration as a factor, which is the original template parameter that might still have untransformed template depths after the constraint normalization. This patch transforms the pack declaration when normalizing constraint expressions and pluses a fix in HandleFunctionTemplateDecl() where the associated declaration is incorrect for nested specifiers. Note that the fix in HandleFunctionTemplateDecl(), as well as the handling logic for NestedNameSpecifier, would be removed once Krystian's refactoring patch lands. But I still want to incorporate it in the patch for the correction purpose, though it hasn't caused any problems so far - I just tripped over that in getFullyPackExpandedSize() when I tried to extract the transformed declarations from the TemplateArgument. Fixes #93099 --------- Co-authored-by: Matheus Izvekov <mizve...@gmail.com> Added: Modified: clang/docs/ReleaseNotes.rst clang/lib/Sema/SemaConcept.cpp clang/lib/Sema/SemaTemplateInstantiate.cpp clang/test/SemaTemplate/concepts-out-of-line-def.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index a7c1bb80a49dbb..6a1e60b9b5097e 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -453,6 +453,8 @@ Bug Fixes to C++ Support - Mangle friend function templates with a constraint that depends on a template parameter from an enclosing template as members of the enclosing class. (#GH110247) - Fixed an issue in constraint evaluation, where type constraints on the lambda expression containing outer unexpanded parameters were not correctly expanded. (#GH101754) +- Fixed a bug in constraint expression comparison where the ``sizeof...`` expression was not handled properly + in certain friend declarations. (#GH93099) Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp index 6a1b32598bb4a6..67fc603e9ce1d5 100644 --- a/clang/lib/Sema/SemaConcept.cpp +++ b/clang/lib/Sema/SemaConcept.cpp @@ -975,11 +975,14 @@ static const Expr *SubstituteConstraintExpressionWithoutSatisfaction( // parameters that the surrounding function hasn't been instantiated yet. Note // this may happen while we're comparing two templates' constraint // equivalence. - LocalInstantiationScope ScopeForParameters(S); - if (auto *FD = DeclInfo.getDecl()->getAsFunction()) + std::optional<LocalInstantiationScope> ScopeForParameters; + if (const NamedDecl *ND = DeclInfo.getDecl(); + ND && ND->isFunctionOrFunctionTemplate()) { + ScopeForParameters.emplace(S); + const FunctionDecl *FD = ND->getAsFunction(); for (auto *PVD : FD->parameters()) { if (!PVD->isParameterPack()) { - ScopeForParameters.InstantiatedLocal(PVD, PVD); + ScopeForParameters->InstantiatedLocal(PVD, PVD); continue; } // This is hacky: we're mapping the parameter pack to a size-of-1 argument @@ -998,9 +1001,10 @@ static const Expr *SubstituteConstraintExpressionWithoutSatisfaction( // that we can eliminate the Scope in the cases where the declarations are // not necessarily instantiated. It would also benefit the noexcept // specifier comparison. - ScopeForParameters.MakeInstantiatedLocalArgPack(PVD); - ScopeForParameters.InstantiatedLocalPackArg(PVD, PVD); + ScopeForParameters->MakeInstantiatedLocalArgPack(PVD); + ScopeForParameters->InstantiatedLocalPackArg(PVD, PVD); } + } std::optional<Sema::CXXThisScopeRAII> ThisScope; diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index e874ab563e2f83..b36381422851f8 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -371,7 +371,7 @@ Response HandleFunctionTemplateDecl(const FunctionTemplateDecl *FTD, Specialization->getTemplateInstantiationArgs().asArray(); } Result.addOuterTemplateArguments( - const_cast<FunctionTemplateDecl *>(FTD), Arguments, + TSTy->getTemplateName().getAsTemplateDecl(), Arguments, /*Final=*/false); } } @@ -1737,6 +1737,33 @@ namespace { return inherited::TransformLambdaBody(E, Body); } + ExprResult RebuildSizeOfPackExpr(SourceLocation OperatorLoc, + NamedDecl *Pack, SourceLocation PackLoc, + SourceLocation RParenLoc, + std::optional<unsigned> Length, + ArrayRef<TemplateArgument> PartialArgs) { + if (SemaRef.CodeSynthesisContexts.back().Kind != + Sema::CodeSynthesisContext::ConstraintNormalization) + return inherited::RebuildSizeOfPackExpr(OperatorLoc, Pack, PackLoc, + RParenLoc, Length, PartialArgs); + +#ifndef NDEBUG + for (auto *Iter = TemplateArgs.begin(); Iter != TemplateArgs.end(); + ++Iter) + for (const TemplateArgument &TA : Iter->Args) + assert(TA.getKind() != TemplateArgument::Pack || TA.pack_size() == 1); +#endif + Sema::ArgumentPackSubstitutionIndexRAII SubstIndex( + SemaRef, /*NewSubstitutionIndex=*/0); + Decl *NewPack = TransformDecl(PackLoc, Pack); + if (!NewPack) + return ExprError(); + + return inherited::RebuildSizeOfPackExpr(OperatorLoc, + cast<NamedDecl>(NewPack), PackLoc, + RParenLoc, Length, PartialArgs); + } + ExprResult TransformRequiresExpr(RequiresExpr *E) { LocalInstantiationScope Scope(SemaRef, /*CombineWithOuterScope=*/true); ExprResult TransReq = inherited::TransformRequiresExpr(E); diff --git a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp index 5450d105a6f54a..8ca399a0f729a9 100644 --- a/clang/test/SemaTemplate/concepts-out-of-line-def.cpp +++ b/clang/test/SemaTemplate/concepts-out-of-line-def.cpp @@ -666,3 +666,37 @@ int foo() { } } // namespace eve + +namespace GH93099 { + +// Issues with sizeof...(expr) + +template <typename T = int> struct C { + template <int... N> + requires(sizeof...(N) > 0) + friend class NTTP; + + template <class... Tp> + requires(sizeof...(Tp) > 0) + friend class TP; + + template <template <typename> class... TTp> + requires(sizeof...(TTp) > 0) + friend class TTP; +}; + +template <int... N> + requires(sizeof...(N) > 0) +class NTTP; + +template <class... Tp> + requires(sizeof...(Tp) > 0) +class TP; + +template <template <typename> class... TTp> + requires(sizeof...(TTp) > 0) +class TTP; + +C v; + +} // namespace GH93099 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
