[Clang][WIP] Normalize constraints before checking for satisfaction #141776

cor3ntin · 2025-05-28T14:33:04Z

In the standard, constraint satisfaction checking is done on the normalized form of a constraint.

Clang instead substitute on the non-normalized form, which cause us to report substitution failures in template arguments or concept ids, which is non-conforming but unavoidable witjout a parameter mapping

This patch normalizes before satisfaction checking. However, we preserve concept-id nodes in the normalized form, solely for diagnostics purposes.

This is a very incomplete attempt at addressing #61811 and related concepts related conformance bugs, ideally to make the implementation of concept template parameters easier

There is stil ~20 failing test files, mostly caused by poor adjustement of the template depth.

github-actions · 2025-06-01T11:33:36Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff HEAD~1 HEAD --extensions cpp,h -- clang/include/clang/AST/ASTConcept.h clang/include/clang/AST/TemplateBase.h clang/include/clang/Sema/Sema.h clang/include/clang/Sema/SemaConcept.h clang/include/clang/Sema/Template.h clang/lib/AST/ASTConcept.cpp clang/lib/AST/ASTContext.cpp clang/lib/Sema/SemaConcept.cpp clang/lib/Sema/SemaDeclCXX.cpp clang/lib/Sema/SemaExprCXX.cpp clang/lib/Sema/SemaTemplate.cpp clang/lib/Sema/SemaTemplateDeduction.cpp clang/lib/Sema/SemaTemplateInstantiate.cpp clang/lib/Sema/SemaTemplateVariadic.cpp clang/lib/Sema/TreeTransform.h clang/lib/Serialization/ASTWriterStmt.cpp clang/test/CXX/drs/cwg25xx.cpp clang/test/CXX/expr/expr.prim/expr.prim.id/p3.cpp clang/test/CXX/expr/expr.prim/expr.prim.req/compound-requirement.cpp clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp clang/test/CXX/temp/temp.constr/temp.constr.normal/p1.cpp clang/test/SemaCXX/cxx23-assume.cpp clang/test/SemaCXX/cxx2c-fold-exprs.cpp clang/test/SemaCXX/invalid-requirement-requires-expr.cpp clang/test/SemaCXX/overload-resolution-deferred-templates.cpp clang/test/SemaTemplate/concepts-recovery-expr.cpp clang/test/SemaTemplate/concepts.cpp clang/test/SemaTemplate/instantiate-abbreviated-template.cpp clang/test/SemaTemplate/instantiate-expanded-type-constraint.cpp clang/test/SemaTemplate/instantiate-template-argument.cpp

View the diff from clang-format here.

diff --git a/clang/include/clang/Sema/SemaConcept.h b/clang/include/clang/Sema/SemaConcept.h
index 29759a730..9f19ed045 100644
--- a/clang/include/clang/Sema/SemaConcept.h
+++ b/clang/include/clang/Sema/SemaConcept.h
@@ -287,11 +287,11 @@ protected:
 
 public:
   using NormalizedConstraint::getParameterMapping;
+  using NormalizedConstraint::getUsedTemplateParamList;
   using NormalizedConstraint::hasMatchingParameterMapping;
   using NormalizedConstraint::hasParameterMapping;
   using NormalizedConstraint::mappingOccurenceList;
   using NormalizedConstraint::updateParameterMapping;
-  using NormalizedConstraint::getUsedTemplateParamList;
 
   const NamedDecl *getConstraintDecl() const { return Atomic.ConstraintDecl; }
 
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 232003f25..0b44a7b7b 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -556,7 +556,7 @@ static bool calculateConstraintSatisfaction(
   if (!NumExpansions)
     return false;
 
-  if(*NumExpansions == 0) {
+  if (*NumExpansions == 0) {
     Satisfaction.IsSatisfied = Conjunction;
     return true;
   }
@@ -573,7 +573,8 @@ static bool calculateConstraintSatisfaction(
     if (!Success && Conjunction)
       return false;
     if (!Conjunction && Satisfaction.IsSatisfied) {
-      Satisfaction.Details.erase(Satisfaction.Details.begin() + EffectiveDetailEndIndex,
+      Satisfaction.Details.erase(Satisfaction.Details.begin() +
+                                     EffectiveDetailEndIndex,
                                  Satisfaction.Details.end());
       break;
     }
@@ -691,10 +692,11 @@ static bool calculateConstraintSatisfaction(
   Satisfaction.IsSatisfied = false;
 
   Ok = calculateConstraintSatisfaction(S, Constraint.getRHS(), Template,
-                                         TemplateNameLoc, MLTAL, Satisfaction,
-                                         PackSubstitutionIndex);
+                                       TemplateNameLoc, MLTAL, Satisfaction,
+                                       PackSubstitutionIndex);
   if (Ok && Satisfaction.IsSatisfied && !Satisfaction.ContainsErrors)
-    Satisfaction.Details.erase(Satisfaction.Details.begin() + EffectiveDetailEndIndex,
+    Satisfaction.Details.erase(Satisfaction.Details.begin() +
+                                   EffectiveDetailEndIndex,
                                Satisfaction.Details.end());
   return Ok;
 }
@@ -1389,9 +1391,10 @@ static void diagnoseUnsatisfiedRequirement(Sema &S,
                                            concepts::NestedRequirement *Req,
                                            bool First) {
   DiagnoseUnsatisfiedConstraint(S, Req->getConstraintSatisfaction().records(),
-                                Req->hasInvalidConstraint() ? SourceLocation() :
-                                Req->getConstraintExpr()->getExprLoc(), First,
-                                Req);
+                                Req->hasInvalidConstraint()
+                                    ? SourceLocation()
+                                    : Req->getConstraintExpr()->getExprLoc(),
+                                First, Req);
 }
 
 static void diagnoseWellFormedUnsatisfiedConstraintExpr(Sema &S,
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index b7c463d1c..b3ba0023c 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -4756,7 +4756,7 @@ ExprResult Sema::CheckConceptTemplateId(
     LocalInstantiationScope Scope(*this);
 
     EnterExpressionEvaluationContext EECtx{
-      *this, ExpressionEvaluationContext::Unevaluated, CSD};
+        *this, ExpressionEvaluationContext::Unevaluated, CSD};
 
     Error = CheckConstraintSatisfaction(
         NamedConcept, AssociatedConstraint(NamedConcept->getConstraintExpr()),
@@ -4764,8 +4764,7 @@ ExprResult Sema::CheckConceptTemplateId(
         SourceRange(SS.isSet() ? SS.getBeginLoc() : ConceptNameInfo.getLoc(),
                     TemplateArgs->getRAngleLoc()),
         Satisfaction, CL);
-
-    }
+  }
 
   if (!DoCheckConstraintSatisfaction)
     Satisfaction.IsSatisfied = false;
@@ -5899,9 +5898,11 @@ bool Sema::CheckTemplateArgumentList(
       // For constraint parameter mapping, we have already built a pack in
       // TransformTemplateArguments
       // if (inParameterMappingSubstitution()) {
-      //   llvm::copy(SugaredArgumentPack, std::back_inserter(CTAI.SugaredConverted));
+      //   llvm::copy(SugaredArgumentPack,
+      //   std::back_inserter(CTAI.SugaredConverted));
       //   SugaredArgumentPack.clear();
-      //   llvm::copy(CanonicalArgumentPack, std::back_inserter(CTAI.CanonicalConverted));
+      //   llvm::copy(CanonicalArgumentPack,
+      //   std::back_inserter(CTAI.CanonicalConverted));
       //   CanonicalArgumentPack.clear();
       //   ++Param;
       //   continue;
diff --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index 3d9d5aff6..e7e4359c4 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -1443,10 +1443,10 @@ public:
   DeclarationName getBaseEntity() { return Entity; }
 
   bool TryExpandParameterPacks(SourceLocation EllipsisLoc,
-                                SourceRange PatternRange,
-                                ArrayRef<UnexpandedParameterPack> Unexpanded,
-                                bool &ShouldExpand, bool &RetainExpansion,
-                                UnsignedOrNone &NumExpansions) {
+                               SourceRange PatternRange,
+                               ArrayRef<UnexpandedParameterPack> Unexpanded,
+                               bool &ShouldExpand, bool &RetainExpansion,
+                               UnsignedOrNone &NumExpansions) {
     if (SemaRef.CurrentInstantiationScope &&
         SemaRef.inConstraintSubstitution()) {
       for (UnexpandedParameterPack ParmPack : Unexpanded) {

In the standard, constraint satisfaction checking is done on the normalized form of a constraint. Clang instead substitute on the non-normalized form, which cause us to report substitution failures in template arguments or concept ids, which is non-conforming but unavoidable witjout a parameter mapping This patch normalizes before satisfaction checking. However, we preserve concept-id nodes in the normalized form, solely for diagnostics purposes. This is a very incomplete attempt at addressing llvm#61811 and related concepts related conformance bugs, ideally to make the implementation of concept template parameters easier There is stil ~20 failing test files, mostly caused by poor adjustement of the template depth.

…zation Fix the access checking after normalization

clang/test/SemaTemplate/alias-template-with-lambdas.cpp

…zation Fix SemaTemplate/concepts-lambda.cpp

…zation Stash recent changes

AST/ByteCode/libcxx/deref-to-array.cpp now compiles But there are more tests getting regressed now, as I have disabled ShouldPreserveTemplateArgumentsPacks, which caused bugs for deref-to-array.cpp Failed Tests (13): Clang :: AST/ByteCode/libcxx/minmax.cpp Clang :: AST/ByteCode/libcxx/primitive-temporary.cpp Clang :: CXX/drs/cwg25xx.cpp Clang :: CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp Clang :: CXX/temp/temp.constr/temp.constr.normal/p1.cpp Clang :: CXX/temp/temp.param/p10-2a.cpp Clang :: CodeGenCXX/mangle-concept.cpp Clang :: Modules/pr62943.cppm Clang :: SemaCXX/cxx2c-fold-exprs.cpp Clang :: SemaTemplate/concepts-recursive-inst.cpp Clang :: SemaTemplate/concepts.cpp Clang :: SemaTemplate/instantiate-template-argument.cpp Clang :: SemaTemplate/temp_arg_nontype_cxx2c.cpp

Only 5 tests left now: Failed Tests (5): Clang :: CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp Clang :: Modules/pr62943.cppm Clang :: SemaCXX/cxx2c-fold-exprs.cpp Clang :: SemaTemplate/concepts-recursive-inst.cpp Clang :: SemaTemplate/instantiate-template-argument.cpp

…zation Checkpoint

shafik

I need to make another pass later.

shafik · 2025-06-16T20:15:38Z

clang/include/clang/AST/ASTConcept.h

+/// substituted constraint expr, if the template arguments could be
+/// substituted into them, or a diagnostic if substitution resulted in
+/// an invalid expression.
+using UnsatisfiedConstraintRecord =


Record is such an overloaded term, can we find a better term? I feel like UnsatisfiedConstraintUnion would almost be better here, maybe?

shafik · 2025-06-16T20:17:39Z

clang/include/clang/AST/ASTConcept.h

@@ -100,6 +102,10 @@ struct ASTConstraintSatisfaction final :
    return getTrailingObjects() + NumRecords;
  }

+  ArrayRef<UnsatisfiedConstraintRecord> records() const {


Please, can we get a more descriptive name for this member function. Especially because searching for this name will hit so many other things on a naive search.

shafik · 2025-06-16T20:23:41Z

clang/include/clang/Sema/SemaConcept.h

+  using ExprOrConcept =
+      llvm::PointerUnion<const Expr *, const ConceptReference *>;
+
+  struct AtomicBits {


Suggested change

struct AtomicBits {

struct AtomicContraintBits {

Same for other Bits structs.

shafik · 2025-06-16T20:24:45Z

clang/include/clang/Sema/SemaConcept.h

+  };
+
+  union {
+    AtomicBits Atomic;


Suggested change

AtomicBits Atomic;

AtomicConstraintBits AtomicBits;

Same for the rest of the names. Again very overloaded terms, more specific names are better and will then make searching a much better experience for future devs.

shafik · 2025-06-16T22:06:00Z

clang/include/clang/Sema/SemaConcept.h

-struct alignas(ConstraintAlignment) FoldExpandedConstraint;
+public:
+  ConstraintKind getKind() const {
+    return static_cast<ConstraintKind>(Atomic.Kind);


Maybe worth commenting this is only valid due to initial common sequence.

shafik · 2025-06-16T22:06:39Z

clang/include/clang/Sema/SemaConcept.h

+  }
+
+  bool hasParameterMapping() const {
+    assert(getKind() != ConstraintKind::Compound);


I am curious why you only assert against ConstraintLind::Compound

cor3ntin requested review from erichkeane and zyn0217 May 28, 2025 14:33

cor3ntin force-pushed the corentin/use_normalization_for_satisfaction branch 2 times, most recently from 58027d6 to f29061c Compare June 5, 2025 09:29

cor3ntin added 8 commits June 8, 2025 10:43

Fix handlind of concept-id which appear as an atomic constraint

cdf8663

Fix some tests

b9ce9a4

Fold expression fixes

99a1db5

fix nested diags and canonicalization

1cce903

fix compouund constraint checking

19c301d

fix rebase

85a61b4

cleanup

0b7a92a

cor3ntin force-pushed the corentin/use_normalization_for_satisfaction branch from f29061c to 0b7a92a Compare June 8, 2025 08:50

zyn0217 and others added 12 commits June 9, 2025 13:11

Fix the access checking after normalization

8cbad8d

Merge pull request #46 from zyn0217/fix-access-checking-after-normali…

6f462d8

…zation Fix the access checking after normalization

Fix SemaTemplate/concepts-lambda.cpp

a0d644d

Fix pack expansion

c37199b

clang/test/SemaTemplate/alias-template-with-lambdas.cpp

Merge pull request #47 from zyn0217/fix-access-checking-after-normali…

d256511

…zation Fix SemaTemplate/concepts-lambda.cpp

Fix alias-template-with-lambdas.cpp

e6c2227

Stash recent changes

4abbb11

Merge pull request #48 from zyn0217/fix-access-checking-after-normali…

96834df

…zation Stash recent changes

cleanup

4c738cb

Merge pull request #49 from zyn0217/fix-access-checking-after-normali…

ba2fdcd

…zation Checkpoint

shafik reviewed Jun 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Clang][WIP] Normalize constraints before checking for satisfaction #141776

[Clang][WIP] Normalize constraints before checking for satisfaction #141776

Uh oh!

cor3ntin commented May 28, 2025

Uh oh!

github-actions bot commented Jun 1, 2025 •

edited

Loading

Uh oh!

shafik left a comment

Uh oh!

shafik Jun 16, 2025

Uh oh!

shafik Jun 16, 2025

Uh oh!

shafik Jun 16, 2025

Uh oh!

shafik Jun 16, 2025

Uh oh!

shafik Jun 16, 2025

Uh oh!

shafik Jun 16, 2025

Uh oh!

Uh oh!

[Clang][WIP] Normalize constraints before checking for satisfaction #141776

Are you sure you want to change the base?

[Clang][WIP] Normalize constraints before checking for satisfaction #141776

Uh oh!

Conversation

cor3ntin commented May 28, 2025

Uh oh!

github-actions bot commented Jun 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shafik left a comment

Choose a reason for hiding this comment

Uh oh!

shafik Jun 16, 2025

Choose a reason for hiding this comment

Uh oh!

shafik Jun 16, 2025

Choose a reason for hiding this comment

Uh oh!

shafik Jun 16, 2025

Choose a reason for hiding this comment

Uh oh!

shafik Jun 16, 2025

Choose a reason for hiding this comment

Uh oh!

shafik Jun 16, 2025

Choose a reason for hiding this comment

Uh oh!

shafik Jun 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Jun 1, 2025 •

edited

Loading