llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Matheus Izvekov (mizvekov) <details> <summary>Changes</summary> This adds a way to attach source locations to trivially created template arguments such as packs, or converted expressions when there is no expression available. This also avoids crashes due to missing source locations. In a few places where this matters, we already create expressions from the converted arguments, but this requires access to Sema, where currently creating trivial typelocs only requires access to to the ASTContext. So this creates a new storage kind for TemplateArgumentLocs, where a single SourceLocation is stored, embedded in the pointer where possible. As a drive-by, strenghten asserts by enforcing the TemplateArgumentLocs are created with the right kinds of locations. Fixes #<!-- -->186655 --- Full diff: https://github.com/llvm/llvm-project/pull/187352.diff 8 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+1) - (modified) clang/include/clang/AST/TemplateBase.h (+54-3) - (modified) clang/lib/AST/TemplateBase.cpp (+19) - (modified) clang/lib/AST/TypeLoc.cpp (+2-5) - (modified) clang/lib/Sema/SemaExpr.cpp (+4-4) - (modified) clang/lib/Sema/SemaTemplate.cpp (+3-3) - (modified) clang/lib/Sema/SemaTemplateDeduction.cpp (+1-1) - (modified) clang/test/SemaCXX/type_pack_element.cpp (+7) ``````````diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index e1adaba4be7fc..71e955cfd0b53 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -389,6 +389,7 @@ Miscellaneous Clang Crashes Fixed - Fixed a crash when using loop hint with a value dependent argument inside a generic lambda. (#GH172289) - Fixed a crash in C++ overload resolution with ``_Atomic``-qualified argument types. (#GH170433) +- Fixed a crash related to missing source locations (#GH186655) - Fixed a crash when casting a parenthesized unresolved template-id or array section. (#GH183505) - Fixed a crash when initializing a ``constexpr`` pointer with a floating-point literal in C23. (#GH180313) - Fixed an assertion when diagnosing address-space qualified ``new``/``delete`` in language-defined address spaces such as OpenCL ``__local``. (#GH178319) diff --git a/clang/include/clang/AST/TemplateBase.h b/clang/include/clang/AST/TemplateBase.h index de248ac3cf703..b137ecf034426 100644 --- a/clang/include/clang/AST/TemplateBase.h +++ b/clang/include/clang/AST/TemplateBase.h @@ -493,6 +493,10 @@ struct TemplateArgumentLocInfo { TemplateArgumentLocInfo(TypeSourceInfo *Declarator) { Pointer = Declarator; } TemplateArgumentLocInfo(Expr *E) { Pointer = E; } + + // For trivial source locations for converted template argument kinds. + TemplateArgumentLocInfo(ASTContext &Ctx, SourceLocation Loc); + // Ctx is used for allocation -- this case is unusually large and also rare, // so we store the payload out-of-line. TemplateArgumentLocInfo(ASTContext &Ctx, SourceLocation TemplateKwLoc, @@ -518,9 +522,29 @@ struct TemplateArgumentLocInfo { return getTemplate()->EllipsisLoc; } + bool isNull() const { return Pointer.isNull(); } + + bool isTrivial() const { return isa<LocOrPointer>(Pointer); } + + SourceLocation getTrivialLoc() const { + assert(isTrivial()); + auto *P = cast<LocOrPointer>(Pointer); + if constexpr (embedLocInPointer) + return SourceLocation::getFromRawEncoding( + (reinterpret_cast<uintptr_t>(P) >> lowBitsRequired) - 1u); + else + return *static_cast<SourceLocation *>(P); + } + private: - llvm::PointerUnion<TemplateTemplateArgLocInfo *, Expr *, TypeSourceInfo *> + static constexpr bool embedLocInPointer = sizeof(void *) > + sizeof(SourceLocation); + using LocOrPointer = + std::conditional_t<embedLocInPointer, void, SourceLocation> *; + llvm::PointerUnion<TemplateTemplateArgLocInfo *, Expr *, TypeSourceInfo *, + LocOrPointer> Pointer; + static constexpr unsigned lowBitsRequired = 2; }; /// Location wrapper for a TemplateArgument. TemplateArgument is to @@ -534,16 +558,43 @@ class TemplateArgumentLoc { TemplateArgumentLoc(const TemplateArgument &Argument, TemplateArgumentLocInfo Opaque) - : Argument(Argument), LocInfo(Opaque) {} + : Argument(Argument), LocInfo(Opaque) { + switch (Argument.getKind()) { + case TemplateArgument::Null: + assert(Opaque.isNull()); + return; + case TemplateArgument::Pack: + assert(Opaque.isTrivial()); + return; + case TemplateArgument::NullPtr: + case TemplateArgument::Integral: + case TemplateArgument::Declaration: + case TemplateArgument::StructuralValue: + assert(Opaque.isTrivial() || Opaque.getAsExpr() != nullptr); + return; + case TemplateArgument::Expression: + assert(Opaque.getAsExpr() != nullptr); + return; + case TemplateArgument::Type: + assert(Opaque.getAsTypeSourceInfo() != nullptr); + return; + case TemplateArgument::Template: + case TemplateArgument::TemplateExpansion: + assert(Opaque.getTemplate() != nullptr); + return; + } + llvm_unreachable("Unknown TemplateArgument kind"); + } TemplateArgumentLoc(const TemplateArgument &Argument, TypeSourceInfo *TInfo) : Argument(Argument), LocInfo(TInfo) { assert(Argument.getKind() == TemplateArgument::Type); + assert(TInfo != nullptr); } TemplateArgumentLoc(const TemplateArgument &Argument, Expr *E) : Argument(Argument), LocInfo(E) { - + assert(E != nullptr); // Permit any kind of template argument that can be represented with an // expression. assert(Argument.getKind() == TemplateArgument::NullPtr || diff --git a/clang/lib/AST/TemplateBase.cpp b/clang/lib/AST/TemplateBase.cpp index 131ae6e8a478f..e8702d8336aea 100644 --- a/clang/lib/AST/TemplateBase.cpp +++ b/clang/lib/AST/TemplateBase.cpp @@ -626,9 +626,13 @@ SourceRange TemplateArgumentLoc::getSourceRange() const { return getSourceExpression()->getSourceRange(); case TemplateArgument::Declaration: + if (LocInfo.isTrivial()) + return SourceRange(LocInfo.getTrivialLoc()); return getSourceDeclExpression()->getSourceRange(); case TemplateArgument::NullPtr: + if (LocInfo.isTrivial()) + return SourceRange(LocInfo.getTrivialLoc()); return getSourceNullPtrExpression()->getSourceRange(); case TemplateArgument::Type: @@ -650,12 +654,18 @@ SourceRange TemplateArgumentLoc::getSourceRange() const { return SourceRange(getTemplateNameLoc(), getTemplateEllipsisLoc()); case TemplateArgument::Integral: + if (LocInfo.isTrivial()) + return SourceRange(LocInfo.getTrivialLoc()); return getSourceIntegralExpression()->getSourceRange(); case TemplateArgument::StructuralValue: + if (LocInfo.isTrivial()) + return SourceRange(LocInfo.getTrivialLoc()); return getSourceStructuralValueExpression()->getSourceRange(); case TemplateArgument::Pack: + return SourceRange(LocInfo.getTrivialLoc()); + case TemplateArgument::Null: return SourceRange(); } @@ -737,6 +747,15 @@ clang::TemplateArgumentLocInfo::TemplateArgumentLocInfo( Pointer = Template; } +clang::TemplateArgumentLocInfo::TemplateArgumentLocInfo( + ASTContext &Ctx, SourceLocation TrivialLoc) { + if constexpr (embedLocInPointer) + Pointer = reinterpret_cast<void *>((TrivialLoc.getRawEncoding() + 1u) + << lowBitsRequired); + else + Pointer = new (Ctx) SourceLocation(TrivialLoc); +} + const ASTTemplateArgumentListInfo * ASTTemplateArgumentListInfo::Create(const ASTContext &C, const TemplateArgumentListInfo &List) { diff --git a/clang/lib/AST/TypeLoc.cpp b/clang/lib/AST/TypeLoc.cpp index 53edfdb65a4d5..7e72a85136966 100644 --- a/clang/lib/AST/TypeLoc.cpp +++ b/clang/lib/AST/TypeLoc.cpp @@ -723,11 +723,12 @@ void TemplateSpecializationTypeLoc::initializeArgLocs( case TemplateArgument::Null: llvm_unreachable("Impossible TemplateArgument"); + case TemplateArgument::Pack: case TemplateArgument::Integral: case TemplateArgument::Declaration: case TemplateArgument::NullPtr: case TemplateArgument::StructuralValue: - ArgInfos[i] = TemplateArgumentLocInfo(); + ArgInfos[i] = TemplateArgumentLocInfo(Context, Loc); break; case TemplateArgument::Expression: @@ -755,10 +756,6 @@ void TemplateSpecializationTypeLoc::initializeArgLocs( : Loc); break; } - - case TemplateArgument::Pack: - ArgInfos[i] = TemplateArgumentLocInfo(); - break; } } } diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index db6d93ce54791..2b0a786302aab 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -2366,14 +2366,14 @@ Sema::ActOnStringLiteral(ArrayRef<Token> StringToks, Scope *UDLScope) { TemplateArgumentLocInfo TypeArgInfo(Context.getTrivialTypeSourceInfo(CharTy)); ExplicitArgs.addArgument(TemplateArgumentLoc(TypeArg, TypeArgInfo)); + SourceLocation Loc = StringTokLocs.back(); for (unsigned I = 0, N = Lit->getLength(); I != N; ++I) { Value = Lit->getCodeUnit(I); TemplateArgument Arg(Context, Value, CharTy); - TemplateArgumentLocInfo ArgInfo; + TemplateArgumentLocInfo ArgInfo(Context, Loc.getLocWithOffset(I)); ExplicitArgs.addArgument(TemplateArgumentLoc(Arg, ArgInfo)); } - return BuildLiteralOperatorCall(R, OpNameInfo, {}, StringTokLocs.back(), - &ExplicitArgs); + return BuildLiteralOperatorCall(R, OpNameInfo, {}, Loc, &ExplicitArgs); } case LOLR_Raw: case LOLR_ErrorNoDiagnostic: @@ -3915,7 +3915,7 @@ ExprResult Sema::ActOnNumericConstant(const Token &Tok, Scope *UDLScope) { for (unsigned I = 0, N = Literal.getUDSuffixOffset(); I != N; ++I) { Value = TokSpelling[I]; TemplateArgument Arg(Context, Value, Context.CharTy); - TemplateArgumentLocInfo ArgInfo; + TemplateArgumentLocInfo ArgInfo(Context, TokLoc.getLocWithOffset(I)); ExplicitArgs.addArgument(TemplateArgumentLoc(Arg, ArgInfo)); } return BuildLiteralOperatorCall(R, OpNameInfo, {}, TokLoc, &ExplicitArgs); diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp index c3ff9f83be364..5d23660585ea5 100644 --- a/clang/lib/Sema/SemaTemplate.cpp +++ b/clang/lib/Sema/SemaTemplate.cpp @@ -5972,9 +5972,9 @@ bool Sema::CheckTemplateArgumentList( CTAI.CanonicalConverted, Params->getDepth())); } - ArgLoc = - TemplateArgumentLoc(TemplateArgument::CreatePackCopy(Context, Args), - ArgLoc.getLocInfo()); + ArgLoc = TemplateArgumentLoc( + TemplateArgument::CreatePackCopy(Context, Args), + TemplateArgumentLocInfo(Context, ArgLoc.getLocation())); } else { SaveAndRestore _1(CTAI.PartialOrdering, false); if (CheckTemplateArgument(*Param, ArgLoc, Template, TemplateLoc, diff --git a/clang/lib/Sema/SemaTemplateDeduction.cpp b/clang/lib/Sema/SemaTemplateDeduction.cpp index 265e5f4d2491d..f18a34415ba43 100644 --- a/clang/lib/Sema/SemaTemplateDeduction.cpp +++ b/clang/lib/Sema/SemaTemplateDeduction.cpp @@ -2908,7 +2908,7 @@ Sema::getTrivialTemplateArgumentLoc(const TemplateArgument &Arg, return TemplateArgumentLoc(Arg, Arg.getAsExpr()); case TemplateArgument::Pack: - return TemplateArgumentLoc(Arg, TemplateArgumentLocInfo()); + return TemplateArgumentLoc(Arg, TemplateArgumentLocInfo(Context, Loc)); } llvm_unreachable("Invalid TemplateArgument Kind!"); diff --git a/clang/test/SemaCXX/type_pack_element.cpp b/clang/test/SemaCXX/type_pack_element.cpp index d22d5fa2ba67c..0509358887af0 100644 --- a/clang/test/SemaCXX/type_pack_element.cpp +++ b/clang/test/SemaCXX/type_pack_element.cpp @@ -43,3 +43,10 @@ static_assert(__is_same(__type_pack_element<5, X<0>, X<1>, X<2>, X<3>, X<4>, X<5 template <SizeT Index, typename ...T> using ErrorTypePackElement1 = __type_pack_element<Index, T...>; // expected-error{{may not be accessed at an out of bounds index}} using illformed1 = ErrorTypePackElement1<3, X<0>, X<1>>; // expected-note{{in instantiation}} + +namespace GH186655 { + template <class {}, class C> struct S; + // expected-error@-1 {{cannot be defined in a type specifier}} + template <int T> struct S<T, __type_pack_element<sizeof(T)>> {}; + // expected-error@-1 {{a parameter pack may not be accessed at an out of bounds index}} +} // namespace GH186655 `````````` </details> https://github.com/llvm/llvm-project/pull/187352 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
