From: Matheus Izvekov Date: Wed, 31 Aug 2022 23:44:38 +0000 (+0200) Subject: [clang] fix profiling of template arguments of template and declaration kind X-Git-Tag: upstream/17.0.6~34292 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=acb767f5cda59302aa9100afcf6133d66779d42c;p=platform%2Fupstream%2Fllvm.git [clang] fix profiling of template arguments of template and declaration kind Template arguments of template and declaration kind were being profiled only by their canonical properties, which would cause incorrect uniquing of constrained AutoTypes, leading to a crash in some cases. This exposed some places in CheckTemplateArgumentList where non-canonical arguments where being pushed into the resulting converted list. We also throw in some asserts to catch early and explain the crashes. Note that the fix for the 'declaration' kind is untestable at this point, because there should be no cases right now in the AST where we try to unique a non-canonical converted template argument. This fixes GH55567. Signed-off-by: Matheus Izvekov Differential Revision: https://reviews.llvm.org/D133072 --- diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 20fcc8f..28f4e19 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -5116,7 +5116,9 @@ ASTContext::getDependentTemplateSpecializationType( CanonArgs); // Find the insert position again. - DependentTemplateSpecializationTypes.FindNodeOrInsertPos(ID, InsertPos); + [[maybe_unused]] auto *Nothing = + DependentTemplateSpecializationTypes.FindNodeOrInsertPos(ID, InsertPos); + assert(!Nothing && "canonical type broken"); } void *Mem = Allocate((sizeof(DependentTemplateSpecializationType) + @@ -5731,7 +5733,9 @@ QualType ASTContext::getAutoTypeInternal( Canon = getAutoTypeInternal(QualType(), Keyword, IsDependent, IsPack, TypeConstraintConcept, CanonArgs, true); // Find the insert position again. - AutoTypes.FindNodeOrInsertPos(ID, InsertPos); + [[maybe_unused]] auto *Nothing = + AutoTypes.FindNodeOrInsertPos(ID, InsertPos); + assert(!Nothing && "canonical type broken"); } } else { Canon = DeducedType.getCanonicalType(); diff --git a/clang/lib/AST/TemplateBase.cpp b/clang/lib/AST/TemplateBase.cpp index e0f5916..6d9ab2b 100644 --- a/clang/lib/AST/TemplateBase.cpp +++ b/clang/lib/AST/TemplateBase.cpp @@ -321,26 +321,15 @@ void TemplateArgument::Profile(llvm::FoldingSetNodeID &ID, case Declaration: getParamTypeForDecl().Profile(ID); - ID.AddPointer(getAsDecl()? getAsDecl()->getCanonicalDecl() : nullptr); + ID.AddPointer(getAsDecl()); break; + case TemplateExpansion: + ID.AddInteger(TemplateArg.NumExpansions); + LLVM_FALLTHROUGH; case Template: - case TemplateExpansion: { - TemplateName Template = getAsTemplateOrTemplatePattern(); - if (TemplateTemplateParmDecl *TTP - = dyn_cast_or_null( - Template.getAsTemplateDecl())) { - ID.AddBoolean(true); - ID.AddInteger(TTP->getDepth()); - ID.AddInteger(TTP->getPosition()); - ID.AddBoolean(TTP->isParameterPack()); - } else { - ID.AddBoolean(false); - ID.AddPointer(Context.getCanonicalTemplateName(Template) - .getAsVoidPointer()); - } + getAsTemplateOrTemplatePattern().Profile(ID); break; - } case Integral: getAsIntegral().Profile(ID); diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index b06612d..e9076bb 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -5643,7 +5643,8 @@ bool Sema::CheckTemplateArgument(NamedDecl *Param, if (CheckTemplateTemplateArgument(TempParm, Params, Arg)) return true; - Converted.push_back(Arg.getArgument()); + Converted.push_back( + Context.getCanonicalTemplateArgument(Arg.getArgument())); break; case TemplateArgument::Expression: @@ -5813,13 +5814,14 @@ bool Sema::CheckTemplateArgumentList( if (!ArgumentPack.empty()) { // If we were part way through filling in an expanded parameter pack, // fall back to just producing individual arguments. - Converted.insert(Converted.end(), - ArgumentPack.begin(), ArgumentPack.end()); + for (const TemplateArgument &I : ArgumentPack) + Converted.push_back(Context.getCanonicalTemplateArgument(I)); ArgumentPack.clear(); } while (ArgIdx < NumArgs) { - Converted.push_back(NewArgs[ArgIdx].getArgument()); + Converted.push_back(Context.getCanonicalTemplateArgument( + NewArgs[ArgIdx].getArgument())); ++ArgIdx; } @@ -5952,7 +5954,8 @@ bool Sema::CheckTemplateArgumentList( if (ArgIdx < NumArgs && CurrentInstantiationScope && CurrentInstantiationScope->getPartiallySubstitutedPack()) { while (ArgIdx < NumArgs && NewArgs[ArgIdx].getArgument().isPackExpansion()) - Converted.push_back(NewArgs[ArgIdx++].getArgument()); + Converted.push_back(Context.getCanonicalTemplateArgument( + NewArgs[ArgIdx++].getArgument())); } // If we have any leftover arguments, then there were too many arguments. diff --git a/clang/test/SemaTemplate/concepts.cpp b/clang/test/SemaTemplate/concepts.cpp index 23c7942..332eed5 100644 --- a/clang/test/SemaTemplate/concepts.cpp +++ b/clang/test/SemaTemplate/concepts.cpp @@ -256,3 +256,9 @@ C auto **j1 = g(); // expected-error {{deduced type 'int' does not satisfy 'C' C auto **&j2 = g(); // expected-error {{deduced type 'int' does not satisfy 'C'}} C auto **&&j3 = g(); // expected-error {{deduced type 'int' does not satisfy 'C'}} } + +namespace GH55567 { +template class> concept C = true; +template struct S {}; +void f(C auto); +} // namespace GH55567