Correctly handle Substitution failure in concept specialization.

[usaxena95]

Summary

Fixes: https://github.com/llvm/llvm-project/issues/45563

template<class T>  concept True = true;

template <class T>
concept C1 = requires (T) {
   requires True<typename T::value> || True<T>;
};

template <class T>
constexpr bool foo() 
requires True<typename T::value> || True<T> {
    return true;
}
static_assert(C1<double>); // Previously failed due to SFINAE error
static_assert(foo<int>()); // but this works fine.

The issue here is the discrepancy between how a nested requirement is evaluated Vs how a non-nested requirement is evaluated.

In non-nested cases, we explicitly handle Binary operators transforming, substituting and evaluating each branch separately while for a nested requirement we substituting the complete the constraint expr leading to SFINAE failure for the complete expression. Restricting the SFINAE failure to the atomic constraint and marking the constraint only as "not-satisfied" solves the issue.

1. https://eel.is/c++draft/expr.prim.req#nested-1

Substitution of template arguments into a nested-requirement does not result in substitution into the constraint-expression other than as specified in [temp.constr.constr].

https://eel.is/c++draft/expr.prim.req#nested-example-1

[Example 1:
template<typename U> concept C = sizeof(U) == 1;

template<typename T> concept D = requires (T t) {
  requires C<decltype (+t)>;
};
D<T> is satisfied if sizeof(decltype (+t)) == 1 ([temp.constr.atomic]). — end example]

2. https://eel.is/c++draft/temp.constr.constr#temp.constr.atomic-3

To determine if an atomic constraint is satisfied, the parameter mapping and template arguments are first substituted into its expression. If substitution results in an invalid type or expression, the constraint is not satisfied.

Therefore in case of substitution failure in a constraint expression, the corresponding atomic constraint should be failed and not the entire constraint expression/nested requires expression.

TODO: I want to properly propagate the substitution failure.

[erichkeane]

I dont have a good hold as to why this is the solution, can you better explain the issue and the solution that you made? I will take a look when I get a chance next week, as the ISO meeting is taking up my week.

[shafik]

clang/lib/AST/ExprConcepts.cpp

122–123

I think I got the parameter names correct.

[usaxena95]

Moving closer to show diagnostic of entity for which SFNIAE occurred.

[erichkeane]

I think this is a move in the right direction, and generally is probably pretty close. The new bool for ArgsHasSubstitutionFailure isn't something I get the need for yet though.

we also need release notes.

clang/include/clang/AST/ExprConcepts.h

103

Does this really belong here instead of as a part of the ConceptSpecializationDecl?

159

I had a previous patch at something else where I was moving toward doing this change, so I think this is probably something inevitable.

However, I'm having a tough time splitting this patch mentally between the 'fix' and the infrastructure changes needed. A part of me thinks we should split this patch a bit in that direction.

[usaxena95]

Addressed commented. Added Release notes.
Removed infrastructure changes and deferred them to a future patch.

clang/include/clang/AST/ExprConcepts.h

103

I am not aware of the original goals for this. For example, ASTTemplateArgumentListInfo is part of ConceptReference while TemplateArgument is part of ImplicitConceptSpecializationDecl. Both of them are invalid in such a case.
I am open to suggestion to where to place this.

159

I agree. This is a larger change which probably belongs to a separate change. Until then I think it is fine to not provide diagnostic of the exact expression of the SubstitutedEntity for which SF occurred.

clang/lib/AST/ExprConcepts.cpp

122–123

Could you please elaborate what is your suggestion here ?

[erichkeane]

clang/include/clang/AST/ExprConcepts.h

103

Yeah, the design of all of these is a little wonky unfortunately. The ConceptReference is so it can be a base of a TypeConstraint as well, so anything that also needs to be in type-constraint belongs there.

ConceptSpecializationDecl is a recent split off ConstraintSpecializationExpr whose intent is to be something that later instantiation (like instantiating a lambda defined in a concept decl) can use to make sure we get template arguments correct. So I suspect the args failure needs to be contained in that as well.

IF this is something that can also be invalid in a TypeConstraint, it belongs in ConceptReference AND ConceptSpecializationDecl separately I believe (as we'll likely need to check that's validity later).

[ilya-biryukov]

We discussed this with Utkarsh offline, he's OOO for some time, so I wanted to leave the summary here.
This patch attempts to attack the problem by keeping the information about substitution failure in template arguments of ConceptSpecializationExpr.
However, it seems that the standard does not give any special treatment to those as compared to other forms of substitution failure and we should rethink our approach. The problem seems to be in how we evaluate SFINAE inside requires-expression and/or nested-requirement.

E.g. if we replace a concept by a template variable, a failure to substitute template arguments should not cause the whole requires-expression to evaluate to false:

template <class T> constexpr bool true_v = true; 
template <class T> constexpr int foo() requires requires { requires true_v<typename T::type> || true_v<T>; }
    { return 123; }

static_assert(foo<int>() == 123);

GCC and MSVC succeed here, Clang fails: https://gcc.godbolt.org/z/WGdKWYM7e. Clang's behavior seems to be non-standard here.
The proper fix seems to be changing Clang behavior so that the requires-expression evaluates to true despite its first branch having a subsitution failure.

Utkarsh has also found an interesting example where GCC (IMO, unexpectedly) evaluates the concept to true despite a substitution failure in its template arguments:

template <class U> concept true_c = true; 
template <class T> constexpr int foo() 
    requires requires { requires true_c<typename T::type>; }
    { return 123; }

static_assert(foo<int>() == 123);

Clang and MSVC fail here, GCC succeeds: https://gcc.godbolt.org/z/5qYjcW75q.
The standard does not seem to be very clear here. It says that an "atomic constraint" that we must evaluate contains both an expression (true) and a template parameter mapping (U -> typename T::type, where T is int). The template parameter mapping is clearly not well-formed, the expression still evaluates to true and the standard does not seem to mention what to do when template parameter mapping is not well-formed. IMO, to form the template parameter mapping we must substitute T -> int into the template argument list (<typename T::type>) and at that point we see a substitution failure, hence requires-expression should evaluate to false. Someone should probably raise this with WG21 to know for sure.

[usaxena95]

Second attempt along the lines of the last comment: https://reviews.llvm.org/D138914
PTAL.

clang/include/clang/AST/ASTConcept.h

 #include "clang/AST/Expr.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/SmallVector.h"
 #include <utility>
 
 namespace clang {
 class ConceptDecl;
+namespace concepts {
+struct SubstitutionDiagnostic;
+} // namespace concepts
 
 /// The result of a constraint satisfaction check, containing the necessary
 /// information to diagnose an unsatisfied constraint.
 class ConstraintSatisfaction : public llvm::FoldingSetNode {
   // The template-like entity that 'owns' the constraint checked here (can be a
   // constrained entity or a concept).
   const NamedDecl *ConstraintOwner = nullptr;
   llvm::SmallVector<TemplateArgument, 4> TemplateArgs;
 
 public:
 
   ConstraintSatisfaction() = default;
 
   ConstraintSatisfaction(const NamedDecl *ConstraintOwner,
                          ArrayRef<TemplateArgument> TemplateArgs) :
       ConstraintOwner(ConstraintOwner), TemplateArgs(TemplateArgs.begin(),
                                                      TemplateArgs.end()) { }
 
-  using SubstitutionDiagnostic = std::pair<SourceLocation, StringRef>;
-  using Detail = llvm::PointerUnion<Expr *, SubstitutionDiagnostic *>;
+  using Detail = llvm::PointerUnion<Expr *, concepts::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
   /// substituted into them, or a diagnostic if substitution resulted in an
   /// invalid expression.
   llvm::SmallVector<std::pair<const Expr *, Detail>, 4> Details;
 
   void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &C) {
     Profile(ID, C, ConstraintOwner, TemplateArgs);
   }
 
   static void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &C,
                       const NamedDecl *ConstraintOwner,
                       ArrayRef<TemplateArgument> TemplateArgs);
 };
 
 /// Pairs of unsatisfied atomic constraint expressions along with the
 /// substituted constraint expr, if the template arguments could be
 /// substituted into them, or a diagnostic if substitution resulted in
 /// an invalid expression.
 using UnsatisfiedConstraintRecord =
-    std::pair<const Expr *,
-              llvm::PointerUnion<Expr *,
-                                 std::pair<SourceLocation, StringRef> *>>;
+    std::pair<const Expr *, ConstraintSatisfaction::Detail>;
 
 /// \brief The result of a constraint satisfaction check, containing the
 /// necessary information to diagnose an unsatisfied constraint.
 ///
 /// This is safe to store in an AST node, as opposed to ConstraintSatisfaction.
 struct ASTConstraintSatisfaction final :
     llvm::TrailingObjects<ASTConstraintSatisfaction,
                           UnsatisfiedConstraintRecord> {

clang/include/clang/AST/ExprConcepts.h

 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/Support/TrailingObjects.h"
 #include <utility>
 #include <string>
 
 namespace clang {
+namespace sema {
+class TemplateDeductionInfo;
+}
+
 class ASTStmtReader;
 class ASTStmtWriter;
 
 /// \brief Represents the specialization of a concept - evaluates to a prvalue
 /// of type bool.
 ///
 /// According to C++2a [expr.prim.id]p3 an id-expression that denotes the
 /// specialization of a concept results in a prvalue of type bool.
 class ConceptSpecializationExpr final : public Expr, public ConceptReference {
   friend class ASTReader;
   friend class ASTStmtReader;
 
 public:
-  using SubstitutionDiagnostic = std::pair<SourceLocation, std::string>;
 
 protected:
   /// \brief The Implicit Concept Specialization Decl, which holds the template
   /// arguments for this specialization.
   ImplicitConceptSpecializationDecl *SpecDecl;
 
   /// \brief Information about the satisfaction of the named concept with the
   /// 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,
                             DeclarationNameInfo ConceptNameInfo,
                             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,
                             const ConstraintSatisfaction *Satisfaction,
                             bool Dependent,
                             bool ContainsUnexpandedParameterPack);
   ConceptSpecializationExpr(EmptyShell Empty);
 
 public:
   static ConceptSpecializationExpr *
   Create(const ASTContext &C, NestedNameSpecifierLoc NNS,
          SourceLocation TemplateKWLoc, DeclarationNameInfo ConceptNameInfo,
          NamedDecl *FoundDecl, ConceptDecl *NamedConcept,
          const ASTTemplateArgumentListInfo *ArgsAsWritten,
          ImplicitConceptSpecializationDecl *SpecDecl,
          const ConstraintSatisfaction *Satisfaction);
 
   static ConceptSpecializationExpr *
   Create(const ASTContext &C, ConceptDecl *NamedConcept,
          ImplicitConceptSpecializationDecl *SpecDecl,
          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 {
     return SpecDecl->getTemplateArguments();
   }
 
+  bool hasSubstitutionFailureInArgs() const {
+    return ArgsHasSubstitutionFailure;
+  }
+
   const ImplicitConceptSpecializationDecl *getSpecializationDecl() const {
     assert(SpecDecl && "Template Argument Decl not initialized");
     return SpecDecl;
   }
 
   /// \brief Whether or not the concept with the given arguments was satisfied
   /// when the expression was created.
   /// The expression must not be dependent.
child_range children() {
     return child_range(child_iterator(), child_iterator());
   }
   const_child_range children() const {
     return const_child_range(const_child_iterator(), const_child_iterator());
   }
 };
 
 namespace concepts {
+  // todo: add doc ?
+struct SubstitutionDiagnostic {
+  StringRef SubstitutedEntity;
+  // FIXME: Store diagnostics semantically and not as prerendered strings.
+  //  Fixing this probably requires serialization of PartialDiagnostic
+  //  objects.
+  SourceLocation DiagLoc;
+  StringRef DiagMessage;
+};
 
 /// \brief A static requirement that can be used in a requires-expression to
 /// check properties of types and expression.
 class Requirement {
 public:
   // Note - simple and compound requirements are both represented by the same
   // class (ExprRequirement).
   enum RequirementKind { RK_Type, RK_Simple, RK_Compound, RK_Nested };
 private:
   const RequirementKind Kind;
   // FIXME: use RequirementDependence to model dependence?
   bool Dependent : 1;
   bool ContainsUnexpandedParameterPack : 1;
   bool Satisfied : 1;
 public:
-  struct SubstitutionDiagnostic {
-    StringRef SubstitutedEntity;
-    // FIXME: Store diagnostics semantically and not as prerendered strings.
-    //  Fixing this probably requires serialization of PartialDiagnostic
-    //  objects.
-    SourceLocation DiagLoc;
-    StringRef DiagMessage;
-  };
 
   Requirement(RequirementKind Kind, bool IsDependent,
               bool ContainsUnexpandedParameterPack, bool IsSatisfied = true) :
       Kind(Kind), Dependent(IsDependent),
       ContainsUnexpandedParameterPack(ContainsUnexpandedParameterPack),
       Satisfied(IsSatisfied) {}
 
   RequirementKind getKind() const { return Kind; }

clang/include/clang/Sema/Sema.h

   ActOnCompoundRequirement(
       Expr *E, SourceLocation NoexceptLoc, CXXScopeSpec &SS,
       TemplateIdAnnotation *TypeConstraint, unsigned Depth);
   concepts::Requirement *ActOnNestedRequirement(Expr *Constraint);
   concepts::ExprRequirement *
   BuildExprRequirement(
       Expr *E, bool IsSatisfied, SourceLocation NoexceptLoc,
       concepts::ExprRequirement::ReturnTypeRequirement ReturnTypeRequirement);
-  concepts::ExprRequirement *
-  BuildExprRequirement(
-      concepts::Requirement::SubstitutionDiagnostic *ExprSubstDiag,
-      bool IsSatisfied, SourceLocation NoexceptLoc,
+  concepts::ExprRequirement *BuildExprRequirement(
+      concepts::SubstitutionDiagnostic *ExprSubstDiag, bool IsSatisfied,
+      SourceLocation NoexceptLoc,
       concepts::ExprRequirement::ReturnTypeRequirement ReturnTypeRequirement);
   concepts::TypeRequirement *BuildTypeRequirement(TypeSourceInfo *Type);
   concepts::TypeRequirement *
-  BuildTypeRequirement(
-      concepts::Requirement::SubstitutionDiagnostic *SubstDiag);
+  BuildTypeRequirement(concepts::SubstitutionDiagnostic *SubstDiag);
   concepts::NestedRequirement *BuildNestedRequirement(Expr *E);
   concepts::NestedRequirement *
-  BuildNestedRequirement(
-      concepts::Requirement::SubstitutionDiagnostic *SubstDiag);
+  BuildNestedRequirement(concepts::SubstitutionDiagnostic *SubstDiag);
   ExprResult ActOnRequiresExpr(SourceLocation RequiresKWLoc,
                                RequiresExprBodyDecl *Body,
                                ArrayRef<ParmVarDecl *> LocalParameters,
                                ArrayRef<concepts::Requirement *> Requirements,
                                SourceLocation ClosingBraceLoc);
 
   //===--------------------------------------------------------------------===//
   // C++ Variadic Templates (C++0x [temp.variadic])

clang/lib/AST/ASTConcept.cpp

 //===--- ASTConcept.cpp - Concepts Related AST Data Structures --*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
 /// \brief This file defines AST data structures related to concepts.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "clang/AST/ASTConcept.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/ExprConcepts.h"
 #include "clang/AST/TemplateBase.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/FoldingSet.h"
 using namespace clang;
 
 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 *>())
       new (getTrailingObjects<UnsatisfiedConstraintRecord>() + I)
          UnsatisfiedConstraintRecord{Detail.first,
                                      UnsatisfiedConstraintRecord::second_type(
                                          Detail.second.get<Expr *>())};
     else {
       auto &SubstitutionDiagnostic =
-          *Detail.second.get<std::pair<SourceLocation, StringRef> *>();
-      unsigned MessageSize = SubstitutionDiagnostic.second.size();
+          *Detail.second.get<concepts::SubstitutionDiagnostic *>();
+      unsigned MessageSize = SubstitutionDiagnostic.DiagMessage.size();
       char *Mem = new (C) char[MessageSize];
-      memcpy(Mem, SubstitutionDiagnostic.second.data(), MessageSize);
-      auto *NewSubstDiag = new (C) std::pair<SourceLocation, StringRef>(
-          SubstitutionDiagnostic.first, StringRef(Mem, MessageSize));
+      memcpy(Mem, SubstitutionDiagnostic.DiagMessage.data(), MessageSize);
+      auto *NewSubstDiag = new (C) concepts::SubstitutionDiagnostic{
+          /*todo: copy me*/ SubstitutionDiagnostic.SubstitutedEntity,
+          SubstitutionDiagnostic.DiagLoc, StringRef(Mem, MessageSize)};
       new (getTrailingObjects<UnsatisfiedConstraintRecord>() + I)
          UnsatisfiedConstraintRecord{Detail.first,
                                      UnsatisfiedConstraintRecord::second_type(
                                          NewSubstDiag)};
     }
   }
 }
 

clang/lib/AST/ComputeDependence.cpp

ExprDependence clang::computeDependence(TypeTraitExpr *E) {
   return D;
 }
 
 ExprDependence clang::computeDependence(ConceptSpecializationExpr *E,
                                         bool ValueDependent) {
   auto TA = TemplateArgumentDependence::None;
   const auto InterestingDeps = TemplateArgumentDependence::Instantiation |
                                TemplateArgumentDependence::UnexpandedPack;
+  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;
   auto Res = D | toExprDependence(TA);
   if(!ValueDependent && E->getSatisfaction().ContainsErrors)
     Res |= ExprDependence::Error;
   return Res;
 }

clang/lib/AST/ExprConcepts.cpp

 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/DependenceFlags.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 "clang/Sema/Sema.h"
+#include "clang/Sema/TemplateDeduction.h"
 #include "llvm/Support/TrailingObjects.h"
 #include <algorithm>
 #include <string>
 #include <utility>
 
 using namespace clang;
 
 ConceptSpecializationExpr::ConceptSpecializationExpr(
     const ASTContext &C, NestedNameSpecifierLoc NNS,
     SourceLocation TemplateKWLoc, DeclarationNameInfo ConceptNameInfo,
     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.
   assert(!NestedNameSpec ||
          (!NestedNameSpec.getNestedNameSpecifier()->isInstantiationDependent() &&
           !NestedNameSpec.getNestedNameSpecifier()
               ->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)
     : Expr(ConceptSpecializationExprClass, Empty) {}
 
 ConceptSpecializationExpr *ConceptSpecializationExpr::Create(
     const ASTContext &C, NestedNameSpecifierLoc NNS,
     SourceLocation TemplateKWLoc, DeclarationNameInfo ConceptNameInfo,
ConceptSpecializationExpr *ConceptSpecializationExpr::Create(
     ImplicitConceptSpecializationDecl *SpecDecl,
     const ConstraintSatisfaction *Satisfaction, bool Dependent,
     bool ContainsUnexpandedParameterPack) {
   return new (C)
       ConceptSpecializationExpr(C, NamedConcept, SpecDecl, Satisfaction,
                                 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());
   auto TPL =
       TypeConstraintInfo.getPointer().get<TemplateParameterList *>();
   return cast<TemplateTypeParmDecl>(TPL->getParam(0))
       ->getTypeConstraint();
 }

clang/lib/AST/StmtPrinter.cpp

 
 void StmtPrinter::VisitConceptSpecializationExpr(ConceptSpecializationExpr *E) {
   NestedNameSpecifierLoc NNS = E->getNestedNameSpecifierLoc();
   if (NNS)
     NNS.getNestedNameSpecifier()->print(OS, Policy);
   if (E->getTemplateKWLoc().isValid())
     OS << "template ";
   OS << E->getFoundDecl()->getName();
+  if (E->hasSubstitutionFailureInArgs())
+    OS << "<<error-expression>>";
+  else
     printTemplateArgumentList(OS, E->getTemplateArgsAsWritten()->arguments(),
                               Policy,
                               E->getNamedConcept()->getTemplateParameters());
 }
 
 void StmtPrinter::VisitRequiresExpr(RequiresExpr *E) {
   OS << "requires ";
   auto LocalParameters = E->getLocalParameters();
   if (!LocalParameters.empty()) {
     OS << "(";
     for (ParmVarDecl *LocalParam : LocalParameters) {

clang/lib/Sema/SemaConcept.cpp

if (SubstitutedAtomicExpr.get()->containsErrors()) {
     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(),
+        ConstraintExpr, new (S.Context) concepts::SubstitutionDiagnostic{
+                            "", SubstitutedAtomicExpr.get()->getBeginLoc(),
                             StringRef(Mem, MessageSize)});
     return SubstitutedAtomicExpr;
   }
 
   EnterExpressionEvaluationContext ConstantEvaluated(
       S, Sema::ExpressionEvaluationContext::ConstantEvaluated);
   SmallVector<PartialDiagnosticAt, 2> EvaluationDiags;
   Expr::EvalResult EvalResult;
   EvalResult.Diag = &EvaluationDiags;
return calculateConstraintSatisfaction(
             //  it as a string defeats the purpose of the diagnostic mechanism.
             SmallString<128> DiagString;
             DiagString = ": ";
             SubstDiag.second.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(
-                AtomicExpr,
-                new (S.Context) ConstraintSatisfaction::SubstitutionDiagnostic{
+                AtomicExpr, new (S.Context) concepts::SubstitutionDiagnostic{
+                                "todo: fill substituted entity",
                                 SubstDiag.first, StringRef(Mem, MessageSize)});
             Satisfaction.IsSatisfied = false;
             return ExprEmpty();
           }
         }
 
         if (!S.CheckConstraintExpression(SubstitutedExpression.get()))
           return ExprError();
 
default:
     return;
   }
 }
 
 static void diagnoseUnsatisfiedRequirement(Sema &S,
                                            concepts::NestedRequirement *Req,
                                            bool First) {
   if (Req->isSubstitutionFailure()) {
-    concepts::Requirement::SubstitutionDiagnostic *SubstDiag =
+    concepts::SubstitutionDiagnostic *SubstDiag =
         Req->getSubstitutionDiagnostic();
     if (!SubstDiag->DiagMessage.empty())
       S.Diag(SubstDiag->DiagLoc,
              diag::note_nested_requirement_substitution_error)
              << (int)First << SubstDiag->SubstitutedEntity
              << SubstDiag->DiagMessage;
     else
       S.Diag(SubstDiag->DiagLoc,
case BO_NE:
         }
       }
       break;
 
     default:
       break;
     }
   } else if (auto *CSE = dyn_cast<ConceptSpecializationExpr>(SubstExpr)) {
+    if (CSE->hasSubstitutionFailureInArgs()) {
+      for (const auto &Diags : CSE->getSatisfaction()) {
+        auto *SubstDiag =
+            Diags.second.get<concepts::SubstitutionDiagnostic *>();
+        S.Diag(SubstDiag->DiagLoc,
+               // todo: create another diag for concept specialisation. this one
+               // is not needed anymore.
+               diag::note_nested_requirement_substitution_error)
+            << (int)First << SubstDiag->SubstitutedEntity
+            << SubstDiag->DiagMessage;
+      }
+      return;
+    }
     if (CSE->getTemplateArgsAsWritten()->NumTemplateArgs == 1) {
       S.Diag(
           CSE->getSourceRange().getBegin(),
           diag::
           note_single_arg_concept_specialization_constraint_evaluated_to_false)
           << (int)First
           << CSE->getTemplateArgsAsWritten()->arguments()[0].getArgument()
           << CSE->getNamedConcept();

 }
 
 template<typename SubstitutionDiagnostic>
 static void diagnoseUnsatisfiedConstraintExpr(
     Sema &S, const Expr *E,
     const llvm::PointerUnion<Expr *, SubstitutionDiagnostic *> &Record,
     bool First = true) {
   if (auto *Diag = Record.template dyn_cast<SubstitutionDiagnostic *>()){
-    S.Diag(Diag->first, diag::note_substituted_constraint_expr_is_ill_formed)
-        << Diag->second;
+    S.Diag(Diag->DiagLoc, diag::note_substituted_constraint_expr_is_ill_formed)
+        << Diag->DiagMessage;
     return;
   }
 
   diagnoseWellFormedUnsatisfiedConstraintExpr(S,
       Record.template get<Expr *>(), First);
 }
 
 void

clang/lib/Sema/SemaExprCXX.cpp

if (Constraint.isInvalid()) {
         Status = concepts::ExprRequirement::SS_ConstraintsNotSatisfied;
     }
   }
   return new (Context) concepts::ExprRequirement(E, IsSimple, NoexceptLoc,
                                                  ReturnTypeRequirement, Status,
                                                  SubstitutedConstraintExpr);
 }
 
-concepts::ExprRequirement *
-Sema::BuildExprRequirement(
-    concepts::Requirement::SubstitutionDiagnostic *ExprSubstitutionDiagnostic,
-    bool IsSimple, SourceLocation NoexceptLoc,
+concepts::ExprRequirement *Sema::BuildExprRequirement(
+    concepts::SubstitutionDiagnostic *ExprSubstitutionDiagnostic, bool IsSimple,
+    SourceLocation NoexceptLoc,
     concepts::ExprRequirement::ReturnTypeRequirement ReturnTypeRequirement) {
   return new (Context) concepts::ExprRequirement(ExprSubstitutionDiagnostic,
                                                  IsSimple, NoexceptLoc,
                                                  ReturnTypeRequirement);
 }
 
 concepts::TypeRequirement *
 Sema::BuildTypeRequirement(TypeSourceInfo *Type) {
   return new (Context) concepts::TypeRequirement(Type);
 }
 
 concepts::TypeRequirement *
-Sema::BuildTypeRequirement(
-    concepts::Requirement::SubstitutionDiagnostic *SubstDiag) {
+Sema::BuildTypeRequirement(concepts::SubstitutionDiagnostic *SubstDiag) {
   return new (Context) concepts::TypeRequirement(SubstDiag);
 }
 
 concepts::Requirement *Sema::ActOnNestedRequirement(Expr *Constraint) {
   return BuildNestedRequirement(Constraint);
 }
 
 concepts::NestedRequirement *
 Sema::BuildNestedRequirement(Expr *Constraint) {
   ConstraintSatisfaction Satisfaction;
   if (!Constraint->isInstantiationDependent() &&
       CheckConstraintSatisfaction(nullptr, {Constraint}, /*TemplateArgs=*/{},
                                   Constraint->getSourceRange(), Satisfaction))
     return nullptr;
   return new (Context) concepts::NestedRequirement(Context, Constraint,
                                                    Satisfaction);
 }
 
 concepts::NestedRequirement *
-Sema::BuildNestedRequirement(
-    concepts::Requirement::SubstitutionDiagnostic *SubstDiag) {
+Sema::BuildNestedRequirement(concepts::SubstitutionDiagnostic *SubstDiag) {
   return new (Context) concepts::NestedRequirement(SubstDiag);
 }
 
 RequiresExprBodyDecl *
 Sema::ActOnStartRequiresExpr(SourceLocation RequiresKWLoc,
                              ArrayRef<ParmVarDecl *> LocalParameters,
                              Scope *BodyScope) {
   assert(BodyScope);

clang/lib/Sema/SemaTemplateInstantiate.cpp

QualType TemplateInstantiator::TransformSubstTemplateTypeParmPackType(
 
   TemplateArgument Pack = T->getArgumentPack();
   TemplateArgument Arg = getPackSubstitutedTemplateArgument(getSema(), Pack);
   return BuildSubstTemplateTypeParmType(
       TLB, SuppressObjCLifetime, T->getFinal(), NewReplaced, T->getIndex(),
       getPackIndex(Pack), Arg, TL.getNameLoc());
 }
 
-template<typename EntityPrinter>
-static concepts::Requirement::SubstitutionDiagnostic *
+template <typename EntityPrinter>
+static concepts::SubstitutionDiagnostic *
 createSubstDiag(Sema &S, TemplateDeductionInfo &Info, EntityPrinter Printer) {
   SmallString<128> Message;
   SourceLocation ErrorLoc;
   if (Info.hasSFINAEDiagnostic()) {
     PartialDiagnosticAt PDA(SourceLocation(),
                             PartialDiagnostic::NullDiagnostic{});
     Info.takeSFINAEDiagnostic(PDA);
     PDA.second.EmitToString(S.getDiagnostics(), Message);
     ErrorLoc = PDA.first;
   } else {
     ErrorLoc = Info.getLocation();
   }
   char *MessageBuf = new (S.Context) char[Message.size()];
   std::copy(Message.begin(), Message.end(), MessageBuf);
   SmallString<128> Entity;
   llvm::raw_svector_ostream OS(Entity);
   Printer(OS);
   char *EntityBuf = new (S.Context) char[Entity.size()];
   std::copy(Entity.begin(), Entity.end(), EntityBuf);
-  return new (S.Context) concepts::Requirement::SubstitutionDiagnostic{
+  return new (S.Context) concepts::SubstitutionDiagnostic{
       StringRef(EntityBuf, Entity.size()), ErrorLoc,
       StringRef(MessageBuf, Message.size())};
 }
 
 ExprResult TemplateInstantiator::TransformRequiresTypeParams(
     SourceLocation KWLoc, SourceLocation RBraceLoc, const RequiresExpr *RE,
     RequiresExprBodyDecl *Body, ArrayRef<ParmVarDecl *> Params,
     SmallVectorImpl<QualType> &PTypes,
TemplateInstantiator::TransformTypeRequirement(concepts::TypeRequirement *Req) {
   TemplateDeductionInfo Info(Req->getType()->getTypeLoc().getBeginLoc());
   Sema::InstantiatingTemplate TypeInst(SemaRef,
       Req->getType()->getTypeLoc().getBeginLoc(), Req, Info,
       Req->getType()->getTypeLoc().getSourceRange());
   if (TypeInst.isInvalid())
     return nullptr;
   TypeSourceInfo *TransType = TransformType(Req->getType());
   if (!TransType || Trap.hasErrorOccurred())
-    return RebuildTypeRequirement(createSubstDiag(SemaRef, Info,
-        [&] (llvm::raw_ostream& OS) {
-            Req->getType()->getType().print(OS, SemaRef.getPrintingPolicy());
+    return RebuildTypeRequirement(
+        createSubstDiag(SemaRef, Info, [&](llvm::raw_ostream &OS) {
         }));
   return RebuildTypeRequirement(TransType);
 }
 
 concepts::ExprRequirement *
 TemplateInstantiator::TransformExprRequirement(concepts::ExprRequirement *Req) {
   if (!Req->isDependent() && !AlwaysRebuild())
     return Req;
 
   Sema::SFINAETrap Trap(SemaRef);
 
-  llvm::PointerUnion<Expr *, concepts::Requirement::SubstitutionDiagnostic *>
-      TransExpr;
+  llvm::PointerUnion<Expr *, concepts::SubstitutionDiagnostic *> TransExpr;
   if (Req->isExprSubstitutionFailure())
     TransExpr = Req->getExprSubstitutionDiagnostic();
   else {
     Expr *E = Req->getExpr();
     TemplateDeductionInfo Info(E->getBeginLoc());
     Sema::InstantiatingTemplate ExprInst(SemaRef, E->getBeginLoc(), Req, Info,
                                          E->getSourceRange());
     if (ExprInst.isInvalid())
       return nullptr;
     ExprResult TransExprRes = TransformExpr(E);
     if (!TransExprRes.isInvalid() && !Trap.hasErrorOccurred() &&
         TransExprRes.get()->hasPlaceholderType())
       TransExprRes = SemaRef.CheckPlaceholderExpr(TransExprRes.get());
     if (TransExprRes.isInvalid() || Trap.hasErrorOccurred())
-      TransExpr = createSubstDiag(SemaRef, Info, [&](llvm::raw_ostream &OS) {
+      TransExpr =
+          createSubstDiag(SemaRef, Info, [&](llvm::raw_ostream &OS) {
             E->printPretty(OS, nullptr, SemaRef.getPrintingPolicy());
           });
     else
       TransExpr = TransExprRes.get();
   }
 
   llvm::Optional<concepts::ExprRequirement::ReturnTypeRequirement> TransRetReq;
   const auto &RetReq = Req->getReturnTypeRequirement();
   if (RetReq.isEmpty())
     TransRetReq.emplace();
   else if (RetReq.isSubstitutionFailure())
     TransRetReq.emplace(RetReq.getSubstitutionDiagnostic());
   else if (RetReq.isTypeConstraint()) {
     TemplateParameterList *OrigTPL =
         RetReq.getTypeConstraintTemplateParameterList();
     TemplateDeductionInfo Info(OrigTPL->getTemplateLoc());
     Sema::InstantiatingTemplate TPLInst(SemaRef, OrigTPL->getTemplateLoc(),
                                         Req, Info, OrigTPL->getSourceRange());
     if (TPLInst.isInvalid())
       return nullptr;
     TemplateParameterList *TPL = TransformTemplateParameterList(OrigTPL);
     if (!TPL)
-      TransRetReq.emplace(createSubstDiag(SemaRef, Info,
-          [&] (llvm::raw_ostream& OS) {
-              RetReq.getTypeConstraint()->getImmediatelyDeclaredConstraint()
-                  ->printPretty(OS, nullptr, SemaRef.getPrintingPolicy());
+      TransRetReq.emplace(
+          createSubstDiag(SemaRef, Info, [&](llvm::raw_ostream &OS) {
+            RetReq.getTypeConstraint()
+                ->getImmediatelyDeclaredConstraint()
           }));
     else {
       TPLInst.Clear();
       TransRetReq.emplace(TPL);
     }
   }
   assert(TransRetReq && "All code paths leading here must set TransRetReq");
   if (Expr *E = TransExpr.dyn_cast<Expr *>())
     return RebuildExprRequirement(E, Req->isSimple(), Req->getNoexceptLoc(),
                                   std::move(*TransRetReq));
   return RebuildExprRequirement(
-      TransExpr.get<concepts::Requirement::SubstitutionDiagnostic *>(),
-      Req->isSimple(), Req->getNoexceptLoc(), std::move(*TransRetReq));
+      TransExpr.get<concepts::SubstitutionDiagnostic *>(), Req->isSimple(),
+      Req->getNoexceptLoc(), std::move(*TransRetReq));
 }
 
 concepts::NestedRequirement *
 TemplateInstantiator::TransformNestedRequirement(
     concepts::NestedRequirement *Req) {
   if (!Req->isDependent() && !AlwaysRebuild())
     return Req;
   if (Req->isSubstitutionFailure()) {
if (!TransConstraint.isInvalid() &&
       bool CheckFailed = SemaRef.CheckConstraintSatisfaction(
           TransConstraint.get(), Satisfaction);
       (void)CheckFailed;
       assert((!CheckFailed || Trap.hasErrorOccurred()) &&
                                  "CheckConstraintSatisfaction failed, "
                                  "but did not produce a SFINAE error");
     }
     if (TransConstraint.isInvalid() || Trap.hasErrorOccurred())
-      return RebuildNestedRequirement(createSubstDiag(SemaRef, Info,
-          [&] (llvm::raw_ostream& OS) {
-              Req->getConstraintExpr()->printPretty(OS, nullptr,
-                                                    SemaRef.getPrintingPolicy());
+      return RebuildNestedRequirement(
+          createSubstDiag(SemaRef, Info, [&](llvm::raw_ostream &OS) {
           }));
   }
   if (TransConstraint.get()->isInstantiationDependent())
     return new (SemaRef.Context)
         concepts::NestedRequirement(TransConstraint.get());
   return new (SemaRef.Context) concepts::NestedRequirement(
       SemaRef.Context, TransConstraint.get(), Satisfaction);
 }

clang/lib/Sema/TreeTransform.h

 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Sema/Designator.h"
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/Ownership.h"
 #include "clang/Sema/ParsedTemplate.h"
 #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>
 
 using namespace llvm::omp;
 
 namespace clang {
 using namespace sema;
ExprResult Result = getSema().CheckConceptTemplateId(SS, TemplateKWLoc,
                                                          ConceptNameInfo,
                                                          FoundDecl,
                                                          NamedConcept, TALI);
     if (Result.isInvalid())
       return ExprError();
     return Result;
   }
 
+  ExprResult RebuildConceptSpecializationSubstitutionFailure(
+      ConceptSpecializationExpr *E,
+      concepts::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.
   /// Subclasses may override this routine to provide different behavior.
   ExprResult RebuildRequiresExpr(SourceLocation RequiresKWLoc,
                                  RequiresExprBodyDecl *Body,
                                  ArrayRef<ParmVarDecl *> LocalParameters,
                                  ArrayRef<concepts::Requirement *> Requirements,
                                  SourceLocation ClosingBraceLoc) {
     return RequiresExpr::Create(SemaRef.Context, RequiresKWLoc, Body,
                                 LocalParameters, Requirements, ClosingBraceLoc);
   }
 
   concepts::TypeRequirement *
-  RebuildTypeRequirement(
-      concepts::Requirement::SubstitutionDiagnostic *SubstDiag) {
+  RebuildTypeRequirement(concepts::SubstitutionDiagnostic *SubstDiag) {
     return SemaRef.BuildTypeRequirement(SubstDiag);
   }
 
   concepts::TypeRequirement *RebuildTypeRequirement(TypeSourceInfo *T) {
     return SemaRef.BuildTypeRequirement(T);
   }
 
   concepts::ExprRequirement *
-  RebuildExprRequirement(
-      concepts::Requirement::SubstitutionDiagnostic *SubstDiag, bool IsSimple,
-      SourceLocation NoexceptLoc,
+  RebuildExprRequirement(concepts::SubstitutionDiagnostic *SubstDiag,
+                         bool IsSimple, SourceLocation NoexceptLoc,
                          concepts::ExprRequirement::ReturnTypeRequirement Ret) {
     return SemaRef.BuildExprRequirement(SubstDiag, IsSimple, NoexceptLoc,
                                         std::move(Ret));
   }
 
   concepts::ExprRequirement *
   RebuildExprRequirement(Expr *E, bool IsSimple, SourceLocation NoexceptLoc,
                          concepts::ExprRequirement::ReturnTypeRequirement Ret) {
     return SemaRef.BuildExprRequirement(E, IsSimple, NoexceptLoc,
                                         std::move(Ret));
   }
 
   concepts::NestedRequirement *
-  RebuildNestedRequirement(
-      concepts::Requirement::SubstitutionDiagnostic *SubstDiag) {
+  RebuildNestedRequirement(concepts::SubstitutionDiagnostic *SubstDiag) {
     return SemaRef.BuildNestedRequirement(SubstDiag);
   }
 
   concepts::NestedRequirement *RebuildNestedRequirement(Expr *Constraint) {
     return SemaRef.BuildNestedRequirement(Constraint);
   }
 
   /// \brief Build a new Objective-C boxed expression.
TreeTransform<Derived>::TransformTypeTraitExpr(TypeTraitExpr *E) {
 
   if (!getDerived().AlwaysRebuild() && !ArgChanged)
     return E;
 
   return getDerived().RebuildTypeTrait(E->getTrait(), E->getBeginLoc(), Args,
                                        E->getEndLoc());
 }
 
-template<typename Derived>
-ExprResult
-TreeTransform<Derived>::TransformConceptSpecializationExpr(
-                                                 ConceptSpecializationExpr *E) {
+namespace {
+// TODO: refactor to use CreateSubstDiag from SemaTemplateInstantiate.cpp.
+inline concepts::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) concepts::SubstitutionDiagnostic{
+      "todo fill me", ErrorLoc, StringRef(MessageBuf, Message.size())};
+}
+} // namespace
+
+template <typename Derived>
+ExprResult TreeTransform<Derived>::TransformConceptSpecializationExpr(
   const ASTTemplateArgumentListInfo *Old = E->getTemplateArgsAsWritten();
   TemplateArgumentListInfo TransArgs(Old->LAngleLoc, Old->RAngleLoc);
-  if (getDerived().TransformTemplateArguments(Old->getTemplateArgs(),
-                                              Old->NumTemplateArgs, TransArgs))
+  {
+    Sema::SFINAETrap Trap(getSema());
+    if (getDerived().TransformTemplateArguments(
+            Old->getTemplateArgs(), Old->NumTemplateArgs, TransArgs)) {
+      if (!E->isInstantiationDependent() || !Trap.hasErrorOccurred())
         return ExprError();
+      // FIXME: Propagate detailed subtitution falilure diagnostics.
+      return getDerived().RebuildConceptSpecializationSubstitutionFailure(
+          E, CreateSubstDiag(SemaRef, **SemaRef.isSFINAEContext()));
+    }
+  }
 
   return getDerived().RebuildConceptSpecializationExpr(
       E->getNestedNameSpecifierLoc(), E->getTemplateKWLoc(),
       E->getConceptNameInfo(), E->getFoundDecl(), E->getNamedConcept(),
       &TransArgs);
 }
 
 template<typename Derived>
TreeTransform<Derived>::TransformTypeRequirement(
   if (!TransType)
     return nullptr;
   return getDerived().RebuildTypeRequirement(TransType);
 }
 
 template<typename Derived>
 concepts::ExprRequirement *
 TreeTransform<Derived>::TransformExprRequirement(concepts::ExprRequirement *Req) {
-  llvm::PointerUnion<Expr *, concepts::Requirement::SubstitutionDiagnostic *> TransExpr;
+  llvm::PointerUnion<Expr *, concepts::SubstitutionDiagnostic *> TransExpr;
   if (Req->isExprSubstitutionFailure())
     TransExpr = Req->getExprSubstitutionDiagnostic();
   else {
     ExprResult TransExprRes = getDerived().TransformExpr(Req->getExpr());
     if (TransExprRes.isUsable() && TransExprRes.get()->hasPlaceholderType())
       TransExprRes = SemaRef.CheckPlaceholderExpr(TransExprRes.get());
     if (TransExprRes.isInvalid())
       return nullptr;
else if (RetReq.isTypeConstraint()) {
     TransRetReq.emplace(TPL);
   }
   assert(TransRetReq && "All code paths leading here must set TransRetReq");
   if (Expr *E = TransExpr.dyn_cast<Expr *>())
     return getDerived().RebuildExprRequirement(E, Req->isSimple(),
                                                Req->getNoexceptLoc(),
                                                std::move(*TransRetReq));
   return getDerived().RebuildExprRequirement(
-      TransExpr.get<concepts::Requirement::SubstitutionDiagnostic *>(),
-      Req->isSimple(), Req->getNoexceptLoc(), std::move(*TransRetReq));
+      TransExpr.get<concepts::SubstitutionDiagnostic *>(), Req->isSimple(),
+      Req->getNoexceptLoc(), std::move(*TransRetReq));
 }
 
 template<typename Derived>
 concepts::NestedRequirement *
 TreeTransform<Derived>::TransformNestedRequirement(
     concepts::NestedRequirement *Req) {
   if (Req->isSubstitutionFailure()) {
     if (getDerived().AlwaysRebuild())

clang/lib/Serialization/ASTReaderStmt.cpp

readConstraintSatisfaction(ASTRecordReader &Record) {
   if (!Satisfaction.IsSatisfied) {
     unsigned NumDetailRecords = Record.readInt();
     for (unsigned i = 0; i != NumDetailRecords; ++i) {
       Expr *ConstraintExpr = Record.readExpr();
       if (/* IsDiagnostic */Record.readInt()) {
         SourceLocation DiagLocation = Record.readSourceLocation();
         std::string DiagMessage = Record.readString();
         Satisfaction.Details.emplace_back(
-            ConstraintExpr, new (Record.getContext())
-                                ConstraintSatisfaction::SubstitutionDiagnostic{
-                                    DiagLocation, DiagMessage});
+            ConstraintExpr,
+            new (Record.getContext()) concepts::SubstitutionDiagnostic{
+                "todo: fill substituted entity", DiagLocation, DiagMessage});
       } else
         Satisfaction.Details.emplace_back(ConstraintExpr, Record.readExpr());
     }
   }
   return Satisfaction;
 }
 
 void ASTStmtReader::VisitConceptSpecializationExpr(
         ConceptSpecializationExpr *E) {
   VisitExpr(E);
   E->NestedNameSpec = Record.readNestedNameSpecifierLoc();
   E->TemplateKWLoc = Record.readSourceLocation();
   E->ConceptName = Record.readDeclarationNameInfo();
   E->NamedConcept = readDeclAs<ConceptDecl>();
   E->FoundDecl = Record.readDeclAs<NamedDecl>();
   E->SpecDecl = Record.readDeclAs<ImplicitConceptSpecializationDecl>();
   E->ArgsAsWritten = Record.readASTTemplateArgumentListInfo();
   E->Satisfaction = E->isValueDependent() ? nullptr :
       ASTConstraintSatisfaction::Create(Record.getContext(),
                                         readConstraintSatisfaction(Record));
 }
 
-static concepts::Requirement::SubstitutionDiagnostic *
+static concepts::SubstitutionDiagnostic *
 readSubstitutionDiagnostic(ASTRecordReader &Record) {
   std::string SubstitutedEntity = Record.readString();
   SourceLocation DiagLoc = Record.readSourceLocation();
   std::string DiagMessage = Record.readString();
   return new (Record.getContext())
-      concepts::Requirement::SubstitutionDiagnostic{SubstitutedEntity, DiagLoc,
-                                                    DiagMessage};
+      concepts::SubstitutionDiagnostic{SubstitutedEntity, DiagLoc, DiagMessage};
 }
 
 void ASTStmtReader::VisitRequiresExpr(RequiresExpr *E) {
   VisitExpr(E);
   unsigned NumLocalParameters = Record.readInt();
   unsigned NumRequirements = Record.readInt();
   E->RequiresExprBits.RequiresKWLoc = Record.readSourceLocation();
   E->RequiresExprBits.IsSatisfied = Record.readInt();
switch (RK) {
           R = new (Record.getContext())
               concepts::TypeRequirement(Record.readTypeSourceInfo());
       } break;
       case concepts::Requirement::RK_Simple:
       case concepts::Requirement::RK_Compound: {
         auto Status =
             static_cast<concepts::ExprRequirement::SatisfactionStatus>(
                 Record.readInt());
-        llvm::PointerUnion<concepts::Requirement::SubstitutionDiagnostic *,
-                           Expr *> E;
+        llvm::PointerUnion<concepts::SubstitutionDiagnostic *, Expr *> E;
         if (Status == concepts::ExprRequirement::SS_ExprSubstitutionFailure) {
           E = readSubstitutionDiagnostic(Record);
         } else
           E = Record.readExpr();
 
         llvm::Optional<concepts::ExprRequirement::ReturnTypeRequirement> Req;
         ConceptSpecializationExpr *SubstitutedConstraintExpr = nullptr;
         SourceLocation NoexceptLoc;
switch (RK) {
           }
         }
         if (Expr *Ex = E.dyn_cast<Expr *>())
           R = new (Record.getContext()) concepts::ExprRequirement(
                   Ex, RK == concepts::Requirement::RK_Simple, NoexceptLoc,
                   std::move(*Req), Status, SubstitutedConstraintExpr);
         else
           R = new (Record.getContext()) concepts::ExprRequirement(
-                  E.get<concepts::Requirement::SubstitutionDiagnostic *>(),
-                  RK == concepts::Requirement::RK_Simple, NoexceptLoc,
-                  std::move(*Req));
+              E.get<concepts::SubstitutionDiagnostic *>(),
       } break;
       case concepts::Requirement::RK_Nested: {
         if (/* IsSubstitutionDiagnostic */Record.readInt()) {
           R = new (Record.getContext()) concepts::NestedRequirement(
               readSubstitutionDiagnostic(Record));
           break;
         }
         Expr *E = Record.readExpr();

clang/lib/Serialization/ASTWriterStmt.cpp

 //===--- ASTWriterStmt.cpp - Statement and Expression Serialization -------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
 /// Implements serialization for Statements and Expressions.
 ///
 //===----------------------------------------------------------------------===//
 
+#include "clang/AST/ASTConcept.h"
 #include "clang/AST/ExprOpenMP.h"
 #include "clang/Serialization/ASTRecordWriter.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/StmtVisitor.h"
if (!Satisfaction.IsSatisfied) {
     Record.push_back(Satisfaction.NumRecords);
     for (const auto &DetailRecord : Satisfaction) {
       Record.AddStmt(const_cast<Expr *>(DetailRecord.first));
       auto *E = DetailRecord.second.dyn_cast<Expr *>();
       Record.push_back(E == nullptr);
       if (E)
         Record.AddStmt(E);
       else {
-        auto *Diag = DetailRecord.second.get<std::pair<SourceLocation,
-                                                       StringRef> *>();
-        Record.AddSourceLocation(Diag->first);
-        Record.AddString(Diag->second);
+        auto *Diag =
+            DetailRecord.second.get<concepts::SubstitutionDiagnostic *>();
+        Record.AddSourceLocation(Diag->DiagLoc);
+        Record.AddString(Diag->DiagMessage);
       }
     }
   }
 }
 
 static void
-addSubstitutionDiagnostic(
-    ASTRecordWriter &Record,
-    const concepts::Requirement::SubstitutionDiagnostic *D) {
+addSubstitutionDiagnostic(ASTRecordWriter &Record,
+                          const concepts::SubstitutionDiagnostic *D) {
   Record.AddString(D->SubstitutedEntity);
   Record.AddSourceLocation(D->DiagLoc);
   Record.AddString(D->DiagMessage);
 }
 
 void ASTStmtWriter::VisitConceptSpecializationExpr(
         ConceptSpecializationExpr *E) {
   VisitExpr(E);
if (auto *TypeReq = dyn_cast<concepts::TypeRequirement>(R)) {
       if (TypeReq->Status == concepts::TypeRequirement::SS_SubstitutionFailure)
         addSubstitutionDiagnostic(Record, TypeReq->getSubstitutionDiagnostic());
       else
         Record.AddTypeSourceInfo(TypeReq->getType());
     } else if (auto *ExprReq = dyn_cast<concepts::ExprRequirement>(R)) {
       Record.push_back(ExprReq->getKind());
       Record.push_back(ExprReq->Status);
       if (ExprReq->isExprSubstitutionFailure()) {
-        addSubstitutionDiagnostic(Record,
-         ExprReq->Value.get<concepts::Requirement::SubstitutionDiagnostic *>());
+        addSubstitutionDiagnostic(
+            Record, ExprReq->Value.get<concepts::SubstitutionDiagnostic *>());
       } else
         Record.AddStmt(ExprReq->Value.get<Expr *>());
       if (ExprReq->getKind() == concepts::Requirement::RK_Compound) {
         Record.AddSourceLocation(ExprReq->NoexceptLoc);
         const auto &RetReq = ExprReq->getReturnTypeRequirement();
         if (RetReq.isSubstitutionFailure()) {
           Record.push_back(2);
           addSubstitutionDiagnostic(Record, RetReq.getSubstitutionDiagnostic());

clang/test/SemaTemplate/concepts.cpp

struct __iterator_traits_member_pointer_or_arrow_or_void<_Ip> {
   using type = decltype(declval<_Ip&>().operator->());
 };
 
 
 void use2() {
   __iterator_traits_member_pointer_or_arrow_or_void<counted_iterator<int>> f;
 }
 }// namespace InheritedFromPartialSpec
+
+namespace nested_requirements {
+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 'todo fill me' would be invalid: 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 'todo fill me' would be invalid: 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'}}
+}
+}
 No newline at end of file