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

120–121

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

101

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

158

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

101

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.

158

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

120–121

Could you please elaborate what is your suggestion here ?

[erichkeane]

clang/include/clang/AST/ExprConcepts.h

101

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/docs/ReleaseNotes.rst

- Clang implements DR2621, correcting a defect in ``using enum`` handling.  The
   name is found via ordinary lookup so typedefs are found.
 - Implemented `P0634r3 <https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0634r3.html>`_,
   which removes the requirement for the ``typename`` keyword in certain contexts.
 - Implemented The Equality Operator You Are Looking For (`P2468 <http://wg21.link/p2468r2>`_).
 - Implemented `P2113R0: Proposed resolution for 2019 comment CA 112 <https://wg21.link/P2113R0>`_
   ([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
 ^^^^^^^^^^^^^^^^^^^^^
 
 - Support label at end of compound statement (`P2324 <https://wg21.link/p2324r2>`_).
 - Implemented `P1169R4: static operator() <https://wg21.link/P1169R4>`_.
 - Implemented "char8_t Compatibility and Portability Fix" (`P2513R3 <https://wg21.link/P2513R3>`_).
   This Change was applied to C++20 as a Defect Report.

clang/include/clang/AST/ExprConcepts.h

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 {
+    assert(SpecDecl);
     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.
public:
 
   SourceLocation getBeginLoc() const LLVM_READONLY {
     if (auto QualifierLoc = getNestedNameSpecifierLoc())
       return QualifierLoc.getBeginLoc();
     return ConceptName.getBeginLoc();
   }
 
   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.
     return ArgsAsWritten->RAngleLoc.isValid() ? ArgsAsWritten->RAngleLoc
                                               : ConceptName.getEndLoc();
   }
 
   // Iterators
   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 {
 
 /// \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;

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def note_expr_requirement_constraints_not_satisfied_simple : Note<
   "%select{and|because}0 %1 does not satisfy %2:">;
 def note_type_requirement_substitution_error : Note<
   "%select{and|because}0 '%1' would be invalid: %2">;
 def note_type_requirement_unknown_substitution_error : Note<
   "%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<
   "similar constraint expressions not considered equivalent; constraint "
   "expressions cannot be considered equivalent unless they originate from the "
   "same concept">;
 def note_ambiguous_atomic_constraints_similar_expression : Note<
   "similar constraint expression here">;

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

 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

 //===-- SemaConcept.cpp - Semantic Analysis for Constraints and Concepts --===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file implements semantic analysis for C++ constraints and concepts.
 //
 //===----------------------------------------------------------------------===//
 
 #include "TreeTransform.h"
+#include "clang/AST/ASTConcept.h"
 #include "clang/Sema/SemaConcept.h"
 #include "clang/Sema/Sema.h"
 #include "clang/Sema/SemaInternal.h"
 #include "clang/Sema/SemaDiagnostic.h"
 #include "clang/Sema/TemplateDeduction.h"
 #include "clang/Sema/Template.h"
 #include "clang/Sema/Overload.h"
 #include "clang/Sema/Initialization.h"
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<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(),
           diag::
           note_single_arg_concept_specialization_constraint_evaluated_to_false)
           << (int)First
           << CSE->getTemplateArgsAsWritten()->arguments()[0].getArgument()
           << CSE->getNamedConcept();

clang/lib/Sema/TreeTransform.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_LIB_SEMA_TREETRANSFORM_H
 #define LLVM_CLANG_LIB_SEMA_TREETRANSFORM_H
 
 #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"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprConcepts.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExprOpenMP.h"
 #include "clang/AST/OpenMPClause.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
 #include "clang/AST/StmtOpenMP.h"
 #include "clang/Basic/DiagnosticParse.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,
+      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.
   /// Subclasses may override this routine to provide different behavior.
   ExprResult RebuildRequiresExpr(SourceLocation RequiresKWLoc,
                                  RequiresExprBodyDecl *Body,
                                  ArrayRef<ParmVarDecl *> LocalParameters,
                                  ArrayRef<concepts::Requirement *> Requirements,
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: 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(
   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();
+      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>

clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp

namespace std_example {
   static_assert(C2<int>); // expected-note{{because 'int' does not satisfy 'C2'}} expected-error{{static assertion failed}}
 }
 
 template<typename T>
 concept K = requires (T::Type X) {
   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>();
+}
+}