zyounan created this revision.
zyounan added reviewers: saar.raz, erichkeane, ilya-biryukov, aaron.ballman.
Herald added a subscriber: kadircet.
Herald added a project: All.
zyounan published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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
<https://reviews.llvm.org/D129499>.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D158061
Files:
clang/include/clang/AST/ExprConcepts.h
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/test/SemaCXX/concept-crash-on-diagnostic.cpp
clang/test/SemaCXX/concept-fatal-error.cpp
Index: clang/test/SemaCXX/concept-fatal-error.cpp
===================================================================
--- clang/test/SemaCXX/concept-fatal-error.cpp
+++ 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 @@
// 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}}
};
Index: clang/test/SemaCXX/concept-crash-on-diagnostic.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/concept-crash-on-diagnostic.cpp
@@ -0,0 +1,33 @@
+// 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; // #1
+
+// expected-error@#1 {{no member named 'value' in 'is_convertible<bool, bool>'}}
+
+template <typename From, typename To>
+concept convertible_to = is_convertible_v<From, To>; // expected-note 0+{{}}
+template <typename IteratorL, typename IteratorR>
+ requires requires(IteratorL lhs, IteratorR rhs) { // expected-note 0+{{}}
+ { lhs == rhs } -> convertible_to<bool>; // #2
+ }
+constexpr bool compare(normal_iterator<IteratorL> lhs, normal_iterator<IteratorR> rhs) {
+ return false;
+}
+
+// We don't know exactly the substituted type for `lhs == rhs`, thus a placeholder 'expr-type' is emitted.
+// expected-note@#2 {{'convertible_to<expr-type, bool>'}}
+
+// Consume remaining notes/errors.
+// expected-note@* 0+{{}}
+// expected-error@* 0+{{}}
+class Object;
+
+void function() {
+ normal_iterator<Object *> begin, end;
+ compare(begin, end);
+}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2276,9 +2276,9 @@
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()) {
@@ -2302,6 +2302,19 @@
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,
Index: clang/lib/Sema/SemaExprCXX.cpp
===================================================================
--- clang/lib/Sema/SemaExprCXX.cpp
+++ 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"
@@ -9076,12 +9077,18 @@
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(),
+ [IDC, this](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,
Index: clang/include/clang/AST/ExprConcepts.h
===================================================================
--- clang/include/clang/AST/ExprConcepts.h
+++ clang/include/clang/AST/ExprConcepts.h
@@ -24,6 +24,7 @@
#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>
@@ -467,6 +468,13 @@
}
};
+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]:
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
