[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 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 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 still ~3 failing test files

Fixes #135190
Fixes #61811

Co-authored-by: Younan Zhang zyn7109@gmail.com

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/ASTReaderStmt.cpp 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 a03620b36..12d75413e 100644
--- a/clang/include/clang/Sema/SemaConcept.h
+++ b/clang/include/clang/Sema/SemaConcept.h
@@ -358,12 +358,10 @@ class ConceptIdConstraint : public NormalizedConstraintWithParamMapping {
       NormalizedConstraintWithParamMapping;
 
 public:
-  static ConceptIdConstraint *Create(ASTContext &Ctx,
-                                     const ConceptReference *ConceptId,
-                                     NormalizedConstraint *SubConstraint,
-                                     const NamedDecl *ConstraintDecl,
-                                     const ConceptSpecializationExpr *CSE,
-                                     UnsignedOrNone PackIndex) {
+  static ConceptIdConstraint *
+  Create(ASTContext &Ctx, const ConceptReference *ConceptId,
+         NormalizedConstraint *SubConstraint, const NamedDecl *ConstraintDecl,
+         const ConceptSpecializationExpr *CSE, UnsignedOrNone PackIndex) {
     return new (Ctx) ConceptIdConstraint(ConceptId, ConstraintDecl,
                                          SubConstraint, CSE, PackIndex);
   }
diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 61e028b21..1ee9a49fe 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -1595,8 +1595,8 @@ substituteParameterMappings(Sema &S, NormalizedConstraintWithParamMapping &N,
       SourceLocation Loc = ArgsAsWritten->NumTemplateArgs > I
                                ? ArgsAsWritten->arguments()[I].getLocation()
                                : SourceLocation();
-      // FIXME: Investigate when we couldn't preserve the SourceLoc. What shall we do??
-      // assert(Loc.isValid());
+      // FIXME: Investigate when we couldn't preserve the SourceLoc. What shall
+      // we do?? assert(Loc.isValid());
       if (OccurringIndices[I]) {
         NamedDecl *Param = TemplateParams->begin()[I];
         new (&(TempArgs)[J])
@@ -1714,10 +1714,10 @@ substituteParameterMappings(Sema &S, ConceptIdConstraint &N,
       return true;
     Sema::CheckTemplateArgumentInfo CTAI;
     if (S.CheckTemplateArgumentList(CSE->getNamedConcept(),
-                                CSE->getConceptNameInfo().getLoc(), Out,
-                                /*DefaultArgs=*/{},
-                                /*PartialTemplateArgs=*/false, CTAI,
-                                /*UpdateArgsWithConversions=*/false))
+                                    CSE->getConceptNameInfo().getLoc(), Out,
+                                    /*DefaultArgs=*/{},
+                                    /*PartialTemplateArgs=*/false, CTAI,
+                                    /*UpdateArgsWithConversions=*/false))
       return true;
     TemplateArgs.replaceOutermostTemplateArguments(
         TemplateArgs.getAssociatedDecl(0).first, CTAI.CanonicalConverted);
@@ -1882,8 +1882,9 @@ NormalizedConstraint *NormalizedConstraint::fromConstraintExpr(
     // if (substituteParameterMappings(S, *SubNF, CSE))
     //   return nullptr;
 
-    return ConceptIdConstraint::Create(
-        S.getASTContext(), CSE->getConceptReference(), SubNF, D, CSE, SubstIndex);
+    return ConceptIdConstraint::Create(S.getASTContext(),
+                                       CSE->getConceptReference(), SubNF, D,
+                                       CSE, SubstIndex);
 
   } else if (auto *FE = dyn_cast<const CXXFoldExpr>(E);
              FE && S.getLangOpts().CPlusPlus26 &&

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

Inspired by llvm@72ac907

…zation Fix Modules/pr62943.cppm

the expressions don't subsume because they have different parameter mappings

cor3ntin · 2025-06-26T10:05:07Z

There are still a few test failures, including in libc++, but I think we are are at the stage where this could benefit from more eyes

This helps reduce the time cost of time_zone.cpp from 44s -> 14s It still needs improvements, as previous clang only takes 3s.

There are 6 tests failing now! Failed Tests (6): Clang :: AST/ByteCode/libcxx/deref-to-array.cpp Clang :: AST/ByteCode/libcxx/primitive-temporary.cpp Clang :: CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp Clang :: CXX/temp/temp.constr/temp.constr.normal/p1.cpp Clang :: Modules/GH60336.cpp Clang :: SemaTemplate/concepts-recursive-inst.cpp

Failed Tests (3): Clang :: CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp Clang :: CXX/temp/temp.constr/temp.constr.normal/p1.cpp Clang :: SemaTemplate/concepts-recursive-inst.cpp

Fix the parameter mapping complexity

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

zyn0217 and others added 3 commits June 17, 2025 19:27

Fix Modules/pr62943.cppm

ff30282

Inspired by llvm@72ac907

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

ab53f2d

…zation Fix Modules/pr62943.cppm

Fix a test case

a4f9264

the expressions don't subsume because they have different parameter mappings

cor3ntin added 3 commits June 25, 2025 15:51

add test for llvm#135190

4fe9a16

cleanup fold expression tests

d3d9ff3

Format

1650151

cor3ntin marked this pull request as ready for review June 26, 2025 10:04

cor3ntin requested a review from Endilll as a code owner June 26, 2025 10:04

zyn0217 and others added 4 commits July 1, 2025 19:02

Add parameter mapping cache

e3dd06f

This helps reduce the time cost of time_zone.cpp from 44s -> 14s It still needs improvements, as previous clang only takes 3s.

Save the checkpoint

3b737ae

Failed Tests (3): Clang :: CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp Clang :: CXX/temp/temp.constr/temp.constr.normal/p1.cpp Clang :: SemaTemplate/concepts-recursive-inst.cpp

Merge pull request #51 from zyn0217/fix-concepts-cache

38b63c0

Fix the parameter mapping complexity

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

cor3ntin commented May 28, 2025 •

edited

Loading

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!

cor3ntin commented Jun 26, 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

Conversation

cor3ntin commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

cor3ntin commented Jun 26, 2025

Uh oh!

Uh oh!

cor3ntin commented May 28, 2025 •

edited

Loading

github-actions bot commented Jun 1, 2025 •

edited

Loading