Skip to content

Commit c2c9c0f

Browse files
zyn0217tru
authored andcommitted
[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 clangd/clangd#1726 Fixes llvm#64723 Fixes llvm#64172 In addition, this patch also fixes an invalid test from D129499. Reviewed By: erichkeane Differential Revision: https://reviews.llvm.org/D158061
1 parent 175a130 commit c2c9c0f

File tree

6 files changed

+85
-16
lines changed

6 files changed

+85
-16
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -852,6 +852,10 @@ Bug Fixes to C++ Support
852852
- Update ``FunctionDeclBitfields.NumFunctionDeclBits``. This fixes:
853853
(`#64171 <https://github.com/llvm/llvm-project/issues/64171>`_).
854854

855+
- Fix a crash caused by substitution failure in expression requirements.
856+
(`#64172 <https://github.com/llvm/llvm-project/issues/64172>`_) and
857+
(`#64723 <https://github.com/llvm/llvm-project/issues/64723>`_).
858+
855859
Bug Fixes to AST Handling
856860
^^^^^^^^^^^^^^^^^^^^^^^^^
857861

clang/include/clang/AST/ExprConcepts.h

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,21 @@
1414
#ifndef LLVM_CLANG_AST_EXPRCONCEPTS_H
1515
#define LLVM_CLANG_AST_EXPRCONCEPTS_H
1616

17-
#include "clang/AST/ASTContext.h"
1817
#include "clang/AST/ASTConcept.h"
18+
#include "clang/AST/ASTContext.h"
1919
#include "clang/AST/Decl.h"
20-
#include "clang/AST/DeclarationName.h"
2120
#include "clang/AST/DeclTemplate.h"
21+
#include "clang/AST/DeclarationName.h"
2222
#include "clang/AST/Expr.h"
2323
#include "clang/AST/NestedNameSpecifier.h"
2424
#include "clang/AST/TemplateBase.h"
2525
#include "clang/AST/Type.h"
2626
#include "clang/Basic/SourceLocation.h"
27+
#include "llvm/ADT/STLFunctionalExtras.h"
2728
#include "llvm/Support/ErrorHandling.h"
2829
#include "llvm/Support/TrailingObjects.h"
29-
#include <utility>
3030
#include <string>
31+
#include <utility>
3132

3233
namespace clang {
3334
class ASTStmtReader;
@@ -467,6 +468,13 @@ class NestedRequirement : public Requirement {
467468
}
468469
};
469470

471+
using EntityPrinter = llvm::function_ref<void(llvm::raw_ostream &)>;
472+
473+
/// \brief create a Requirement::SubstitutionDiagnostic with only a
474+
/// SubstitutedEntity and DiagLoc using Sema's allocator.
475+
Requirement::SubstitutionDiagnostic *
476+
createSubstDiagAt(Sema &S, SourceLocation Location, EntityPrinter Printer);
477+
470478
} // namespace concepts
471479

472480
/// C++2a [expr.prim.req]:

clang/lib/Sema/SemaExprCXX.cpp

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "clang/AST/CharUnits.h"
2020
#include "clang/AST/DeclObjC.h"
2121
#include "clang/AST/ExprCXX.h"
22+
#include "clang/AST/ExprConcepts.h"
2223
#include "clang/AST/ExprObjC.h"
2324
#include "clang/AST/RecursiveASTVisitor.h"
2425
#include "clang/AST/Type.h"
@@ -9074,16 +9075,22 @@ Sema::BuildExprRequirement(
90749075
MLTAL.addOuterRetainedLevels(TPL->getDepth());
90759076
const TypeConstraint *TC = Param->getTypeConstraint();
90769077
assert(TC && "Type Constraint cannot be null here");
9077-
ExprResult Constraint =
9078-
SubstExpr(TC->getImmediatelyDeclaredConstraint(), MLTAL);
9078+
auto *IDC = TC->getImmediatelyDeclaredConstraint();
9079+
assert(IDC && "ImmediatelyDeclaredConstraint can't be null here.");
9080+
ExprResult Constraint = SubstExpr(IDC, MLTAL);
90799081
if (Constraint.isInvalid()) {
9080-
Status = concepts::ExprRequirement::SS_ExprSubstitutionFailure;
9081-
} else {
9082-
SubstitutedConstraintExpr =
9083-
cast<ConceptSpecializationExpr>(Constraint.get());
9084-
if (!SubstitutedConstraintExpr->isSatisfied())
9085-
Status = concepts::ExprRequirement::SS_ConstraintsNotSatisfied;
9086-
}
9082+
return new (Context) concepts::ExprRequirement(
9083+
concepts::createSubstDiagAt(*this, IDC->getExprLoc(),
9084+
[&](llvm::raw_ostream &OS) {
9085+
IDC->printPretty(OS, /*Helper=*/nullptr,
9086+
getPrintingPolicy());
9087+
}),
9088+
IsSimple, NoexceptLoc, ReturnTypeRequirement);
9089+
}
9090+
SubstitutedConstraintExpr =
9091+
cast<ConceptSpecializationExpr>(Constraint.get());
9092+
if (!SubstitutedConstraintExpr->isSatisfied())
9093+
Status = concepts::ExprRequirement::SS_ConstraintsNotSatisfied;
90879094
}
90889095
return new (Context) concepts::ExprRequirement(E, IsSimple, NoexceptLoc,
90899096
ReturnTypeRequirement, Status,

clang/lib/Sema/SemaTemplateInstantiate.cpp

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2276,9 +2276,9 @@ QualType TemplateInstantiator::TransformSubstTemplateTypeParmPackType(
22762276
getPackIndex(Pack), Arg, TL.getNameLoc());
22772277
}
22782278

2279-
template<typename EntityPrinter>
22802279
static concepts::Requirement::SubstitutionDiagnostic *
2281-
createSubstDiag(Sema &S, TemplateDeductionInfo &Info, EntityPrinter Printer) {
2280+
createSubstDiag(Sema &S, TemplateDeductionInfo &Info,
2281+
concepts::EntityPrinter Printer) {
22822282
SmallString<128> Message;
22832283
SourceLocation ErrorLoc;
22842284
if (Info.hasSFINAEDiagnostic()) {
@@ -2302,6 +2302,19 @@ createSubstDiag(Sema &S, TemplateDeductionInfo &Info, EntityPrinter Printer) {
23022302
StringRef(MessageBuf, Message.size())};
23032303
}
23042304

2305+
concepts::Requirement::SubstitutionDiagnostic *
2306+
concepts::createSubstDiagAt(Sema &S, SourceLocation Location,
2307+
EntityPrinter Printer) {
2308+
SmallString<128> Entity;
2309+
llvm::raw_svector_ostream OS(Entity);
2310+
Printer(OS);
2311+
char *EntityBuf = new (S.Context) char[Entity.size()];
2312+
llvm::copy(Entity, EntityBuf);
2313+
return new (S.Context) concepts::Requirement::SubstitutionDiagnostic{
2314+
/*SubstitutedEntity=*/StringRef(EntityBuf, Entity.size()),
2315+
/*DiagLoc=*/Location, /*DiagMessage=*/StringRef()};
2316+
}
2317+
23052318
ExprResult TemplateInstantiator::TransformRequiresTypeParams(
23062319
SourceLocation KWLoc, SourceLocation RBraceLoc, const RequiresExpr *RE,
23072320
RequiresExprBodyDecl *Body, ArrayRef<ParmVarDecl *> Params,
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
// RUN: %clang_cc1 -fsyntax-only -std=c++20 -verify %s
2+
3+
template <typename Iterator> class normal_iterator {};
4+
5+
template <typename From, typename To> struct is_convertible {};
6+
7+
template <typename From, typename To>
8+
inline constexpr bool is_convertible_v = is_convertible<From, To>::value; // expected-error {{no member named 'value' in 'is_convertible<bool, bool>'}}
9+
10+
template <typename From, typename To>
11+
concept convertible_to = is_convertible_v<From, To>; // #1
12+
13+
template <typename IteratorL, typename IteratorR>
14+
requires requires(IteratorL lhs, IteratorR rhs) { // #2
15+
{ lhs == rhs } -> convertible_to<bool>; // #3
16+
}
17+
constexpr bool compare(normal_iterator<IteratorL> lhs, normal_iterator<IteratorR> rhs) { // #4
18+
return false;
19+
}
20+
21+
class Object;
22+
23+
void function() {
24+
normal_iterator<Object *> begin, end;
25+
compare(begin, end); // expected-error {{no matching function for call to 'compare'}} #5
26+
}
27+
28+
// expected-note@#1 {{in instantiation of variable template specialization 'is_convertible_v<bool, bool>' requested here}}
29+
// expected-note@#1 {{substituting template arguments into constraint expression here}}
30+
// expected-note@#3 {{checking the satisfaction of concept 'convertible_to<bool, bool>'}}
31+
// expected-note@#2 {{substituting template arguments into constraint expression here}}
32+
// expected-note@#5 {{checking constraint satisfaction for template 'compare<Object *, Object *>'}}
33+
// expected-note@#5 {{in instantiation of function template specialization 'compare<Object *, Object *>' requested here}}
34+
35+
// expected-note@#4 {{candidate template ignored: constraints not satisfied [with IteratorL = Object *, IteratorR = Object *]}}
36+
// We don't know exactly the substituted type for `lhs == rhs`, thus a placeholder 'expr-type' is emitted.
37+
// expected-note@#3 {{because 'convertible_to<expr-type, bool>' would be invalid}}
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
1-
// RUN: not %clang_cc1 -fsyntax-only -std=c++20 -ferror-limit 1 -verify %s
1+
// RUN: %clang_cc1 -fsyntax-only -std=c++20 -ferror-limit 1 -verify %s
22

33
template <class>
44
concept f = requires { 42; };
55
struct h {
66
// The missing semicolon will trigger an error and -ferror-limit=1 will make it fatal
77
// We test that we do not crash in such cases (#55401)
88
int i = requires { { i } f } // expected-error {{expected ';' at end of declaration list}}
9-
// expected-error@* {{too many errros emitted}}
9+
// expected-error@* {{too many errors emitted}}
1010
};

0 commit comments

Comments
 (0)