From f4ea3bd4b2086e6de10131b197aaf7d066a24df8 Mon Sep 17 00:00:00 2001 From: Matheus Izvekov Date: Sat, 3 Sep 2022 18:36:59 +0200 Subject: [PATCH] [clang] Fixes how we represent / emulate builtin templates We change the template specialization of builtin templates to behave like aliases. Though unlike real alias templates, these might still produce a canonical TemplateSpecializationType when some important argument is dependent. For example, we can't do anything about make_integer_seq when the count is dependent, or a type_pack_element when the index is dependent. We change type deduction to not try to deduce canonical TSTs of builtin templates. We also change those buitin templates to produce substitution sugar, just like a real instantiation would, making the resulting type correctly represent the template arguments used to specialize the underlying template. And make_integer_seq will now produce a TST for the specialization of it's first argument, which we use as the underlying type of the builtin alias. When performing member access on the resulting type, it's now possible to map from a Subst* node to the template argument as-written used in a regular fashion, without special casing. And this fixes a bunch of bugs with relation to these builtin templates factoring into deduction. Fixes GH42102 and GH51928. Depends on D133261 Signed-off-by: Matheus Izvekov Differential Revision: https://reviews.llvm.org/D133262 --- clang/docs/ReleaseNotes.rst | 3 + clang/include/clang/AST/ASTContext.h | 2 +- clang/include/clang/AST/DeclTemplate.h | 3 + clang/lib/AST/ASTContext.cpp | 11 +-- clang/lib/AST/DeclTemplate.cpp | 10 +++ clang/lib/CodeGen/CGDebugInfo.cpp | 7 +- clang/lib/Sema/SemaTemplate.cpp | 89 +++++++++++++------ clang/lib/Sema/SemaTemplateDeduction.cpp | 15 +++- clang/test/SemaTemplate/make_integer_seq.cpp | 118 +++++++++++++++++--------- clang/test/SemaTemplate/type_pack_element.cpp | 70 ++++++++++----- 10 files changed, 229 insertions(+), 99 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index e4648cd..1ec21a7 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -122,6 +122,9 @@ Bug Fixes - Clang will now no longer treat a C 'overloadable' function without a prototype as a variadic function with the attribute. This should make further diagnostics more clear. +- Fixes to builtin template emulation of regular templates. + `Issue 42102 `_ + `Issue 51928 `_ Improvements to Clang's diagnostics diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index c022846..14661ec 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -1636,7 +1636,7 @@ public: ArrayRef Args) const; QualType getTemplateSpecializationType(TemplateName T, - const TemplateArgumentListInfo &Args, + ArrayRef Args, QualType Canon = QualType()) const; TypeSourceInfo * diff --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h index baed5ca..dbea107 100644 --- a/clang/include/clang/AST/DeclTemplate.h +++ b/clang/include/clang/AST/DeclTemplate.h @@ -440,6 +440,9 @@ public: /// Get the underlying, templated declaration. NamedDecl *getTemplatedDecl() const { return TemplatedDecl; } + // Should a specialization behave like an alias for another type. + bool isTypeAlias() const; + // Implement isa/cast/dyncast/etc. static bool classof(const Decl *D) { return classofKind(D->getKind()); } diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index c326435..5bf03d8 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -4848,7 +4848,8 @@ ASTContext::getTemplateSpecializationTypeInfo(TemplateName Name, QualType Underlying) const { assert(!Name.getAsDependentTemplateName() && "No dependent template names here!"); - QualType TST = getTemplateSpecializationType(Name, Args, Underlying); + QualType TST = + getTemplateSpecializationType(Name, Args.arguments(), Underlying); TypeSourceInfo *DI = CreateTypeSourceInfo(TST); TemplateSpecializationTypeLoc TL = @@ -4864,14 +4865,14 @@ ASTContext::getTemplateSpecializationTypeInfo(TemplateName Name, QualType ASTContext::getTemplateSpecializationType(TemplateName Template, - const TemplateArgumentListInfo &Args, + ArrayRef Args, QualType Underlying) const { assert(!Template.getAsDependentTemplateName() && "No dependent template names here!"); SmallVector ArgVec; ArgVec.reserve(Args.size()); - for (const TemplateArgumentLoc &Arg : Args.arguments()) + for (const TemplateArgumentLoc &Arg : Args) ArgVec.push_back(Arg.getArgument()); return getTemplateSpecializationType(Template, ArgVec, Underlying); @@ -4897,8 +4898,8 @@ ASTContext::getTemplateSpecializationType(TemplateName Template, if (QualifiedTemplateName *QTN = Template.getAsQualifiedTemplateName()) Template = QTN->getUnderlyingTemplate(); - bool IsTypeAlias = - isa_and_nonnull(Template.getAsTemplateDecl()); + const auto *TD = Template.getAsTemplateDecl(); + bool IsTypeAlias = TD && TD->isTypeAlias(); QualType CanonType; if (!Underlying.isNull()) CanonType = getCanonicalType(Underlying); diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp index e3dd780..6bb5a49 100644 --- a/clang/lib/AST/DeclTemplate.cpp +++ b/clang/lib/AST/DeclTemplate.cpp @@ -250,6 +250,16 @@ bool TemplateDecl::hasAssociatedConstraints() const { return false; } +bool TemplateDecl::isTypeAlias() const { + switch (getKind()) { + case TemplateDecl::TypeAliasTemplate: + case TemplateDecl::BuiltinTemplate: + return true; + default: + return false; + }; +} + //===----------------------------------------------------------------------===// // RedeclarableTemplateDecl Implementation //===----------------------------------------------------------------------===// diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 49dcf77..73cb808 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -1263,10 +1263,11 @@ llvm::DIType *CGDebugInfo::CreateType(const TemplateSpecializationType *Ty, assert(Ty->isTypeAlias()); llvm::DIType *Src = getOrCreateType(Ty->getAliasedType(), Unit); - auto *AliasDecl = - cast(Ty->getTemplateName().getAsTemplateDecl()) - ->getTemplatedDecl(); + const TemplateDecl *TD = Ty->getTemplateName().getAsTemplateDecl(); + if (isa(TD)) + return Src; + const auto *AliasDecl = cast(TD)->getTemplatedDecl(); if (AliasDecl->hasAttr()) return Src; diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index ed507fd..b8cf69f 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -3501,45 +3501,70 @@ checkBuiltinTemplateIdType(Sema &SemaRef, BuiltinTemplateDecl *BTD, SourceLocation TemplateLoc, TemplateArgumentListInfo &TemplateArgs) { ASTContext &Context = SemaRef.getASTContext(); + + TemplateParameterList *TPL = BTD->getTemplateParameters(); + + // Wrap the type in substitution sugar. + auto getSubstType = [&](unsigned IndexReplaced, QualType Replacement) { + QualType TTP = SemaRef.Context.getTemplateTypeParmType( + 0, IndexReplaced, false, + cast(TPL->getParam(IndexReplaced))); + return SemaRef.Context.getSubstTemplateTypeParmType( + cast(TTP), Replacement.getCanonicalType()); + }; + switch (BTD->getBuiltinTemplateKind()) { case BTK__make_integer_seq: { // Specializations of __make_integer_seq are treated like // S. + QualType OrigType = Converted[1].getAsType(); // C++14 [inteseq.intseq]p1: // T shall be an integer type. - if (!Converted[1].getAsType()->isIntegralType(Context)) { + if (!OrigType->isDependentType() && !OrigType->isIntegralType(Context)) { SemaRef.Diag(TemplateArgs[1].getLocation(), diag::err_integer_sequence_integral_element_type); return QualType(); } - // C++14 [inteseq.make]p1: - // If N is negative the program is ill-formed. TemplateArgument NumArgsArg = Converted[2]; - llvm::APSInt NumArgs = NumArgsArg.getAsIntegral(); - if (NumArgs < 0) { + if (NumArgsArg.isDependent()) + return Context.getCanonicalTemplateSpecializationType(TemplateName(BTD), + Converted); + + TemplateArgumentListInfo SyntheticTemplateArgs; + // The type argument, wrapped in substitution sugar, gets reused as the + // first template argument in the synthetic template argument list. + QualType SyntheticType = getSubstType(1, OrigType); + SyntheticTemplateArgs.addArgument( + TemplateArgumentLoc(TemplateArgument(SyntheticType), + SemaRef.Context.getTrivialTypeSourceInfo( + SyntheticType, TemplateArgs[1].getLocation()))); + + if (llvm::APSInt NumArgs = NumArgsArg.getAsIntegral(); NumArgs >= 0) { + // Expand N into 0 ... N-1. + for (llvm::APSInt I(NumArgs.getBitWidth(), NumArgs.isUnsigned()); + I < NumArgs; ++I) { + TemplateArgument TA(Context, I, SyntheticType); + SyntheticTemplateArgs.addArgument(SemaRef.getTrivialTemplateArgumentLoc( + TA, SyntheticType, TemplateArgs[2].getLocation())); + } + } else { + // C++14 [inteseq.make]p1: + // If N is negative the program is ill-formed. SemaRef.Diag(TemplateArgs[2].getLocation(), diag::err_integer_sequence_negative_length); return QualType(); } - QualType ArgTy = NumArgsArg.getIntegralType(); - TemplateArgumentListInfo SyntheticTemplateArgs; - // The type argument gets reused as the first template argument in the - // synthetic template argument list. - SyntheticTemplateArgs.addArgument(TemplateArgs[1]); - // Expand N into 0 ... N-1. - for (llvm::APSInt I(NumArgs.getBitWidth(), NumArgs.isUnsigned()); - I < NumArgs; ++I) { - TemplateArgument TA(Context, I, ArgTy); - SyntheticTemplateArgs.addArgument(SemaRef.getTrivialTemplateArgumentLoc( - TA, ArgTy, TemplateArgs[2].getLocation())); - } + // Wrap the template in substitution sugar. + TemplateName TN = SemaRef.Context.getSubstTemplateTemplateParm( + cast(TPL->getParam(0)), + Converted[0].getAsTemplate()); + // The first template argument will be reused as the template decl that // our synthetic template arguments will be applied to. - return SemaRef.CheckTemplateIdType(Converted[0].getAsTemplate(), - TemplateLoc, SyntheticTemplateArgs); + return SemaRef.CheckTemplateIdType(TN, TemplateLoc, SyntheticTemplateArgs); } case BTK__type_pack_element: @@ -3549,11 +3574,15 @@ checkBuiltinTemplateIdType(Sema &SemaRef, BuiltinTemplateDecl *BTD, assert(Converted.size() == 2 && "__type_pack_element should be given an index and a parameter pack"); - // If the Index is out of bounds, the program is ill-formed. TemplateArgument IndexArg = Converted[0], Ts = Converted[1]; + if (IndexArg.isDependent() || Ts.isDependent()) + return Context.getCanonicalTemplateSpecializationType(TemplateName(BTD), + Converted); + llvm::APSInt Index = IndexArg.getAsIntegral(); assert(Index >= 0 && "the index used with __type_pack_element should be of " "type std::size_t, and hence be non-negative"); + // If the Index is out of bounds, the program is ill-formed. if (Index >= Ts.pack_size()) { SemaRef.Diag(TemplateArgs[0].getLocation(), diag::err_type_pack_element_out_of_bounds); @@ -3562,7 +3591,7 @@ checkBuiltinTemplateIdType(Sema &SemaRef, BuiltinTemplateDecl *BTD, // We simply return the type at index `Index`. auto Nth = std::next(Ts.pack_begin(), Index.getExtValue()); - return Nth->getAsType(); + return getSubstType(1, Nth->getAsType()); } llvm_unreachable("unexpected BuiltinTemplateDecl!"); } @@ -3730,7 +3759,8 @@ QualType Sema::CheckTemplateIdType(TemplateName Name, // We might have a substituted template template parameter pack. If so, // build a template specialization type for it. if (Name.getAsSubstTemplateTemplateParmPack()) - return Context.getTemplateSpecializationType(Name, TemplateArgs); + return Context.getTemplateSpecializationType(Name, + TemplateArgs.arguments()); Diag(TemplateLoc, diag::err_template_id_not_a_type) << Name; @@ -3808,6 +3838,9 @@ QualType Sema::CheckTemplateIdType(TemplateName Name, return QualType(); } + } else if (auto *BTD = dyn_cast(Template)) { + CanonType = checkBuiltinTemplateIdType(*this, BTD, Converted, TemplateLoc, + TemplateArgs); } else if (Name.isDependent() || TemplateSpecializationType::anyDependentTemplateArguments( TemplateArgs, Converted)) { @@ -3857,8 +3890,8 @@ QualType Sema::CheckTemplateIdType(TemplateName Name, break; } } - } else if (ClassTemplateDecl *ClassTemplate - = dyn_cast(Template)) { + } else if (ClassTemplateDecl *ClassTemplate = + dyn_cast(Template)) { // Find the class template specialization declaration that // corresponds to these arguments. void *InsertPos = nullptr; @@ -3895,15 +3928,15 @@ QualType Sema::CheckTemplateIdType(TemplateName Name, CanonType = Context.getTypeDeclType(Decl); assert(isa(CanonType) && "type of non-dependent specialization is not a RecordType"); - } else if (auto *BTD = dyn_cast(Template)) { - CanonType = checkBuiltinTemplateIdType(*this, BTD, Converted, TemplateLoc, - TemplateArgs); + } else { + llvm_unreachable("Unhandled template kind"); } // Build the fully-sugared type for this class template // specialization, which refers back to the class template // specialization we created or found. - return Context.getTemplateSpecializationType(Name, TemplateArgs, CanonType); + return Context.getTemplateSpecializationType(Name, TemplateArgs.arguments(), + CanonType); } void Sema::ActOnUndeclaredTypeTemplateName(Scope *S, TemplateTy &ParsedName, diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp index 82f6d94..5e0f5f2 100644 --- a/clang/lib/Sema/SemaTemplateDeduction.cpp +++ b/clang/lib/Sema/SemaTemplateDeduction.cpp @@ -563,6 +563,12 @@ DeduceTemplateSpecArguments(Sema &S, TemplateParameterList *TemplateParams, // FIXME: Try to preserve type sugar here, which is hard // because of the unresolved template arguments. const auto *TP = UP.getCanonicalType()->castAs(); + TemplateName TNP = TP->getTemplateName(); + + // If the parameter is an alias template, there is nothing to deduce. + if (const auto *TD = TNP.getAsTemplateDecl(); TD && TD->isTypeAlias()) + return Sema::TDK_Success; + ArrayRef PResolved = TP->template_arguments(); QualType UA = A; @@ -574,10 +580,15 @@ DeduceTemplateSpecArguments(Sema &S, TemplateParameterList *TemplateParams, // FIXME: Should not lose sugar here. if (const auto *SA = dyn_cast(UA.getCanonicalType())) { + TemplateName TNA = SA->getTemplateName(); + + // If the argument is an alias template, there is nothing to deduce. + if (const auto *TD = TNA.getAsTemplateDecl(); TD && TD->isTypeAlias()) + return Sema::TDK_Success; + // Perform template argument deduction for the template name. if (auto Result = - DeduceTemplateArguments(S, TemplateParams, TP->getTemplateName(), - SA->getTemplateName(), Info, Deduced)) + DeduceTemplateArguments(S, TemplateParams, TNP, TNA, Info, Deduced)) return Result; // Perform template argument deduction on each template // argument. Ignore any missing/extra arguments, since they could be diff --git a/clang/test/SemaTemplate/make_integer_seq.cpp b/clang/test/SemaTemplate/make_integer_seq.cpp index 78d66c7..dfdb65c 100644 --- a/clang/test/SemaTemplate/make_integer_seq.cpp +++ b/clang/test/SemaTemplate/make_integer_seq.cpp @@ -4,21 +4,31 @@ template struct A {}; using test1 = __make_integer_seq; // CHECK: |-TypeAliasDecl 0x{{[0-9A-Fa-f]+}} col:7 test1 '__make_integer_seq':'A' -// CHECK-NEXT: | `-ElaboratedType 0x{{[0-9A-Fa-f]+}} '__make_integer_seq' sugar -// CHECK-NEXT: | `-TemplateSpecializationType 0x{{[0-9A-Fa-f]+}} '__make_integer_seq' sugar __make_integer_seq -// CHECK-NEXT: | |-TemplateArgument template A -// CHECK-NEXT: | |-TemplateArgument type 'int' -// CHECK-NEXT: | | `-BuiltinType 0x{{[0-9A-Fa-f]+}} 'int' -// CHECK-NEXT: | |-TemplateArgument expr -// CHECK-NEXT: | | `-ConstantExpr 0x{{[0-9A-Fa-f]+}} 'int' -// CHECK-NEXT: | | |-value: Int 1 -// CHECK-NEXT: | | `-IntegerLiteral 0x{{[0-9A-Fa-f]+}} 'int' 1 -// CHECK-NEXT: | `-RecordType 0x{{[0-9A-Fa-f]+}} 'A' -// CHECK-NEXT: | `-ClassTemplateSpecialization 0x{{[0-9A-Fa-f]+}} 'A' +// CHECK-NEXT: `-ElaboratedType 0x{{[0-9A-Fa-f]+}} '__make_integer_seq' sugar +// CHECK-NEXT: `-TemplateSpecializationType 0x{{[0-9A-Fa-f]+}} '__make_integer_seq' sugar alias __make_integer_seq +// CHECK-NEXT: |-TemplateArgument template A +// CHECK-NEXT: |-TemplateArgument type 'int' +// CHECK-NEXT: | `-BuiltinType 0x{{[0-9A-Fa-f]+}} 'int' +// CHECK-NEXT: |-TemplateArgument expr +// CHECK-NEXT: | `-ConstantExpr 0x{{[0-9A-Fa-f]+}} 'int' +// CHECK-NEXT: | |-value: Int 1 +// CHECK-NEXT: | `-IntegerLiteral 0x{{[0-9A-Fa-f]+}} 'int' 1 +// CHECK-NEXT: `-TemplateSpecializationType 0x{{[0-9A-Fa-f]+}} 'A' sugar A +// CHECK-NEXT: |-TemplateArgument type 'int':'int' +// CHECK-NEXT: | `-SubstTemplateTypeParmType 0x{{[0-9A-Fa-f]+}} 'int' sugar +// CHECK-NEXT: | |-TemplateTypeParmType 0x{{[0-9A-Fa-f]+}} 'auto' dependent depth 0 index 1 +// CHECK-NEXT: | | `-TemplateTypeParm 0x{{[0-9A-Fa-f]+}} '' +// CHECK-NEXT: | `-BuiltinType 0x{{[0-9A-Fa-f]+}} 'int' +// CHECK-NEXT: |-TemplateArgument expr +// CHECK-NEXT: | `-ConstantExpr 0x{{[0-9A-Fa-f]+}} 'int' +// CHECK-NEXT: | |-value: Int 0 +// CHECK-NEXT: | `-IntegerLiteral 0x{{[0-9A-Fa-f]+}} 'int':'int' 0 +// CHECK-NEXT: `-RecordType 0x{{[0-9A-Fa-f]+}} 'A' +// CHECK-NEXT: `-ClassTemplateSpecialization 0x{{[0-9A-Fa-f]+}} 'A' template using B = __make_integer_seq; using test2 = B; -// CHECK: |-TypeAliasDecl 0x{{[0-9A-Fa-f]+}} col:7 test2 'B':'A' +// CHECK: |-TypeAliasDecl 0x{{[0-9A-Fa-f]+}} col:7 test2 'B':'A' // CHECK-NEXT: `-ElaboratedType 0x{{[0-9A-Fa-f]+}} 'B' sugar // CHECK-NEXT: `-TemplateSpecializationType 0x{{[0-9A-Fa-f]+}} 'B' sugar alias B // CHECK-NEXT: |-TemplateArgument type 'int' @@ -28,7 +38,7 @@ using test2 = B; // CHECK-NEXT: | |-value: Int 1 // CHECK-NEXT: | `-IntegerLiteral 0x{{[0-9A-Fa-f]+}} 'int' 1 // CHECK-NEXT: `-ElaboratedType 0x{{[0-9A-Fa-f]+}} '__make_integer_seq' sugar -// CHECK-NEXT: `-TemplateSpecializationType 0x{{[0-9A-Fa-f]+}} '__make_integer_seq' sugar __make_integer_seq +// CHECK-NEXT: `-TemplateSpecializationType 0x{{[0-9A-Fa-f]+}} '__make_integer_seq' sugar alias __make_integer_seq // CHECK-NEXT: |-TemplateArgument template A // CHECK-NEXT: |-TemplateArgument type 'int':'int' // CHECK-NEXT: | `-SubstTemplateTypeParmType 0x{{[0-9A-Fa-f]+}} 'int' sugar @@ -36,61 +46,93 @@ using test2 = B; // CHECK-NEXT: | | `-TemplateTypeParm 0x{{[0-9A-Fa-f]+}} 'B1' // CHECK-NEXT: | `-BuiltinType 0x{{[0-9A-Fa-f]+}} 'int' // CHECK-NEXT: |-TemplateArgument expr -// CHECK-NEXT: | `-ConstantExpr 0x{{[0-9A-Fa-f]+}} 'int' +// CHECK-NEXT: | `-ConstantExpr 0x{{[0-9A-Fa-f]+}} 'int' // CHECK-NEXT: | |-value: Int 1 // CHECK-NEXT: | `-SubstNonTypeTemplateParmExpr 0x{{[0-9A-Fa-f]+}} 'int' // CHECK-NEXT: | |-NonTypeTemplateParmDecl 0x{{[0-9A-Fa-f]+}} col:24 referenced 'B1' depth 0 index 1 B2 // CHECK-NEXT: | `-IntegerLiteral 0x{{[0-9A-Fa-f]+}} 'int' 1 -// CHECK-NEXT: `-RecordType 0x{{[0-9A-Fa-f]+}} 'A' -// CHECK-NEXT: `-ClassTemplateSpecialization 0x{{[0-9A-Fa-f]+}} 'A' +// CHECK-NEXT: `-TemplateSpecializationType 0x{{[0-9A-Fa-f]+}} 'A' sugar A +// CHECK-NEXT: |-TemplateArgument type 'int':'int' +// CHECK-NEXT: | `-SubstTemplateTypeParmType 0x{{[0-9A-Fa-f]+}} 'int' sugar +// CHECK-NEXT: | |-TemplateTypeParmType 0x{{[0-9A-Fa-f]+}} 'auto' dependent depth 0 index 1 +// CHECK-NEXT: | | `-TemplateTypeParm 0x{{[0-9A-Fa-f]+}} '' +// CHECK-NEXT: | `-BuiltinType 0x{{[0-9A-Fa-f]+}} 'int' +// CHECK-NEXT: |-TemplateArgument expr +// CHECK-NEXT: | `-ConstantExpr 0x{{[0-9A-Fa-f]+}} 'int' +// CHECK-NEXT: | |-value: Int 0 +// CHECK-NEXT: | `-IntegerLiteral 0x{{[0-9A-Fa-f]+}} 'int':'int' 0 +// CHECK-NEXT: `-RecordType 0x{{[0-9A-Fa-f]+}} 'A' +// CHECK-NEXT: `-ClassTemplateSpecialization 0x{{[0-9A-Fa-f]+}} 'A' template