[llvm-branch-commits] [clang] c2c9c0f - [clang] Construct ExprRequirement with SubstitutionDiagnostic on SubstFailure
Tobias Hieta via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Tue Sep 5 00:02:25 PDT 2023
Author: Younan Zhang
Date: 2023-09-05T09:00:27+02:00
New Revision: c2c9c0f1388e435c4b2416d658ea005d5e724202
URL: https://github.com/llvm/llvm-project/commit/c2c9c0f1388e435c4b2416d658ea005d5e724202
DIFF: https://github.com/llvm/llvm-project/commit/c2c9c0f1388e435c4b2416d658ea005d5e724202.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 76cc074dede7621..456c72451436951 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -852,6 +852,10 @@ Bug Fixes to C++ Support
- Update ``FunctionDeclBitfields.NumFunctionDeclBits``. This fixes:
(`#64171 <https://github.com/llvm/llvm-project/issues/64171>`_).
+- 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
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/AST/ExprConcepts.h b/clang/include/clang/AST/ExprConcepts.h
index d900e980852b45b..13d4568119eb208 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;
@@ -467,6 +468,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 08a025a3c800130..1cff4a75790ec74 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"
@@ -9074,16 +9075,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 8702e2ca3a1b3bb..394006a57747d5d 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2276,9 +2276,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()) {
@@ -2302,6 +2302,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 000000000000000..00a39f9f03b79cd
--- /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 c299b39fdeb2338..c606b9e21a36467 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}}
};
More information about the llvm-branch-commits
mailing list