Skip to content

Commit 684a789

Browse files
author
Erich Keane
committed
Reapply "[Concepts] Recover properly from a RecoveryExpr in a concept"
This reverts commit 192d69f. This fixes the condition to check whether this is a situation where we are in a recovery-expr'ed concept a little better, so we don't access an inactive member of a union, which should make the bots happy. Differential Revision: https://reviews.llvm.org/D134542
1 parent f7e1ce0 commit 684a789

File tree

11 files changed

+244
-7
lines changed

11 files changed

+244
-7
lines changed

‎clang/docs/ReleaseNotes.rst‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,11 @@ Improvements to Clang's diagnostics
212212
of FAM-like arrays.
213213
- Clang now correctly diagnoses a warning when defercencing a void pointer in C mode.
214214
This fixes `Issue 53631 <https://github.com/llvm/llvm-project/issues/53631>`_
215+
- Clang will now diagnose an overload set where a candidate has a constraint that
216+
refers to an expression with a previous error as nothing viable, so that it
217+
doesn't generate strange cascading errors, particularly in cases where a
218+
subsuming constraint fails, which would result in a less-specific overload to
219+
be selected.
215220

216221
Non-comprehensive list of changes in this release
217222
-------------------------------------------------

‎clang/include/clang/AST/ASTConcept.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ class ConstraintSatisfaction : public llvm::FoldingSetNode {
4444
using Detail = llvm::PointerUnion<Expr *, SubstitutionDiagnostic *>;
4545

4646
bool IsSatisfied = false;
47+
bool ContainsErrors = false;
4748

4849
/// \brief Pairs of unsatisfied atomic constraint expressions along with the
4950
/// substituted constraint expr, if the template arguments could be
@@ -78,6 +79,7 @@ struct ASTConstraintSatisfaction final :
7879
UnsatisfiedConstraintRecord> {
7980
std::size_t NumRecords;
8081
bool IsSatisfied : 1;
82+
bool ContainsErrors : 1;
8183

8284
const UnsatisfiedConstraintRecord *begin() const {
8385
return getTrailingObjects<UnsatisfiedConstraintRecord>();

‎clang/include/clang/Basic/DiagnosticSemaKinds.td‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2845,6 +2845,8 @@ def err_template_arg_list_constraints_not_satisfied : Error<
28452845
"template template parameter|template}0 %1%2">;
28462846
def note_substituted_constraint_expr_is_ill_formed : Note<
28472847
"because substituted constraint expression is ill-formed%0">;
2848+
def note_constraint_references_error
2849+
: Note<"constraint depends on a previously diagnosed expression">;
28482850
def note_atomic_constraint_evaluated_to_false : Note<
28492851
"%select{and|because}0 '%1' evaluated to false">;
28502852
def note_concept_specialization_constraint_evaluated_to_false : Note<

‎clang/include/clang/Sema/Overload.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -928,6 +928,8 @@ class Sema;
928928
return ExplicitCallArguments;
929929
}
930930

931+
bool NotValidBecauseConstraintExprHasError() const;
932+
931933
private:
932934
friend class OverloadCandidateSet;
933935
OverloadCandidate()

‎clang/lib/AST/ASTConcept.cpp‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,11 @@
1919
#include "llvm/ADT/FoldingSet.h"
2020
using namespace clang;
2121

22-
ASTConstraintSatisfaction::ASTConstraintSatisfaction(const ASTContext &C,
23-
const ConstraintSatisfaction &Satisfaction):
24-
NumRecords{Satisfaction.Details.size()},
25-
IsSatisfied{Satisfaction.IsSatisfied} {
22+
ASTConstraintSatisfaction::ASTConstraintSatisfaction(
23+
const ASTContext &C, const ConstraintSatisfaction &Satisfaction)
24+
: NumRecords{Satisfaction.Details.size()},
25+
IsSatisfied{Satisfaction.IsSatisfied}, ContainsErrors{
26+
Satisfaction.ContainsErrors} {
2627
for (unsigned I = 0; I < NumRecords; ++I) {
2728
auto &Detail = Satisfaction.Details[I];
2829
if (Detail.second.is<Expr *>())
@@ -46,7 +47,6 @@ ASTConstraintSatisfaction::ASTConstraintSatisfaction(const ASTContext &C,
4647
}
4748
}
4849

49-
5050
ASTConstraintSatisfaction *
5151
ASTConstraintSatisfaction::Create(const ASTContext &C,
5252
const ConstraintSatisfaction &Satisfaction) {

‎clang/lib/AST/ComputeDependence.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -853,7 +853,10 @@ ExprDependence clang::computeDependence(ConceptSpecializationExpr *E,
853853

854854
ExprDependence D =
855855
ValueDependent ? ExprDependence::Value : ExprDependence::None;
856-
return D | toExprDependence(TA);
856+
auto Res = D | toExprDependence(TA);
857+
if(!ValueDependent && E->getSatisfaction().ContainsErrors)
858+
Res |= ExprDependence::Error;
859+
return Res;
857860
}
858861

859862
ExprDependence clang::computeDependence(ObjCArrayLiteral *E) {

‎clang/lib/Sema/SemaConcept.cpp‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,30 @@ calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr,
206206
// Evaluator has decided satisfaction without yielding an expression.
207207
return ExprEmpty();
208208

209+
// We don't have the ability to evaluate this, since it contains a
210+
// RecoveryExpr, so we want to fail overload resolution. Otherwise,
211+
// we'd potentially pick up a different overload, and cause confusing
212+
// diagnostics. SO, add a failure detail that will cause us to make this
213+
// overload set not viable.
214+
if (SubstitutedAtomicExpr.get()->containsErrors()) {
215+
Satisfaction.IsSatisfied = false;
216+
Satisfaction.ContainsErrors = true;
217+
218+
PartialDiagnostic Msg = S.PDiag(diag::note_constraint_references_error);
219+
SmallString<128> DiagString;
220+
DiagString = ": ";
221+
Msg.EmitToString(S.getDiagnostics(), DiagString);
222+
unsigned MessageSize = DiagString.size();
223+
char *Mem = new (S.Context) char[MessageSize];
224+
memcpy(Mem, DiagString.c_str(), MessageSize);
225+
Satisfaction.Details.emplace_back(
226+
ConstraintExpr,
227+
new (S.Context) ConstraintSatisfaction::SubstitutionDiagnostic{
228+
SubstitutedAtomicExpr.get()->getBeginLoc(),
229+
StringRef(Mem, MessageSize)});
230+
return SubstitutedAtomicExpr;
231+
}
232+
209233
EnterExpressionEvaluationContext ConstantEvaluated(
210234
S, Sema::ExpressionEvaluationContext::ConstantEvaluated);
211235
SmallVector<PartialDiagnosticAt, 2> EvaluationDiags;

‎clang/lib/Sema/SemaOverload.cpp‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10119,6 +10119,13 @@ void Sema::diagnoseEquivalentInternalLinkageDeclarations(
1011910119
}
1012010120
}
1012110121

10122+
bool OverloadCandidate::NotValidBecauseConstraintExprHasError() const {
10123+
return FailureKind == ovl_fail_bad_deduction &&
10124+
DeductionFailure.Result == Sema::TDK_ConstraintsNotSatisfied &&
10125+
static_cast<CNSInfo *>(DeductionFailure.Data)
10126+
->Satisfaction.ContainsErrors;
10127+
}
10128+
1012210129
/// Computes the best viable function (C++ 13.3.3)
1012310130
/// within an overload candidate set.
1012410131
///
@@ -10171,10 +10178,18 @@ OverloadCandidateSet::BestViableFunction(Sema &S, SourceLocation Loc,
1017110178
Best = end();
1017210179
for (auto *Cand : Candidates) {
1017310180
Cand->Best = false;
10174-
if (Cand->Viable)
10181+
if (Cand->Viable) {
1017510182
if (Best == end() ||
1017610183
isBetterOverloadCandidate(S, *Cand, *Best, Loc, Kind))
1017710184
Best = Cand;
10185+
} else if (Cand->NotValidBecauseConstraintExprHasError()) {
10186+
// This candidate has constraint that we were unable to evaluate because
10187+
// it referenced an expression that contained an error. Rather than fall
10188+
// back onto a potentially unintended candidate (made worse by by
10189+
// subsuming constraints), treat this as 'no viable candidate'.
10190+
Best = end();
10191+
return OR_No_Viable_Function;
10192+
}
1017810193
}
1017910194

1018010195
// If we didn't find any viable functions, abort.

‎clang/lib/Serialization/ASTReaderStmt.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -772,6 +772,7 @@ static ConstraintSatisfaction
772772
readConstraintSatisfaction(ASTRecordReader &Record) {
773773
ConstraintSatisfaction Satisfaction;
774774
Satisfaction.IsSatisfied = Record.readInt();
775+
Satisfaction.ContainsErrors = Record.readInt();
775776
if (!Satisfaction.IsSatisfied) {
776777
unsigned NumDetailRecords = Record.readInt();
777778
for (unsigned i = 0; i != NumDetailRecords; ++i) {

‎clang/lib/Serialization/ASTWriterStmt.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,7 @@ static void
402402
addConstraintSatisfaction(ASTRecordWriter &Record,
403403
const ASTConstraintSatisfaction &Satisfaction) {
404404
Record.push_back(Satisfaction.IsSatisfied);
405+
Record.push_back(Satisfaction.ContainsErrors);
405406
if (!Satisfaction.IsSatisfied) {
406407
Record.push_back(Satisfaction.NumRecords);
407408
for (const auto &DetailRecord : Satisfaction) {

0 commit comments

Comments
 (0)