usaxena95 updated this revision to Diff 475054.
usaxena95 marked 3 inline comments as done.
usaxena95 added a comment.
Addressed commented. Added Release notes.
Removed infrastructure changes and deferred them to a future patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D137712/new/
https://reviews.llvm.org/D137712
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/ExprConcepts.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/ComputeDependence.cpp
clang/lib/AST/ExprConcepts.cpp
clang/lib/AST/StmtPrinter.cpp
clang/lib/Sema/SemaConcept.cpp
clang/lib/Sema/TreeTransform.h
clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp
Index: clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp
===================================================================
--- clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp
+++ clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp
@@ -51,3 +51,80 @@
X.next();
};
+
+namespace SubstitutionFailureNestedRequires {
+template<class T> concept True = true;
+
+struct S { double value; };
+
+template <class T>
+concept Pipes = requires (T x) {
+ requires True<decltype(x.value)> || True<T>;
+ requires True<T> || True<decltype(x.value)>;
+};
+
+template <class T>
+concept Amps1 = requires (T x) {
+ requires True<decltype(x.value)> && True<T>;
+ // expected-note@-1{{because 'member reference base type 'int' is not a structure or union'}}
+};
+template <class T>
+concept Amps2 = requires (T x) {
+ requires True<T> && True<decltype(x.value)>;
+};
+
+static_assert(Pipes<S>);
+static_assert(Pipes<double>);
+
+static_assert(Amps1<S>);
+static_assert(!Amps1<double>);
+
+static_assert(Amps2<S>);
+static_assert(!Amps2<double>);
+
+template<class T>
+void foo1() requires requires (T x) { // expected-note {{candidate template ignored: constraints not satisfied [with T = int]}}
+ requires
+ True<decltype(x.value)> // expected-note {{because 'member reference base type 'int' is not a structure or union'}}
+ && True<T>;
+} {}
+template<class T> void fooPipes() requires Pipes<T> {}
+template<class T> void fooAmps1() requires Amps1<T> {}
+// expected-note@-1 {{candidate template ignored: constraints not satisfied [with T = int]}} \
+// expected-note@-1 {{because 'int' does not satisfy 'Amps1'}}
+
+void foo() {
+ foo1<S>();
+ foo1<int>(); // expected-error {{no matching function for call to 'foo1'}}
+ fooPipes<S>();
+ fooPipes<int>();
+ fooAmps1<S>();
+ fooAmps1<int>(); // expected-error {{no matching function for call to 'fooAmps1'}}
+}
+
+template<class T>
+concept HasNoValue = requires (T x) {
+ requires !True<decltype(x.value)> && True<T>;
+};
+static_assert(HasNoValue<int>);
+static_assert(!HasNoValue<S>);
+
+template<class T> constexpr bool NotAConceptTrue = true;
+template <class T>
+concept SFinNestedRequires = requires (T x) {
+ // Only SF in a concept specialisation should be evaluated to false.
+ // Rest is still a SF.
+ requires NotAConceptTrue<decltype(x.value)> || NotAConceptTrue<T>;
+ // expected-note@-1 {{because 'NotAConceptTrue<decltype(x.value)> || NotAConceptTrue<T>' would be invalid: member reference base type 'int' is not a structure or union}}
+};
+static_assert(!SFinNestedRequires<int>); // Unsatisfied but not a hard error.
+static_assert(SFinNestedRequires<S>);
+template <class T>
+void foo() requires SFinNestedRequires<T> {}
+// expected-note@-1 {{candidate template ignored: constraints not satisfied [with T = int]}}
+// expected-note@-2 {{because 'int' does not satisfy 'SFinNestedRequires'}}
+void bar() {
+ foo<int>(); // expected-error {{no matching function for call to 'foo'}}
+ foo<S>();
+}
+}
Index: clang/lib/Sema/TreeTransform.h
===================================================================
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -15,6 +15,7 @@
#include "CoroutineStmtBuilder.h"
#include "TypeLocBuilder.h"
+#include "clang/AST/ASTConcept.h"
#include "clang/AST/Decl.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DeclTemplate.h"
@@ -37,6 +38,7 @@
#include "clang/Sema/ScopeInfo.h"
#include "clang/Sema/SemaDiagnostic.h"
#include "clang/Sema/SemaInternal.h"
+#include "clang/Sema/TemplateDeduction.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/Support/ErrorHandling.h"
#include <algorithm>
@@ -3477,6 +3479,23 @@
return Result;
}
+ ExprResult RebuildConceptSpecializationSubstitutionFailure(
+ ConceptSpecializationExpr *E,
+ ConstraintSatisfaction::SubstitutionDiagnostic *SubstDiags) {
+ ConstraintSatisfaction Satisfaction;
+ Satisfaction.IsSatisfied = false;
+ Satisfaction.ContainsErrors = false;
+ Satisfaction.Details.emplace_back(E, SubstDiags);
+ CXXScopeSpec SS;
+ SS.Adopt(E->getNestedNameSpecifierLoc());
+ return ConceptSpecializationExpr::CreateSubstitutionFailure(
+ SemaRef.Context,
+ SS.isSet() ? SS.getWithLocInContext(SemaRef.Context)
+ : NestedNameSpecifierLoc{},
+ E->getTemplateKWLoc(), E->getConceptNameInfo(), E->getFoundDecl(),
+ E->getNamedConcept(), &Satisfaction);
+ }
+
/// \brief Build a new requires expression.
///
/// By default, performs semantic analysis to build the new expression.
@@ -12611,15 +12630,42 @@
E->getEndLoc());
}
-template<typename Derived>
-ExprResult
-TreeTransform<Derived>::TransformConceptSpecializationExpr(
- ConceptSpecializationExpr *E) {
+namespace {
+// TODO: Use SubstitutionDiagnostic instead of pair<Loc, Message> to propagate
+// SubstitutedEntity.
+// TODO: Refactor to use CreateSubstDiag from SemaTemplateInstantiate.cpp.
+inline ConstraintSatisfaction::SubstitutionDiagnostic *
+CreateSubstDiag(Sema &S, sema::TemplateDeductionInfo &Info) {
+ SmallString<128> Message;
+ SourceLocation ErrorLoc;
+
+ PartialDiagnosticAt PDA(SourceLocation(),
+ PartialDiagnostic::NullDiagnostic{});
+ Info.takeSFINAEDiagnostic(PDA);
+ PDA.second.EmitToString(S.getDiagnostics(), Message);
+ ErrorLoc = PDA.first;
+ char *MessageBuf = new (S.Context) char[Message.size()];
+ std::copy(Message.begin(), Message.end(), MessageBuf);
+ return new (S.Context) ConstraintSatisfaction::SubstitutionDiagnostic{
+ ErrorLoc, StringRef(MessageBuf, Message.size())};
+}
+} // namespace
+
+template <typename Derived>
+ExprResult TreeTransform<Derived>::TransformConceptSpecializationExpr(
+ ConceptSpecializationExpr *E) {
const ASTTemplateArgumentListInfo *Old = E->getTemplateArgsAsWritten();
TemplateArgumentListInfo TransArgs(Old->LAngleLoc, Old->RAngleLoc);
- if (getDerived().TransformTemplateArguments(Old->getTemplateArgs(),
- Old->NumTemplateArgs, TransArgs))
- return ExprError();
+ {
+ Sema::SFINAETrap Trap(getSema());
+ if (getDerived().TransformTemplateArguments(
+ Old->getTemplateArgs(), Old->NumTemplateArgs, TransArgs)) {
+ if (!E->isInstantiationDependent() || !Trap.hasErrorOccurred())
+ return ExprError();
+ return getDerived().RebuildConceptSpecializationSubstitutionFailure(
+ E, CreateSubstDiag(SemaRef, **SemaRef.isSFINAEContext()));
+ }
+ }
return getDerived().RebuildConceptSpecializationExpr(
E->getNestedNameSpecifierLoc(), E->getTemplateKWLoc(),
Index: clang/lib/Sema/SemaConcept.cpp
===================================================================
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -11,6 +11,7 @@
//===----------------------------------------------------------------------===//
#include "TreeTransform.h"
+#include "clang/AST/ASTConcept.h"
#include "clang/Sema/SemaConcept.h"
#include "clang/Sema/Sema.h"
#include "clang/Sema/SemaInternal.h"
@@ -941,6 +942,17 @@
break;
}
} else if (auto *CSE = dyn_cast<ConceptSpecializationExpr>(SubstExpr)) {
+ if (CSE->hasSubstitutionFailureInArgs()) {
+ for (const auto &Diags : CSE->getSatisfaction()) {
+ auto *SubstDiag =
+ Diags.second
+ .get<ConstraintSatisfaction::SubstitutionDiagnostic *>();
+ S.Diag(SubstDiag->first,
+ diag::note_concept_specialisation_substitution_error)
+ << (int)First << SubstDiag->second;
+ }
+ return;
+ }
if (CSE->getTemplateArgsAsWritten()->NumTemplateArgs == 1) {
S.Diag(
CSE->getSourceRange().getBegin(),
Index: clang/lib/AST/StmtPrinter.cpp
===================================================================
--- clang/lib/AST/StmtPrinter.cpp
+++ clang/lib/AST/StmtPrinter.cpp
@@ -2467,9 +2467,12 @@
if (E->getTemplateKWLoc().isValid())
OS << "template ";
OS << E->getFoundDecl()->getName();
- printTemplateArgumentList(OS, E->getTemplateArgsAsWritten()->arguments(),
- Policy,
- E->getNamedConcept()->getTemplateParameters());
+ if (E->hasSubstitutionFailureInArgs())
+ OS << "<<error-expression>>";
+ else
+ printTemplateArgumentList(OS, E->getTemplateArgsAsWritten()->arguments(),
+ Policy,
+ E->getNamedConcept()->getTemplateParameters());
}
void StmtPrinter::VisitRequiresExpr(RequiresExpr *E) {
Index: clang/lib/AST/ExprConcepts.cpp
===================================================================
--- clang/lib/AST/ExprConcepts.cpp
+++ clang/lib/AST/ExprConcepts.cpp
@@ -36,14 +36,15 @@
NamedDecl *FoundDecl, ConceptDecl *NamedConcept,
const ASTTemplateArgumentListInfo *ArgsAsWritten,
ImplicitConceptSpecializationDecl *SpecDecl,
- const ConstraintSatisfaction *Satisfaction)
+ const ConstraintSatisfaction *Satisfaction, bool ArgsHasSubstitutionFailure)
: Expr(ConceptSpecializationExprClass, C.BoolTy, VK_PRValue, OK_Ordinary),
ConceptReference(NNS, TemplateKWLoc, ConceptNameInfo, FoundDecl,
NamedConcept, ArgsAsWritten),
SpecDecl(SpecDecl),
Satisfaction(Satisfaction
? ASTConstraintSatisfaction::Create(C, *Satisfaction)
- : nullptr) {
+ : nullptr),
+ ArgsHasSubstitutionFailure(ArgsHasSubstitutionFailure) {
setDependence(computeDependence(this, /*ValueDependent=*/!Satisfaction));
// Currently guaranteed by the fact concepts can only be at namespace-scope.
@@ -53,6 +54,13 @@
->containsUnexpandedParameterPack()));
assert((!isValueDependent() || isInstantiationDependent()) &&
"should not be value-dependent");
+ if (ArgsHasSubstitutionFailure) {
+ assert(Satisfaction);
+ assert(!Satisfaction->IsSatisfied);
+ assert(!Satisfaction->ContainsErrors);
+ assert(!ArgsAsWritten);
+ assert(!SpecDecl);
+ }
}
ConceptSpecializationExpr::ConceptSpecializationExpr(EmptyShell Empty)
@@ -103,6 +111,17 @@
Dependent, ContainsUnexpandedParameterPack);
}
+ConceptSpecializationExpr *ConceptSpecializationExpr::CreateSubstitutionFailure(
+ const ASTContext &C, NestedNameSpecifierLoc NNS,
+ SourceLocation TemplateKWLoc, DeclarationNameInfo ConceptNameInfo,
+ NamedDecl *FoundDecl, ConceptDecl *NamedConcept,
+ const ConstraintSatisfaction *Satisfaction) {
+ return new (C) ConceptSpecializationExpr(
+ C, NNS, TemplateKWLoc, ConceptNameInfo, FoundDecl, NamedConcept,
+ /*ArgsAsWritten=*/nullptr, /*SpecDecl=*/nullptr, Satisfaction,
+ /*ArgsHasSubstitutionFailure=*/true);
+}
+
const TypeConstraint *
concepts::ExprRequirement::ReturnTypeRequirement::getTypeConstraint() const {
assert(isTypeConstraint());
Index: clang/lib/AST/ComputeDependence.cpp
===================================================================
--- clang/lib/AST/ComputeDependence.cpp
+++ clang/lib/AST/ComputeDependence.cpp
@@ -844,12 +844,13 @@
auto TA = TemplateArgumentDependence::None;
const auto InterestingDeps = TemplateArgumentDependence::Instantiation |
TemplateArgumentDependence::UnexpandedPack;
- for (const TemplateArgumentLoc &ArgLoc :
- E->getTemplateArgsAsWritten()->arguments()) {
- TA |= ArgLoc.getArgument().getDependence() & InterestingDeps;
- if (TA == InterestingDeps)
- break;
- }
+ if (E->getTemplateArgsAsWritten())
+ for (const TemplateArgumentLoc &ArgLoc :
+ E->getTemplateArgsAsWritten()->arguments()) {
+ TA |= ArgLoc.getArgument().getDependence() & InterestingDeps;
+ if (TA == InterestingDeps)
+ break;
+ }
ExprDependence D =
ValueDependent ? ExprDependence::Value : ExprDependence::None;
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2889,6 +2889,8 @@
"%select{and|because}0 '%1' would be invalid">;
def note_nested_requirement_substitution_error : Note<
"%select{and|because}0 '%1' would be invalid: %2">;
+def note_concept_specialisation_substitution_error : Note<
+ "%select{and|because}0 '%1'">;
def note_nested_requirement_unknown_substitution_error : Note<
"%select{and|because}0 '%1' would be invalid">;
def note_ambiguous_atomic_constraints : Note<
Index: clang/include/clang/AST/ExprConcepts.h
===================================================================
--- clang/include/clang/AST/ExprConcepts.h
+++ clang/include/clang/AST/ExprConcepts.h
@@ -53,6 +53,7 @@
/// given arguments. If this expression is value dependent, this is to be
/// ignored.
ASTConstraintSatisfaction *Satisfaction;
+ bool ArgsHasSubstitutionFailure = false;
ConceptSpecializationExpr(const ASTContext &C, NestedNameSpecifierLoc NNS,
SourceLocation TemplateKWLoc,
@@ -60,7 +61,8 @@
NamedDecl *FoundDecl, ConceptDecl *NamedConcept,
const ASTTemplateArgumentListInfo *ArgsAsWritten,
ImplicitConceptSpecializationDecl *SpecDecl,
- const ConstraintSatisfaction *Satisfaction);
+ const ConstraintSatisfaction *Satisfaction,
+ bool ArgsHasSubstitutionFailure = false);
ConceptSpecializationExpr(const ASTContext &C, ConceptDecl *NamedConcept,
ImplicitConceptSpecializationDecl *SpecDecl,
@@ -84,10 +86,22 @@
const ConstraintSatisfaction *Satisfaction, bool Dependent,
bool ContainsUnexpandedParameterPack);
+ static ConceptSpecializationExpr *
+ CreateSubstitutionFailure(const ASTContext &C, NestedNameSpecifierLoc NNS,
+ SourceLocation TemplateKWLoc,
+ DeclarationNameInfo ConceptNameInfo,
+ NamedDecl *FoundDecl, ConceptDecl *NamedConcept,
+ const ConstraintSatisfaction *Satisfaction);
+
ArrayRef<TemplateArgument> getTemplateArguments() const {
+ assert(SpecDecl);
return SpecDecl->getTemplateArguments();
}
+ bool hasSubstitutionFailureInArgs() const {
+ return ArgsHasSubstitutionFailure;
+ }
+
const ImplicitConceptSpecializationDecl *getSpecializationDecl() const {
assert(SpecDecl && "Template Argument Decl not initialized");
return SpecDecl;
@@ -122,6 +136,7 @@
}
SourceLocation getEndLoc() const LLVM_READONLY {
+ assert(ArgsAsWritten);
// If the ConceptSpecializationExpr is the ImmediatelyDeclaredConstraint
// of a TypeConstraint written syntactically as a constrained-parameter,
// there may not be a template argument list.
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -586,9 +586,11 @@
([temp.func.order]p6.2.1 is not implemented, matching GCC).
- Implemented `P0857R0 <https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0857r0.html>`_,
which specifies constrained lambdas and constrained template *template-parameter*\s.
-
- Do not hide templated base members introduced via using-decl in derived class
(useful specially for constrained members). Fixes `GH50886 <https://github.com/llvm/llvm-project/issues/50886>`_.
+- Substitution failure in concept specialisation now makes the concept unsatisfied instead of
+ an invalid expression. Fixes evaluation of nested requirements involving such concepts
+ along with logical binary operators: `Issue 45563 <https://github.com/llvm/llvm-project/issues/45563>`_.
C++2b Feature Support
^^^^^^^^^^^^^^^^^^^^^
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
