[Clang] Normalize constraints before checking for satisfaction (#161671)

In the standard, constraint satisfaction checking is done on the normalized form of a constraint. Clang instead substitutes on the non-normalized form, which causes us to report substitution failures in template arguments or concept ids, which is non-conforming but unavoidable without a parameter mapping This patch normalizes before satisfaction checking. However, we preserve concept-id nodes in the normalized form, solely for diagnostics purposes. This addresses #61811 and related concepts conformance bugs, ideally to make the remaining implementation of concept template parameters easier Fixes #135190 Fixes #61811 Co-authored-by: Younan Zhang [zyn7109@gmail.com](mailto:zyn7109@gmail.com) --------- Co-authored-by: Younan Zhang <zyn7109@gmail.com>

Author: cor3ntin

clang/docs/InternalsManual.rst

@@ -2859,6 +2859,67 @@ This library is called by the :ref:`Parser library <Parser>` during parsing to
 do semantic analysis of the input. For valid programs, Sema builds an AST for
 parsed constructs.
 
+
+Concept Satisfaction Checking and Subsumption
+---------------------------------------------
+
+As per the C++ standard, constraints are `normalized <https://eel.is/c++draft/temp.constr.normal>`_
+and the normal form is used both for subsumption, and constraint checking.
+Both depend on a parameter mapping that substitutes lazily. In particular,
+we should not substitute in unused arguments.
+
+Clang follows the order of operations prescribed by the standard.
+
+Normalization happens prior to satisfaction and subsumption
+and is handled by ``NormalizedConstraint``.
+
+Clang preserves in the normalized form intermediate concept-ids
+(``ConceptIdConstraint``) This is used for diagnostics only and no substitution
+happens in a ConceptIdConstraint if its expression is satisfied.
+
+The normal form of the associated constraints of a declaration is cached in
+Sema::NormalizationCache such that it is only computed once.
+
+A ``NormalizedConstraint`` is a recursive data structure, where each node
+contains a parameter mapping, represented by the indexes of all parameter
+being used.
+
+Checking satisfaction is done by ``ConstraintSatisfactionChecker``, recursively
+walking ``NormalizedConstraint``. At each level, we substitute the outermost
+level of the template arguments referenced in the parameter mapping of a
+normalized expression (``MultiLevelTemplateArgumentList``).
+
+For the following example,
+
+.. code-block:: c++
+
+ template <typename T>
+ concept A = __is_same(T, int);
+
+ template <typename U>
+ concept B = A<U> && __is_same(U, int);
+
+The normal form of B is
+
+.. code-block:: c++
+
+ __is_same(T, int) /*T->U, innermost level*/
+ && __is_same(U, int) {U->U} /*T->U, outermost level*/
+
+After substitution in the mapping, we substitute in the constraint expression
+using that copy of the ``MultiLevelTemplateArgumentList``, and then evaluate it.
+
+Because this is expensive, it is cached in
+``UnsubstitutedConstraintSatisfactionCache``.
+
+Any error during satisfaction is recorded in ``ConstraintSatisfaction``.
+for nested requirements, ``ConstraintSatisfaction`` is stored (including
+diagnostics) in the AST, which is something we might want to improve.
+
+When an atomic constraint is not satified, we try to substitute into any
+enclosing concept-id using the same mechanism described above, for
+diagnostics purpose, and inject that in the ``ConstraintSatisfaction``.
+
 .. _CodeGen:
 
 The CodeGen Library

clang/docs/ReleaseNotes.rst

@@ -160,6 +160,10 @@ C++23 Feature Support
 C++20 Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 
+- Clang now normalizes constraints before checking whether they are satisfied, as mandated by the standard.
+ As a result, Clang no longer incorrectly diagnoses substitution failures in template arguments only
+ used in concept-ids, and produces better diagnostics for satisfaction failure. (#GH61811) (#GH135190)
+
 C++17 Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 

clang/include/clang/AST/ASTConcept.h

@@ -28,10 +28,20 @@ namespace clang {
 
 class ConceptDecl;
 class TemplateDecl;
+class ConceptReference;
 class Expr;
 class NamedDecl;
 struct PrintingPolicy;
 
+/// Unsatisfied constraint expressions if the template arguments could be
+/// substituted into them, or a diagnostic if substitution resulted in
+/// an invalid expression.
+///
+using ConstraintSubstitutionDiagnostic = std::pair<SourceLocation, StringRef>;
+using UnsatisfiedConstraintRecord =
+ llvm::PointerUnion<const Expr *, const ConceptReference *,
+ const ConstraintSubstitutionDiagnostic *>;
+
 /// The result of a constraint satisfaction check, containing the necessary
 /// information to diagnose an unsatisfied constraint.
 class ConstraintSatisfaction : public llvm::FoldingSetNode {
@@ -48,16 +58,13 @@ class ConstraintSatisfaction : public llvm::FoldingSetNode {
  ArrayRef<TemplateArgument> TemplateArgs)
  : ConstraintOwner(ConstraintOwner), TemplateArgs(TemplateArgs) {}
 
- using SubstitutionDiagnostic = std::pair<SourceLocation, StringRef>;
- using Detail = llvm::PointerUnion<Expr *, SubstitutionDiagnostic *>;
-
  bool IsSatisfied = false;
  bool ContainsErrors = false;
 
  /// \brief 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<Detail, 4> Details;
+ llvm::SmallVector<UnsatisfiedConstraintRecord, 4> Details;
 
  void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &C) {
  Profile(ID, C, ConstraintOwner, TemplateArgs);
@@ -69,19 +76,12 @@ class ConstraintSatisfaction : public llvm::FoldingSetNode {
 
  bool HasSubstitutionFailure() {
  for (const auto &Detail : Details)
- if (Detail.dyn_cast<SubstitutionDiagnostic *>())
+ if (Detail.dyn_cast<const ConstraintSubstitutionDiagnostic *>())
  return true;
  return false;
  }
 };
 
-/// 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 =
- llvm::PointerUnion<Expr *, std::pair<SourceLocation, StringRef> *>;
-
 /// \brief The result of a constraint satisfaction check, containing the
 /// necessary information to diagnose an unsatisfied constraint.
 ///
@@ -101,6 +101,10 @@ struct ASTConstraintSatisfaction final :
  return getTrailingObjects() + NumRecords;
  }
 
+ ArrayRef<UnsatisfiedConstraintRecord> records() const {
+ return {begin(), end()};
+ }
+
  ASTConstraintSatisfaction(const ASTContext &C,
  const ConstraintSatisfaction &Satisfaction);
  ASTConstraintSatisfaction(const ASTContext &C,
@@ -282,6 +286,11 @@ class TypeConstraint {
  }
 };
 
+/// Insertion operator for diagnostics. This allows sending ConceptReferences's
+/// into a diagnostic with <<.
+const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
+ const ConceptReference *C);
+
 } // clang
 
 #endif // LLVM_CLANG_AST_ASTCONCEPT_H

clang/include/clang/AST/ASTContext.h

@@ -3877,7 +3877,6 @@ typename clang::LazyGenerationalUpdatePtr<Owner, T, Update>::ValueType
  return new (Ctx) LazyData(Source, Value);
  return Value;
 }
-
 template <> struct llvm::DenseMapInfo<llvm::FoldingSetNodeID> {
  static FoldingSetNodeID getEmptyKey() { return FoldingSetNodeID{}; }
 

clang/include/clang/Sema/Sema.h

@@ -65,6 +65,7 @@
 #include "clang/Sema/Redeclaration.h"
 #include "clang/Sema/Scope.h"
 #include "clang/Sema/SemaBase.h"
+#include "clang/Sema/SemaConcept.h"
 #include "clang/Sema/TypoCorrection.h"
 #include "clang/Sema/Weak.h"
 #include "llvm/ADT/APInt.h"
@@ -11694,8 +11695,9 @@ class Sema final : public SemaBase {
  ExprResult
  CheckConceptTemplateId(const CXXScopeSpec &SS, SourceLocation TemplateKWLoc,
  const DeclarationNameInfo &ConceptNameInfo,
- NamedDecl *FoundDecl, ConceptDecl *NamedConcept,
- const TemplateArgumentListInfo *TemplateArgs);
+ NamedDecl *FoundDecl, TemplateDecl *NamedConcept,
+ const TemplateArgumentListInfo *TemplateArgs,
+ bool DoCheckConstraintSatisfaction = true);
 
  void diagnoseMissingTemplateArguments(TemplateName Name, SourceLocation Loc);
  void diagnoseMissingTemplateArguments(const CXXScopeSpec &SS,
@@ -12025,6 +12027,13 @@ class Sema final : public SemaBase {
  bool UpdateArgsWithConversions = true,
  bool *ConstraintsNotSatisfied = nullptr);
 
+ bool CheckTemplateArgumentList(
+ TemplateDecl *Template, TemplateParameterList *Params,
+ SourceLocation TemplateLoc, TemplateArgumentListInfo &TemplateArgs,
+ const DefaultArguments &DefaultArgs, bool PartialTemplateArgs,
+ CheckTemplateArgumentInfo &CTAI, bool UpdateArgsWithConversions = true,
+ bool *ConstraintsNotSatisfied = nullptr);
+
  bool CheckTemplateTypeArgument(
  TemplateTypeParmDecl *Param, TemplateArgumentLoc &Arg,
  SmallVectorImpl<TemplateArgument> &SugaredConverted,
@@ -12783,6 +12792,18 @@ class Sema final : public SemaBase {
  void MarkUsedTemplateParameters(const Expr *E, bool OnlyDeduced,
  unsigned Depth, llvm::SmallBitVector &Used);
 
+ /// Mark which template parameters are named in a given expression.
+ ///
+ /// Unlike MarkUsedTemplateParameters, this excludes parameter that
+ /// are used but not directly named by an expression - i.e. it excludes
+ /// any template parameter that denotes the type of a referenced NTTP.
+ ///
+ /// \param Used a bit vector whose elements will be set to \c true
+ /// to indicate when the corresponding template parameter will be
+ /// deduced.
+ void MarkUsedTemplateParametersForSubsumptionParameterMapping(
+ const Expr *E, unsigned Depth, llvm::SmallBitVector &Used);
+
  /// Mark which template parameters can be deduced from a given
  /// template argument list.
  ///
@@ -12799,6 +12820,9 @@ class Sema final : public SemaBase {
  void MarkUsedTemplateParameters(ArrayRef<TemplateArgument> TemplateArgs,
  unsigned Depth, llvm::SmallBitVector &Used);
 
+ void MarkUsedTemplateParameters(ArrayRef<TemplateArgumentLoc> TemplateArgs,
+ unsigned Depth, llvm::SmallBitVector &Used);
+
  void
  MarkDeducedTemplateParameters(const FunctionTemplateDecl *FunctionTemplate,
  llvm::SmallBitVector &Deduced) {
@@ -13096,6 +13120,9 @@ class Sema final : public SemaBase {
  /// Whether we're substituting into constraints.
  bool InConstraintSubstitution;
 
+ /// Whether we're substituting into the parameter mapping of a constraint.
+ bool InParameterMappingSubstitution;
+
  /// The point of instantiation or synthesis within the source code.
  SourceLocation PointOfInstantiation;
 
@@ -13146,8 +13173,10 @@ class Sema final : public SemaBase {
  CodeSynthesisContext()
  : Kind(TemplateInstantiation),
  SavedInNonInstantiationSFINAEContext(false),
- InConstraintSubstitution(false), Entity(nullptr), Template(nullptr),
- TemplateArgs(nullptr), NumTemplateArgs(0), DeductionInfo(nullptr) {}
+ InConstraintSubstitution(false),
+ InParameterMappingSubstitution(false), Entity(nullptr),
+ Template(nullptr), TemplateArgs(nullptr), NumTemplateArgs(0),
+ DeductionInfo(nullptr) {}
 
  /// Determines whether this template is an actual instantiation
  /// that should be counted toward the maximum instantiation depth.
@@ -13359,6 +13388,11 @@ class Sema final : public SemaBase {
  const MultiLevelTemplateArgumentList &TemplateArgs,
  TemplateArgumentListInfo &Outputs);
 
+ bool SubstTemplateArgumentsInParameterMapping(
+ ArrayRef<TemplateArgumentLoc> Args, SourceLocation BaseLoc,
+ const MultiLevelTemplateArgumentList &TemplateArgs,
+ TemplateArgumentListInfo &Out, bool BuildPackExpansionTypes);
+
  /// Retrieve the template argument list(s) that should be used to
  /// instantiate the definition of the given declaration.
  ///
@@ -13820,6 +13854,12 @@ class Sema final : public SemaBase {
  CodeSynthesisContexts.back().InConstraintSubstitution;
  }
 
+ bool inParameterMappingSubstitution() const {
+ return !CodeSynthesisContexts.empty() &&
+ CodeSynthesisContexts.back().InParameterMappingSubstitution &&
+ !inConstraintSubstitution();
+ }
+
  using EntityPrinter = llvm::function_ref<void(llvm::raw_ostream &)>;
 
  /// \brief create a Requirement::SubstitutionDiagnostic with only a
@@ -14704,6 +14744,10 @@ class Sema final : public SemaBase {
  SatisfactionStack.swap(NewSS);
  }
 
+ using ConstrainedDeclOrNestedRequirement =
+ llvm::PointerUnion<const NamedDecl *,
+ const concepts::NestedRequirement *>;
+
  /// Check whether the given expression is a valid constraint expression.
  /// A diagnostic is emitted if it is not, false is returned, and
  /// PossibleNonPrimary will be set to true if the failure might be due to a
@@ -14728,44 +14772,12 @@ class Sema final : public SemaBase {
  /// \returns true if an error occurred and satisfaction could not be checked,
  /// false otherwise.
  bool CheckConstraintSatisfaction(
- const NamedDecl *Template,
+ ConstrainedDeclOrNestedRequirement Entity,
  ArrayRef<AssociatedConstraint> AssociatedConstraints,
  const MultiLevelTemplateArgumentList &TemplateArgLists,
- SourceRange TemplateIDRange, ConstraintSatisfaction &Satisfaction) {
- llvm::SmallVector<Expr *, 4> Converted;
- return CheckConstraintSatisfaction(Template, AssociatedConstraints,
- Converted, TemplateArgLists,
- TemplateIDRange, Satisfaction);
- }
-
- /// \brief Check whether the given list of constraint expressions are
- /// satisfied (as if in a 'conjunction') given template arguments.
- /// Additionally, takes an empty list of Expressions which is populated with
- /// the instantiated versions of the ConstraintExprs.
- /// \param Template the template-like entity that triggered the constraints
- /// check (either a concept or a constrained entity).
- /// \param ConstraintExprs a list of constraint expressions, treated as if
- /// they were 'AND'ed together.
- /// \param ConvertedConstraints a out parameter that will get populated with
- /// the instantiated version of the ConstraintExprs if we successfully checked
- /// satisfaction.
- /// \param TemplateArgList the multi-level list of template arguments to
- /// substitute into the constraint expression. This should be relative to the
- /// top-level (hence multi-level), since we need to instantiate fully at the
- /// time of checking.
- /// \param TemplateIDRange The source range of the template id that
- /// caused the constraints check.
- /// \param Satisfaction if true is returned, will contain details of the
- /// satisfaction, with enough information to diagnose an unsatisfied
- /// expression.
- /// \returns true if an error occurred and satisfaction could not be checked,
- /// false otherwise.
- bool CheckConstraintSatisfaction(
- const NamedDecl *Template,
- ArrayRef<AssociatedConstraint> AssociatedConstraints,
- llvm::SmallVectorImpl<Expr *> &ConvertedConstraints,
- const MultiLevelTemplateArgumentList &TemplateArgList,
- SourceRange TemplateIDRange, ConstraintSatisfaction &Satisfaction);
+ SourceRange TemplateIDRange, ConstraintSatisfaction &Satisfaction,
+ const ConceptReference *TopLevelConceptId = nullptr,
+ Expr **ConvertedExpr = nullptr);
 
  /// \brief Check whether the given non-dependent constraint expression is
  /// satisfied. Returns false and updates Satisfaction with the satisfaction
@@ -14831,16 +14843,17 @@ class Sema final : public SemaBase {
  /// \param First whether this is the first time an unsatisfied constraint is
  /// diagnosed for this error.
  void DiagnoseUnsatisfiedConstraint(const ConstraintSatisfaction &Satisfaction,
+ SourceLocation Loc = {},
  bool First = true);
 
  /// \brief Emit diagnostics explaining why a constraint expression was deemed
  /// unsatisfied.
  void
- DiagnoseUnsatisfiedConstraint(const ASTConstraintSatisfaction &Satisfaction,
+ DiagnoseUnsatisfiedConstraint(const ConceptSpecializationExpr *ConstraintExpr,
  bool First = true);
 
  const NormalizedConstraint *getNormalizedAssociatedConstraints(
- const NamedDecl *ConstrainedDecl,
+ ConstrainedDeclOrNestedRequirement Entity,
  ArrayRef<AssociatedConstraint> AssociatedConstraints);
 
  /// \brief Check whether the given declaration's associated constraints are
@@ -14865,6 +14878,15 @@ class Sema final : public SemaBase {
  const NamedDecl *D1, ArrayRef<AssociatedConstraint> AC1,
  const NamedDecl *D2, ArrayRef<AssociatedConstraint> AC2);
 
+ /// Cache the satisfaction of an atomic constraint.
+ /// The key is based on the unsubstituted expression and the parameter
+ /// mapping. This lets us not substituting the mapping more than once,
+ /// which is (very!) expensive.
+ /// FIXME: this should be private.
+ llvm::DenseMap<llvm::FoldingSetNodeID,
+ UnsubstitutedConstraintSatisfactionCacheResult>
+ UnsubstitutedConstraintSatisfactionCache;
+
 private:
  /// Caches pairs of template-like decls whose associated constraints were
  /// checked for subsumption and whether or not the first's constraints did in
@@ -14875,8 +14897,11 @@ class Sema final : public SemaBase {
  /// constrained declarations). If an error occurred while normalizing the
  /// associated constraints of the template or concept, nullptr will be cached
  /// here.
- llvm::DenseMap<const NamedDecl *, NormalizedConstraint *> NormalizationCache;
+ llvm::DenseMap<ConstrainedDeclOrNestedRequirement, NormalizedConstraint *>
+ NormalizationCache;
 
+ /// Cache whether the associated constraint of a declaration
+ /// is satisfied.
  llvm::ContextualFoldingSet<ConstraintSatisfaction, const ASTContext &>
  SatisfactionCache;