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

This reverts commit e9972de.

Author: ronlieb

clang/docs/InternalsManual.rst

@@ -2859,67 +2859,6 @@ 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,10 +160,6 @@ 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,20 +28,10 @@ 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 {
@@ -58,13 +48,16 @@ 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<UnsatisfiedConstraintRecord, 4> Details;
+  llvm::SmallVector<Detail, 4> Details;
 
   void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &C) {
     Profile(ID, C, ConstraintOwner, TemplateArgs);
@@ -76,12 +69,19 @@ class ConstraintSatisfaction : public llvm::FoldingSetNode {
 
   bool HasSubstitutionFailure() {
     for (const auto &Detail : Details)
-      if (Detail.dyn_cast<const ConstraintSubstitutionDiagnostic *>())
+      if (Detail.dyn_cast<SubstitutionDiagnostic *>())
         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,10 +101,6 @@ struct ASTConstraintSatisfaction final :
     return getTrailingObjects() + NumRecords;
   }
 
-  ArrayRef<UnsatisfiedConstraintRecord> records() const {
-    return {begin(), end()};
-  }
-
   ASTConstraintSatisfaction(const ASTContext &C,
                             const ConstraintSatisfaction &Satisfaction);
   ASTConstraintSatisfaction(const ASTContext &C,
@@ -286,11 +282,6 @@ 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,6 +3877,7 @@ 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,7 +65,6 @@
 #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"
@@ -11695,9 +11694,8 @@ class Sema final : public SemaBase {
   ExprResult
   CheckConceptTemplateId(const CXXScopeSpec &SS, SourceLocation TemplateKWLoc,
                          const DeclarationNameInfo &ConceptNameInfo,
-                         NamedDecl *FoundDecl, TemplateDecl *NamedConcept,
-                         const TemplateArgumentListInfo *TemplateArgs,
-                         bool DoCheckConstraintSatisfaction = true);
+                         NamedDecl *FoundDecl, ConceptDecl *NamedConcept,
+                         const TemplateArgumentListInfo *TemplateArgs);
 
   void diagnoseMissingTemplateArguments(TemplateName Name, SourceLocation Loc);
   void diagnoseMissingTemplateArguments(const CXXScopeSpec &SS,
@@ -12027,13 +12025,6 @@ 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,
@@ -12792,18 +12783,6 @@ 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.
   ///
@@ -12820,9 +12799,6 @@ 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) {
@@ -13120,9 +13096,6 @@ 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;
 
@@ -13173,10 +13146,8 @@ class Sema final : public SemaBase {
     CodeSynthesisContext()
         : Kind(TemplateInstantiation),
           SavedInNonInstantiationSFINAEContext(false),
-          InConstraintSubstitution(false),
-          InParameterMappingSubstitution(false), Entity(nullptr),
-          Template(nullptr), TemplateArgs(nullptr), NumTemplateArgs(0),
-          DeductionInfo(nullptr) {}
+          InConstraintSubstitution(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.
@@ -13388,11 +13359,6 @@ 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.
   ///
@@ -13854,12 +13820,6 @@ 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
@@ -14744,10 +14704,6 @@ 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
@@ -14772,12 +14728,44 @@ class Sema final : public SemaBase {
   /// \returns true if an error occurred and satisfaction could not be checked,
   /// false otherwise.
   bool CheckConstraintSatisfaction(
-      ConstrainedDeclOrNestedRequirement Entity,
+      const NamedDecl *Template,
       ArrayRef<AssociatedConstraint> AssociatedConstraints,
       const MultiLevelTemplateArgumentList &TemplateArgLists,
-      SourceRange TemplateIDRange, ConstraintSatisfaction &Satisfaction,
-      const ConceptReference *TopLevelConceptId = nullptr,
-      Expr **ConvertedExpr = nullptr);
+      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);
 
   /// \brief Check whether the given non-dependent constraint expression is
   /// satisfied. Returns false and updates Satisfaction with the satisfaction
@@ -14843,17 +14831,16 @@ 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 ConceptSpecializationExpr *ConstraintExpr,
+  DiagnoseUnsatisfiedConstraint(const ASTConstraintSatisfaction &Satisfaction,
                                 bool First = true);
 
   const NormalizedConstraint *getNormalizedAssociatedConstraints(
-      ConstrainedDeclOrNestedRequirement Entity,
+      const NamedDecl *ConstrainedDecl,
       ArrayRef<AssociatedConstraint> AssociatedConstraints);
 
   /// \brief Check whether the given declaration's associated constraints are
@@ -14878,15 +14865,6 @@ 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
@@ -14897,11 +14875,8 @@ 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<ConstrainedDeclOrNestedRequirement, NormalizedConstraint *>
-      NormalizationCache;
+  llvm::DenseMap<const NamedDecl *, NormalizedConstraint *> NormalizationCache;
 
-  /// Cache whether the associated constraint of a declaration
-  /// is satisfied.
   llvm::ContextualFoldingSet<ConstraintSatisfaction, const ASTContext &>
       SatisfactionCache;