[clang] e3d14be - [Concepts] Recover properly from a RecoveryExpr in a concept
Erich Keane via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 26 06:34:04 PDT 2022
Author: Erich Keane
Date: 2022-09-26T06:33:48-07:00
New Revision: e3d14bee238b672a7a112311eefee55e142eaefc
URL: https://github.com/llvm/llvm-project/commit/e3d14bee238b672a7a112311eefee55e142eaefc
DIFF: https://github.com/llvm/llvm-project/commit/e3d14bee238b672a7a112311eefee55e142eaefc.diff
LOG: [Concepts] Recover properly from a RecoveryExpr in a concept
Discovered by reducing a different problem, we currently assert because
we failed to make the constraint expressions not dependent, since a
RecoveryExpr cannot be transformed.
This patch fixes that, and gets reasonably nice diagnostics by
introducing a concept (hah!) of "ContainsErrors" to the Satisfaction
types, which causes us to treat the candidate as non-viable.
However, just making THAT candidate non-viable would result in choosing
the 'next best' canddiate, which can result in awkward errors, where we
start evaluating a candidate that is not intended to be selected.
Because of this, and to make diagnostics more relevant, we now just
cause the entire lookup to result in a 'no-viable-candidates'.
This means we will only emit the list of candidates, rather than any
cascading failures.
Added:
clang/test/SemaTemplate/concepts-recovery-expr.cpp
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/ASTConcept.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Overload.h
clang/lib/AST/ASTConcept.cpp
clang/lib/AST/ComputeDependence.cpp
clang/lib/Sema/SemaConcept.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a5e72fc917f38..5fe86f8bdecd5 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -212,6 +212,11 @@ Improvements to Clang's diagnostics
of FAM-like arrays.
- Clang now correctly diagnoses a warning when defercencing a void pointer in C mode.
This fixes `Issue 53631 <https://github.com/llvm/llvm-project/issues/53631>`_
+- Clang will now diagnose an overload set where a candidate has a constraint that
+ refers to an expression with a previous error as nothing viable, so that it
+ doesn't generate strange cascading errors, particularly in cases where a
+ subsuming constraint fails, which would result in a less-specific overload to
+ be selected.
Non-comprehensive list of changes in this release
-------------------------------------------------
diff --git a/clang/include/clang/AST/ASTConcept.h b/clang/include/clang/AST/ASTConcept.h
index 25e00860060f0..ae2f37dc4b321 100644
--- a/clang/include/clang/AST/ASTConcept.h
+++ b/clang/include/clang/AST/ASTConcept.h
@@ -44,6 +44,7 @@ class ConstraintSatisfaction : public llvm::FoldingSetNode {
using Detail = llvm::PointerUnion<Expr *, SubstitutionDiagnostic *>;
bool IsSatisfied = false;
+ bool ContainsErrors = false;
/// \brief Pairs of unsatisfied atomic constraint expressions along with the
/// substituted constraint expr, if the template arguments could be
@@ -78,6 +79,7 @@ struct ASTConstraintSatisfaction final :
UnsatisfiedConstraintRecord> {
std::size_t NumRecords;
bool IsSatisfied : 1;
+ bool ContainsErrors : 1;
const UnsatisfiedConstraintRecord *begin() const {
return getTrailingObjects<UnsatisfiedConstraintRecord>();
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 834b60a2744d6..93c598b2eba48 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2845,6 +2845,8 @@ def err_template_arg_list_constraints_not_satisfied : Error<
"template template parameter|template}0 %1%2">;
def note_substituted_constraint_expr_is_ill_formed : Note<
"because substituted constraint expression is ill-formed%0">;
+def note_constraint_references_error
+ : Note<"constraint depends on a previously diagnosed expression">;
def note_atomic_constraint_evaluated_to_false : Note<
"%select{and|because}0 '%1' evaluated to false">;
def note_concept_specialization_constraint_evaluated_to_false : Note<
diff --git a/clang/include/clang/Sema/Overload.h b/clang/include/clang/Sema/Overload.h
index fb4812675d9a3..6aaa6a481cf84 100644
--- a/clang/include/clang/Sema/Overload.h
+++ b/clang/include/clang/Sema/Overload.h
@@ -928,6 +928,8 @@ class Sema;
return ExplicitCallArguments;
}
+ bool NotValidBecauseConstraintExprHasError() const;
+
private:
friend class OverloadCandidateSet;
OverloadCandidate()
diff --git a/clang/lib/AST/ASTConcept.cpp b/clang/lib/AST/ASTConcept.cpp
index 18582782888c3..67911f7e82157 100644
--- a/clang/lib/AST/ASTConcept.cpp
+++ b/clang/lib/AST/ASTConcept.cpp
@@ -19,10 +19,11 @@
#include "llvm/ADT/FoldingSet.h"
using namespace clang;
-ASTConstraintSatisfaction::ASTConstraintSatisfaction(const ASTContext &C,
- const ConstraintSatisfaction &Satisfaction):
- NumRecords{Satisfaction.Details.size()},
- IsSatisfied{Satisfaction.IsSatisfied} {
+ASTConstraintSatisfaction::ASTConstraintSatisfaction(
+ const ASTContext &C, const ConstraintSatisfaction &Satisfaction)
+ : NumRecords{Satisfaction.Details.size()},
+ IsSatisfied{Satisfaction.IsSatisfied}, ContainsErrors{
+ Satisfaction.ContainsErrors} {
for (unsigned I = 0; I < NumRecords; ++I) {
auto &Detail = Satisfaction.Details[I];
if (Detail.second.is<Expr *>())
@@ -46,7 +47,6 @@ ASTConstraintSatisfaction::ASTConstraintSatisfaction(const ASTContext &C,
}
}
-
ASTConstraintSatisfaction *
ASTConstraintSatisfaction::Create(const ASTContext &C,
const ConstraintSatisfaction &Satisfaction) {
diff --git a/clang/lib/AST/ComputeDependence.cpp b/clang/lib/AST/ComputeDependence.cpp
index 1f573346b4410..334897b32403f 100644
--- a/clang/lib/AST/ComputeDependence.cpp
+++ b/clang/lib/AST/ComputeDependence.cpp
@@ -853,7 +853,10 @@ ExprDependence clang::computeDependence(ConceptSpecializationExpr *E,
ExprDependence D =
ValueDependent ? ExprDependence::Value : ExprDependence::None;
- return D | toExprDependence(TA);
+ auto Res = D | toExprDependence(TA);
+ if(!ValueDependent && E->getSatisfaction().ContainsErrors)
+ Res |= ExprDependence::Error;
+ return Res;
}
ExprDependence clang::computeDependence(ObjCArrayLiteral *E) {
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 614c9b8b05aa3..7426b553fcfb8 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -206,6 +206,30 @@ calculateConstraintSatisfaction(Sema &S, const Expr *ConstraintExpr,
// Evaluator has decided satisfaction without yielding an expression.
return ExprEmpty();
+ // We don't have the ability to evaluate this, since it contains a
+ // RecoveryExpr, so we want to fail overload resolution. Otherwise,
+ // we'd potentially pick up a
diff erent overload, and cause confusing
+ // diagnostics. SO, add a failure detail that will cause us to make this
+ // overload set not viable.
+ if (SubstitutedAtomicExpr.get()->containsErrors()) {
+ Satisfaction.IsSatisfied = false;
+ Satisfaction.ContainsErrors = true;
+
+ PartialDiagnostic Msg = S.PDiag(diag::note_constraint_references_error);
+ SmallString<128> DiagString;
+ DiagString = ": ";
+ Msg.EmitToString(S.getDiagnostics(), DiagString);
+ unsigned MessageSize = DiagString.size();
+ char *Mem = new (S.Context) char[MessageSize];
+ memcpy(Mem, DiagString.c_str(), MessageSize);
+ Satisfaction.Details.emplace_back(
+ ConstraintExpr,
+ new (S.Context) ConstraintSatisfaction::SubstitutionDiagnostic{
+ SubstitutedAtomicExpr.get()->getBeginLoc(),
+ StringRef(Mem, MessageSize)});
+ return SubstitutedAtomicExpr;
+ }
+
EnterExpressionEvaluationContext ConstantEvaluated(
S, Sema::ExpressionEvaluationContext::ConstantEvaluated);
SmallVector<PartialDiagnosticAt, 2> EvaluationDiags;
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 32ece5cc47918..02a05b2b7bcf0 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -10119,6 +10119,12 @@ void Sema::diagnoseEquivalentInternalLinkageDeclarations(
}
}
+bool OverloadCandidate::NotValidBecauseConstraintExprHasError() const {
+ return DeductionFailure.Result == Sema::TDK_ConstraintsNotSatisfied &&
+ static_cast<CNSInfo *>(DeductionFailure.Data)
+ ->Satisfaction.ContainsErrors;
+}
+
/// Computes the best viable function (C++ 13.3.3)
/// within an overload candidate set.
///
@@ -10171,10 +10177,18 @@ OverloadCandidateSet::BestViableFunction(Sema &S, SourceLocation Loc,
Best = end();
for (auto *Cand : Candidates) {
Cand->Best = false;
- if (Cand->Viable)
+ if (Cand->Viable) {
if (Best == end() ||
isBetterOverloadCandidate(S, *Cand, *Best, Loc, Kind))
Best = Cand;
+ } else if (Cand->NotValidBecauseConstraintExprHasError()) {
+ // This candidate has constraint that we were unable to evaluate because
+ // it referenced an expression that contained an error. Rather than fall
+ // back onto a potentially unintended candidate (made worse by by
+ // subsuming constraints), treat this as 'no viable candidate'.
+ Best = end();
+ return OR_No_Viable_Function;
+ }
}
// If we didn't find any viable functions, abort.
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
index f41b247c306f5..3f5a85786da4c 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -772,6 +772,7 @@ static ConstraintSatisfaction
readConstraintSatisfaction(ASTRecordReader &Record) {
ConstraintSatisfaction Satisfaction;
Satisfaction.IsSatisfied = Record.readInt();
+ Satisfaction.ContainsErrors = Record.readInt();
if (!Satisfaction.IsSatisfied) {
unsigned NumDetailRecords = Record.readInt();
for (unsigned i = 0; i != NumDetailRecords; ++i) {
diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp
index 5e5a86ee01a2b..aec3b7240ae99 100644
--- a/clang/lib/Serialization/ASTWriterStmt.cpp
+++ b/clang/lib/Serialization/ASTWriterStmt.cpp
@@ -402,6 +402,7 @@ static void
addConstraintSatisfaction(ASTRecordWriter &Record,
const ASTConstraintSatisfaction &Satisfaction) {
Record.push_back(Satisfaction.IsSatisfied);
+ Record.push_back(Satisfaction.ContainsErrors);
if (!Satisfaction.IsSatisfied) {
Record.push_back(Satisfaction.NumRecords);
for (const auto &DetailRecord : Satisfaction) {
diff --git a/clang/test/SemaTemplate/concepts-recovery-expr.cpp b/clang/test/SemaTemplate/concepts-recovery-expr.cpp
new file mode 100644
index 0000000000000..2f9d432ebac0e
--- /dev/null
+++ b/clang/test/SemaTemplate/concepts-recovery-expr.cpp
@@ -0,0 +1,182 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+// expected-error at +1{{use of undeclared identifier 'b'}}
+constexpr bool CausesRecoveryExpr = b;
+
+template<typename T>
+concept ReferencesCRE = CausesRecoveryExpr;
+
+template<typename T> requires CausesRecoveryExpr // #NVC1REQ
+void NoViableCands1(){} // #NVC1
+
+template<typename T> requires ReferencesCRE<T> // #NVC2REQ
+void NoViableCands2(){} // #NVC2
+
+template<ReferencesCRE T> // #NVC3REQ
+void NoViableCands3(){} // #NVC3
+
+void NVCUse() {
+ NoViableCands1<int>();
+ // expected-error at -1 {{no matching function for call to 'NoViableCands1'}}
+ // expected-note@#NVC1{{candidate template ignored: constraints not satisfied}}
+ // expected-note@#NVC1REQ{{because substituted constraint expression is ill-formed: constraint depends on a previously diagnosed expression}}
+
+ NoViableCands2<int>();
+ // expected-error at -1 {{no matching function for call to 'NoViableCands2'}}
+ // expected-note@#NVC2{{candidate template ignored: constraints not satisfied}}
+ // expected-note@#NVC2REQ{{because substituted constraint expression is ill-formed: constraint depends on a previously diagnosed expression}}
+ NoViableCands3<int>();
+ // expected-error at -1 {{no matching function for call to 'NoViableCands3'}}
+ // expected-note@#NVC3{{candidate template ignored: constraints not satisfied}}
+ // expected-note@#NVC3REQ{{because substituted constraint expression is ill-formed: constraint depends on a previously diagnosed expression}}
+}
+
+template<typename T> requires CausesRecoveryExpr // #OVC1REQ
+void OtherViableCands1(){} // #OVC1
+
+template<typename T>
+void OtherViableCands1(){} // #OVC1_ALT
+
+template<typename T> requires ReferencesCRE<T> // #OVC2REQ
+void OtherViableCands2(){} // #OVC2
+
+template<typename T>
+void OtherViableCands2(){} // #OVC2_ALT
+
+template<ReferencesCRE T> // #OVC3REQ
+void OtherViableCands3(){} // #OVC3
+template<typename T>
+void OtherViableCands3(){} // #OVC3_ALT
+
+void OVCUse() {
+ OtherViableCands1<int>();
+ // expected-error at -1 {{no matching function for call to 'OtherViableCands1'}}
+ // expected-note@#OVC1_ALT {{candidate function}}
+ // expected-note@#OVC1 {{candidate template ignored: constraints not satisfied}}
+ // expected-note@#OVC1REQ{{because substituted constraint expression is ill-formed: constraint depends on a previously diagnosed expression}}
+ OtherViableCands2<int>();
+ // expected-error at -1 {{no matching function for call to 'OtherViableCands2'}}
+ // expected-note@#OVC2_ALT {{candidate function}}
+ // expected-note@#OVC2 {{candidate template ignored: constraints not satisfied}}
+ // expected-note@#OVC2REQ{{because substituted constraint expression is ill-formed: constraint depends on a previously diagnosed expression}}
+ OtherViableCands3<int>();
+ // expected-error at -1 {{no matching function for call to 'OtherViableCands3'}}
+ // expected-note@#OVC3_ALT {{candidate function}}
+ // expected-note@#OVC3 {{candidate template ignored: constraints not satisfied}}
+ // expected-note@#OVC3REQ{{because substituted constraint expression is ill-formed: constraint depends on a previously diagnosed expression}}
+}
+
+template<typename T> requires CausesRecoveryExpr // #OBNVC1REQ
+void OtherBadNoViableCands1(){} // #OBNVC1
+
+template<typename T> requires false // #OBNVC1REQ_ALT
+void OtherBadNoViableCands1(){} // #OBNVC1_ALT
+
+template<typename T> requires ReferencesCRE<T> // #OBNVC2REQ
+void OtherBadNoViableCands2(){} // #OBNVC2
+
+template<typename T> requires false// #OBNVC2REQ_ALT
+void OtherBadNoViableCands2(){} // #OBNVC2_ALT
+
+template<ReferencesCRE T> // #OBNVC3REQ
+void OtherBadNoViableCands3(){} // #OBNVC3
+template<typename T> requires false // #OBNVC3REQ_ALT
+void OtherBadNoViableCands3(){} // #OBNVC3_ALT
+
+void OBNVCUse() {
+ OtherBadNoViableCands1<int>();
+ // expected-error at -1 {{no matching function for call to 'OtherBadNoViableCands1'}}
+ // expected-note@#OBNVC1_ALT {{candidate template ignored: constraints not satisfied}}
+ // expected-note@#OBNVC1REQ_ALT {{because 'false' evaluated to false}}
+ // expected-note@#OBNVC1 {{candidate template ignored: constraints not satisfied}}
+ // expected-note@#OBNVC1REQ{{because substituted constraint expression is ill-formed: constraint depends on a previously diagnosed expression}}
+ OtherBadNoViableCands2<int>();
+ // expected-error at -1 {{no matching function for call to 'OtherBadNoViableCands2'}}
+ // expected-note@#OBNVC2_ALT {{candidate template ignored: constraints not satisfied}}
+ // expected-note@#OBNVC2REQ_ALT {{because 'false' evaluated to false}}
+ // expected-note@#OBNVC2 {{candidate template ignored: constraints not satisfied}}
+ // expected-note@#OBNVC2REQ{{because substituted constraint expression is ill-formed: constraint depends on a previously diagnosed expression}}
+ OtherBadNoViableCands3<int>();
+ // expected-error at -1 {{no matching function for call to 'OtherBadNoViableCands3'}}
+ // expected-note@#OBNVC3_ALT {{candidate template ignored: constraints not satisfied}}
+ // expected-note@#OBNVC3REQ_ALT {{because 'false' evaluated to false}}
+ // expected-note@#OBNVC3 {{candidate template ignored: constraints not satisfied}}
+ // expected-note@#OBNVC3REQ{{because substituted constraint expression is ill-formed: constraint depends on a previously diagnosed expression}}
+}
+
+
+// Same tests with member functions.
+struct OVC {
+template<typename T> requires CausesRecoveryExpr // #MEMOVC1REQ
+void OtherViableCands1(){} // #MEMOVC1
+
+template<typename T>
+void OtherViableCands1(){} // #MEMOVC1_ALT
+
+template<typename T> requires ReferencesCRE<T> // #MEMOVC2REQ
+void OtherViableCands2(){} // #MEMOVC2
+
+template<typename T>
+void OtherViableCands2(){} // #MEMOVC2_ALT
+
+template<ReferencesCRE T> // #MEMOVC3REQ
+void OtherViableCands3(){} // #MEMOVC3
+template<typename T>
+void OtherViableCands3(){} // #MEMOVC3_ALT
+};
+
+void MemOVCUse() {
+ OVC S;
+ S.OtherViableCands1<int>();
+ // expected-error at -1 {{no matching member function for call to 'OtherViableCands1'}}
+ // expected-note@#MEMOVC1_ALT {{candidate function}}
+ // expected-note@#MEMOVC1 {{candidate template ignored: constraints not satisfied}}
+ // expected-note@#MEMOVC1REQ{{because substituted constraint expression is ill-formed: constraint depends on a previously diagnosed expression}}
+ S.OtherViableCands2<int>();
+ // expected-error at -1 {{no matching member function for call to 'OtherViableCands2'}}
+ // expected-note@#MEMOVC2_ALT {{candidate function}}
+ // expected-note@#MEMOVC2 {{candidate template ignored: constraints not satisfied}}
+ // expected-note@#MEMOVC2REQ{{because substituted constraint expression is ill-formed: constraint depends on a previously diagnosed expression}}
+ S.OtherViableCands3<int>();
+ // expected-error at -1 {{no matching member function for call to 'OtherViableCands3'}}
+ // expected-note@#MEMOVC3_ALT {{candidate function}}
+ // expected-note@#MEMOVC3 {{candidate template ignored: constraints not satisfied}}
+ // expected-note@#MEMOVC3REQ{{because substituted constraint expression is ill-formed: constraint depends on a previously diagnosed expression}}
+}
+
+struct StaticOVC {
+template<typename T> requires CausesRecoveryExpr // #SMEMOVC1REQ
+static void OtherViableCands1(){} // #SMEMOVC1
+
+template<typename T>
+static void OtherViableCands1(){} // #SMEMOVC1_ALT
+
+template<typename T> requires ReferencesCRE<T> // #SMEMOVC2REQ
+static void OtherViableCands2(){} // #SMEMOVC2
+
+template<typename T>
+static void OtherViableCands2(){} // #SMEMOVC2_ALT
+
+template<ReferencesCRE T> // #SMEMOVC3REQ
+static void OtherViableCands3(){} // #SMEMOVC3
+template<typename T>
+static void OtherViableCands3(){} // #SMEMOVC3_ALT
+};
+
+void StaticMemOVCUse() {
+ StaticOVC::OtherViableCands1<int>();
+ // expected-error at -1 {{no matching function for call to 'OtherViableCands1'}}
+ // expected-note@#SMEMOVC1_ALT {{candidate function}}
+ // expected-note@#SMEMOVC1 {{candidate template ignored: constraints not satisfied}}
+ // expected-note@#SMEMOVC1REQ{{because substituted constraint expression is ill-formed: constraint depends on a previously diagnosed expression}}
+ StaticOVC::OtherViableCands2<int>();
+ // expected-error at -1 {{no matching function for call to 'OtherViableCands2'}}
+ // expected-note@#SMEMOVC2_ALT {{candidate function}}
+ // expected-note@#SMEMOVC2 {{candidate template ignored: constraints not satisfied}}
+ // expected-note@#SMEMOVC2REQ{{because substituted constraint expression is ill-formed: constraint depends on a previously diagnosed expression}}
+ StaticOVC::OtherViableCands3<int>();
+ // expected-error at -1 {{no matching function for call to 'OtherViableCands3'}}
+ // expected-note@#SMEMOVC3_ALT {{candidate function}}
+ // expected-note@#SMEMOVC3 {{candidate template ignored: constraints not satisfied}}
+ // expected-note@#SMEMOVC3REQ{{because substituted constraint expression is ill-formed: constraint depends on a previously diagnosed expression}}
+}
More information about the cfe-commits
mailing list