Author: Younan Zhang Date: 2023-09-01T15:49:39+08:00 New Revision: 2fd01d75a863184766ee0c82b5c0fc8be172448a
URL: https://github.com/llvm/llvm-project/commit/2fd01d75a863184766ee0c82b5c0fc8be172448a DIFF: https://github.com/llvm/llvm-project/commit/2fd01d75a863184766ee0c82b5c0fc8be172448a.diff LOG: [clang] Construct ExprRequirement with SubstitutionDiagnostic on SubstFailure We're expecting a SubstitutionDiagnostic in diagnoseUnsatisfiedRequirement if the status of ExprRequirement is SubstFailure. Previously, the Requirement was created with Expr on SubstFailure by mistake, which could lead to the assertion failure in the subsequent diagnosis. Fixes https://github.com/clangd/clangd/issues/1726 Fixes https://github.com/llvm/llvm-project/issues/64723 Fixes https://github.com/llvm/llvm-project/issues/64172 In addition, this patch also fixes an invalid test from D129499. Reviewed By: erichkeane Differential Revision: https://reviews.llvm.org/D158061 Added: clang/test/SemaCXX/concept-crash-on-diagnostic.cpp Modified: clang/docs/ReleaseNotes.rst clang/include/clang/AST/ExprConcepts.h clang/lib/Sema/SemaExprCXX.cpp clang/lib/Sema/SemaTemplateInstantiate.cpp clang/test/SemaCXX/concept-fatal-error.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 15d318ef75f05a..fc96ccb992ed78 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -250,6 +250,10 @@ Bug Fixes to C++ Support (`#64962 <https://github.com/llvm/llvm-project/issues/64962>`_) and (`#28679 <https://github.com/llvm/llvm-project/issues/28679>`_). +- Fix a crash caused by substitution failure in expression requirements. + (`#64172 <https://github.com/llvm/llvm-project/issues/64172>`_) and + (`#64723 <https://github.com/llvm/llvm-project/issues/64723>`_). + Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ - Fixed an import failure of recursive friend class template. diff --git a/clang/include/clang/AST/ExprConcepts.h b/clang/include/clang/AST/ExprConcepts.h index 6449f1b56821f0..6f38b2c4b05714 100644 --- a/clang/include/clang/AST/ExprConcepts.h +++ b/clang/include/clang/AST/ExprConcepts.h @@ -14,20 +14,21 @@ #ifndef LLVM_CLANG_AST_EXPRCONCEPTS_H #define LLVM_CLANG_AST_EXPRCONCEPTS_H -#include "clang/AST/ASTContext.h" #include "clang/AST/ASTConcept.h" +#include "clang/AST/ASTContext.h" #include "clang/AST/Decl.h" -#include "clang/AST/DeclarationName.h" #include "clang/AST/DeclTemplate.h" +#include "clang/AST/DeclarationName.h" #include "clang/AST/Expr.h" #include "clang/AST/NestedNameSpecifier.h" #include "clang/AST/TemplateBase.h" #include "clang/AST/Type.h" #include "clang/Basic/SourceLocation.h" +#include "llvm/ADT/STLFunctionalExtras.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/TrailingObjects.h" -#include <utility> #include <string> +#include <utility> namespace clang { class ASTStmtReader; @@ -486,6 +487,13 @@ class NestedRequirement : public Requirement { } }; +using EntityPrinter = llvm::function_ref<void(llvm::raw_ostream &)>; + +/// \brief create a Requirement::SubstitutionDiagnostic with only a +/// SubstitutedEntity and DiagLoc using Sema's allocator. +Requirement::SubstitutionDiagnostic * +createSubstDiagAt(Sema &S, SourceLocation Location, EntityPrinter Printer); + } // namespace concepts /// C++2a [expr.prim.req]: diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index 472fbdbdb5d0e6..833223f7968944 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -19,6 +19,7 @@ #include "clang/AST/CharUnits.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/ExprCXX.h" +#include "clang/AST/ExprConcepts.h" #include "clang/AST/ExprObjC.h" #include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/Type.h" @@ -9080,16 +9081,22 @@ Sema::BuildExprRequirement( MLTAL.addOuterRetainedLevels(TPL->getDepth()); const TypeConstraint *TC = Param->getTypeConstraint(); assert(TC && "Type Constraint cannot be null here"); - ExprResult Constraint = - SubstExpr(TC->getImmediatelyDeclaredConstraint(), MLTAL); + auto *IDC = TC->getImmediatelyDeclaredConstraint(); + assert(IDC && "ImmediatelyDeclaredConstraint can't be null here."); + ExprResult Constraint = SubstExpr(IDC, MLTAL); if (Constraint.isInvalid()) { - Status = concepts::ExprRequirement::SS_ExprSubstitutionFailure; - } else { - SubstitutedConstraintExpr = - cast<ConceptSpecializationExpr>(Constraint.get()); - if (!SubstitutedConstraintExpr->isSatisfied()) - Status = concepts::ExprRequirement::SS_ConstraintsNotSatisfied; - } + return new (Context) concepts::ExprRequirement( + concepts::createSubstDiagAt(*this, IDC->getExprLoc(), + [&](llvm::raw_ostream &OS) { + IDC->printPretty(OS, /*Helper=*/nullptr, + getPrintingPolicy()); + }), + IsSimple, NoexceptLoc, ReturnTypeRequirement); + } + SubstitutedConstraintExpr = + cast<ConceptSpecializationExpr>(Constraint.get()); + if (!SubstitutedConstraintExpr->isSatisfied()) + Status = concepts::ExprRequirement::SS_ConstraintsNotSatisfied; } return new (Context) concepts::ExprRequirement(E, IsSimple, NoexceptLoc, ReturnTypeRequirement, Status, diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp index 810e0322f0a3bc..3b1731edec9523 100644 --- a/clang/lib/Sema/SemaTemplateInstantiate.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp @@ -2271,9 +2271,9 @@ QualType TemplateInstantiator::TransformSubstTemplateTypeParmPackType( getPackIndex(Pack), Arg, TL.getNameLoc()); } -template<typename EntityPrinter> static concepts::Requirement::SubstitutionDiagnostic * -createSubstDiag(Sema &S, TemplateDeductionInfo &Info, EntityPrinter Printer) { +createSubstDiag(Sema &S, TemplateDeductionInfo &Info, + concepts::EntityPrinter Printer) { SmallString<128> Message; SourceLocation ErrorLoc; if (Info.hasSFINAEDiagnostic()) { @@ -2297,6 +2297,19 @@ createSubstDiag(Sema &S, TemplateDeductionInfo &Info, EntityPrinter Printer) { StringRef(MessageBuf, Message.size())}; } +concepts::Requirement::SubstitutionDiagnostic * +concepts::createSubstDiagAt(Sema &S, SourceLocation Location, + EntityPrinter Printer) { + SmallString<128> Entity; + llvm::raw_svector_ostream OS(Entity); + Printer(OS); + char *EntityBuf = new (S.Context) char[Entity.size()]; + llvm::copy(Entity, EntityBuf); + return new (S.Context) concepts::Requirement::SubstitutionDiagnostic{ + /*SubstitutedEntity=*/StringRef(EntityBuf, Entity.size()), + /*DiagLoc=*/Location, /*DiagMessage=*/StringRef()}; +} + ExprResult TemplateInstantiator::TransformRequiresTypeParams( SourceLocation KWLoc, SourceLocation RBraceLoc, const RequiresExpr *RE, RequiresExprBodyDecl *Body, ArrayRef<ParmVarDecl *> Params, diff --git a/clang/test/SemaCXX/concept-crash-on-diagnostic.cpp b/clang/test/SemaCXX/concept-crash-on-diagnostic.cpp new file mode 100644 index 00000000000000..00a39f9f03b79c --- /dev/null +++ b/clang/test/SemaCXX/concept-crash-on-diagnostic.cpp @@ -0,0 +1,37 @@ +// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s + +template <typename Iterator> class normal_iterator {}; + +template <typename From, typename To> struct is_convertible {}; + +template <typename From, typename To> +inline constexpr bool is_convertible_v = is_convertible<From, To>::value; // expected-error {{no member named 'value' in 'is_convertible<bool, bool>'}} + +template <typename From, typename To> +concept convertible_to = is_convertible_v<From, To>; // #1 + +template <typename IteratorL, typename IteratorR> + requires requires(IteratorL lhs, IteratorR rhs) { // #2 + { lhs == rhs } -> convertible_to<bool>; // #3 + } +constexpr bool compare(normal_iterator<IteratorL> lhs, normal_iterator<IteratorR> rhs) { // #4 + return false; +} + +class Object; + +void function() { + normal_iterator<Object *> begin, end; + compare(begin, end); // expected-error {{no matching function for call to 'compare'}} #5 +} + +// expected-note@#1 {{in instantiation of variable template specialization 'is_convertible_v<bool, bool>' requested here}} +// expected-note@#1 {{substituting template arguments into constraint expression here}} +// expected-note@#3 {{checking the satisfaction of concept 'convertible_to<bool, bool>'}} +// expected-note@#2 {{substituting template arguments into constraint expression here}} +// expected-note@#5 {{checking constraint satisfaction for template 'compare<Object *, Object *>'}} +// expected-note@#5 {{in instantiation of function template specialization 'compare<Object *, Object *>' requested here}} + +// expected-note@#4 {{candidate template ignored: constraints not satisfied [with IteratorL = Object *, IteratorR = Object *]}} +// We don't know exactly the substituted type for `lhs == rhs`, thus a placeholder 'expr-type' is emitted. +// expected-note@#3 {{because 'convertible_to<expr-type, bool>' would be invalid}} diff --git a/clang/test/SemaCXX/concept-fatal-error.cpp b/clang/test/SemaCXX/concept-fatal-error.cpp index c299b39fdeb233..c606b9e21a3646 100644 --- a/clang/test/SemaCXX/concept-fatal-error.cpp +++ b/clang/test/SemaCXX/concept-fatal-error.cpp @@ -1,4 +1,4 @@ -// RUN: not %clang_cc1 -fsyntax-only -std=c++20 -ferror-limit 1 -verify %s +// RUN: %clang_cc1 -fsyntax-only -std=c++20 -ferror-limit 1 -verify %s template <class> concept f = requires { 42; }; @@ -6,5 +6,5 @@ struct h { // The missing semicolon will trigger an error and -ferror-limit=1 will make it fatal // We test that we do not crash in such cases (#55401) int i = requires { { i } f } // expected-error {{expected ';' at end of declaration list}} - // expected-error@* {{too many errros emitted}} + // expected-error@* {{too many errors emitted}} }; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits