[Clang] Normalize constraints before checking for satisfaction #141776
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
cor3ntin
force-pushed
the
corentin/use_normalization_for_satisfaction
branch
3 times, most recently
from
f29061c to
0b7a92a
Compare
shafik
reviewed
Jun 16, 2025
| /// substituted constraint expr, if the template arguments could be | ||
| /// substituted into them, or a diagnostic if substitution resulted in | ||
| /// an invalid expression. | ||
| using UnsatisfiedConstraintRecord = |
There was a problem hiding this comment.
Record is such an overloaded term, can we find a better term? I feel like UnsatisfiedConstraintUnion would almost be better here, maybe?
There was a problem hiding this comment.
The name is pre-existing, we could change it afterwards
| return getTrailingObjects() + NumRecords; | ||
| } | ||
|
|
||
| ArrayRef<UnsatisfiedConstraintRecord> records() const { |
There was a problem hiding this comment.
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.
| struct alignas(ConstraintAlignment) FoldExpandedConstraint; | ||
| public: | ||
| ConstraintKind getKind() const { | ||
| return static_cast<ConstraintKind>(Atomic.Kind); |
There was a problem hiding this comment.
Maybe worth commenting this is only valid due to initial common sequence.
|
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 was referenced Jul 22, 2025
cor3ntin
force-pushed
the
corentin/use_normalization_for_satisfaction
branch
from
2da1c7f to
a08930e
Compare
cor3ntin
force-pushed
the
corentin/use_normalization_for_satisfaction
branch
from
141cca5 to
e04b68f
Compare
llvmbot
added
clang
|
@llvm/pr-subscribers-libcxx @llvm/pr-subscribers-hlsl Author: Corentin Jabot (cor3ntin) ChangesIn 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 Co-authored-by: Younan Zhang <[email protected]> Patch is 258.38 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/141776.diff 49 Files Affected:
diff --git a/clang/include/clang/AST/ASTConcept.h b/clang/include/clang/AST/ASTConcept.h
index 72da0059744f2..9babfd39dd668 100644
--- a/clang/include/clang/AST/ASTConcept.h
+++ b/clang/include/clang/AST/ASTConcept.h
@@ -28,13 +28,23 @@ namespace clang {
class ConceptDecl;
class TemplateDecl;
+class ConceptReference;
class Expr;
class NamedDecl;
struct PrintingPolicy;
+/// 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<const Expr *, const ConceptReference *,
+ std::pair<SourceLocation, StringRef> *>;
+
/// The result of a constraint satisfaction check, containing the necessary
/// information to diagnose an unsatisfied constraint.
class ConstraintSatisfaction : public llvm::FoldingSetNode {
+private:
// The template-like entity that 'owns' the constraint checked here (can be a
// constrained entity or a concept).
const NamedDecl *ConstraintOwner = nullptr;
@@ -49,7 +59,6 @@ class ConstraintSatisfaction : public llvm::FoldingSetNode {
: ConstraintOwner(ConstraintOwner), TemplateArgs(TemplateArgs) {}
using SubstitutionDiagnostic = std::pair<SourceLocation, StringRef>;
- using Detail = llvm::PointerUnion<Expr *, SubstitutionDiagnostic *>;
bool IsSatisfied = false;
bool ContainsErrors = false;
@@ -57,7 +66,7 @@ class ConstraintSatisfaction : public llvm::FoldingSetNode {
/// \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);
@@ -75,13 +84,6 @@ class ConstraintSatisfaction : public llvm::FoldingSetNode {
}
};
-/// 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 +103,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 +288,11 @@ class TypeConstraint {
}
};
+/// Insertion operator for diagnostics. This allows sending TemplateName's
+/// into a diagnostic with <<.
+const StreamingDiagnostic &operator<<(const StreamingDiagnostic &DB,
+ const ConceptReference *C);
+
} // clang
#endif // LLVM_CLANG_AST_ASTCONCEPT_H
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 1c17333b722f8..0f808cbe2c17c 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -51,6 +51,27 @@ class FixedPointSemantics;
struct fltSemantics;
template <typename T, unsigned N> class SmallPtrSet;
+template <> struct DenseMapInfo<llvm::FoldingSetNodeID> {
+ static FoldingSetNodeID getEmptyKey() { return FoldingSetNodeID{}; }
+
+ static FoldingSetNodeID getTombstoneKey() {
+ FoldingSetNodeID id;
+ for (size_t i = 0; i < sizeof(id) / sizeof(unsigned); ++i) {
+ id.AddInteger(std::numeric_limits<unsigned>::max());
+ }
+ return id;
+ }
+
+ static unsigned getHashValue(const FoldingSetNodeID &Val) {
+ return Val.ComputeHash();
+ }
+
+ static bool isEqual(const FoldingSetNodeID &LHS,
+ const FoldingSetNodeID &RHS) {
+ return LHS == RHS;
+ }
+};
+
} // namespace llvm
namespace clang {
@@ -3852,4 +3873,26 @@ typename clang::LazyGenerationalUpdatePtr<Owner, T, Update>::ValueType
return Value;
}
+template <> struct llvm::DenseMapInfo<llvm::FoldingSetNodeID> {
+ static FoldingSetNodeID getEmptyKey() { return FoldingSetNodeID{}; }
+
+ static FoldingSetNodeID getTombstoneKey() {
+ FoldingSetNodeID id;
+ for (size_t i = 0; i < sizeof(id) / sizeof(unsigned); ++i) {
+ id.AddInteger(std::numeric_limits<unsigned>::max());
+ }
+ return id;
+ }
+
+ static unsigned getHashValue(const FoldingSetNodeID &Val) {
+ return Val.ComputeHash();
+ }
+
+ static bool isEqual(const FoldingSetNodeID &LHS,
+ const FoldingSetNodeID &RHS) {
+ return LHS == RHS;
+ }
+};
+
+
#endif // LLVM_CLANG_AST_ASTCONTEXT_H
diff --git a/clang/include/clang/AST/TemplateBase.h b/clang/include/clang/AST/TemplateBase.h
index de248ac3cf703..0359f4beb6309 100644
--- a/clang/include/clang/AST/TemplateBase.h
+++ b/clang/include/clang/AST/TemplateBase.h
@@ -475,7 +475,6 @@ class TemplateArgument {
/// Used to insert TemplateArguments into FoldingSets.
void Profile(llvm::FoldingSetNodeID &ID, const ASTContext &Context) const;
};
-
/// Location information for a TemplateArgument.
struct TemplateArgumentLocInfo {
struct TemplateTemplateArgLocInfo {
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 88b67eed5fd37..a508d33018292 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/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"
@@ -11688,8 +11689,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,
@@ -12002,6 +12004,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,
@@ -12776,6 +12785,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) {
@@ -13073,6 +13085,8 @@ class Sema final : public SemaBase {
/// Whether we're substituting into constraints.
bool InConstraintSubstitution;
+ bool InParameterMappingSubstitution;
+
/// The point of instantiation or synthesis within the source code.
SourceLocation PointOfInstantiation;
@@ -13338,6 +13352,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.
///
@@ -13799,6 +13818,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
@@ -14683,6 +14708,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
@@ -14707,44 +14736,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
@@ -14810,16 +14807,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
@@ -14844,6 +14842,9 @@ class Sema final : public SemaBase {
const NamedDecl *D1, ArrayRef<AssociatedConstraint> AC1,
const NamedDecl *D2, ArrayRef<AssociatedConstraint> AC2);
+ llvm::DenseMap<llvm::FoldingSetNodeID, CachedConceptIdConstraint>
+ ConceptIdSatisfactionCache;
+
private:
/// Caches pairs of template-like decls whose associated constraints were
/// checked for subsumption and whether or not the first's constraints did in
@@ -14854,7 +14855,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<const NamedDecl *, NormalizedConstraint *> NormalizationCache;
+ llvm::DenseMap<ConstrainedDeclOrNestedRequirement, NormalizedConstraint *>
+ NormalizationCache;
llvm::ContextualFoldingSet<ConstraintSatisfaction, const ASTContext &>
SatisfactionCache;
diff --git a/clang/include/clang/Sema/SemaConcept.h b/clang/include/clang/Sema/SemaConcept.h
index 648a9c51ae6c1..3ba48b75c1286 100644
--- a/clang/include/clang/Sema/SemaConcept.h
+++ b/clang/include/clang/Sema/SemaConcept.h
@@ -16,130 +16,387 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/DeclTemplate.h"
#include "clang/AST/Expr.h"
+#include "clang/AST/ExprConcepts.h"
#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/UnsignedOrNone.h"
+#include "clang/Sema/Ownership.h"
+#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/FoldingSet.h"
-#include "llvm/ADT/PointerUnion.h"
+#include "llvm/ADT/STLForwardCompat.h"
#include "llvm/ADT/STLFunctionalExtras.h"
+#include "llvm/ADT/SmallBitVector.h"
#include "llvm/ADT/SmallVector.h"
+#include "llvm/Support/Compiler.h"
#include <optional>
#include <utility>
namespace clang {
class Sema;
+class MultiLevelTemplateArgumentList;
-enum { ConstraintAlignment = 8 };
+/// \brief A normalized constraint, as defined in C++ [temp.constr.normal], is
+/// either an atomic constraint, a conjunction of normalized constraints or a
+/// disjunction of normalized constraints.
+
+struct NormalizedConstraint {
+
+ enum class ConstraintKind {
+ Atomic = 0,
+ ConceptId,
+ FoldExpanded,
+ Compound,
+ };
+
+ enum CompoundConstraintKind { CCK_Conjunction, CCK_Disjunction };
+ enum class FoldOperatorKind : unsigned int { And, Or };
+
+ using OccurenceList = llvm::SmallBitVector;
+
+ friend bool
+ substituteParameterMappings(Sema &S, NormalizedConstraint &N,
+ const MultiLevelTemplateArgumentList &MLTAL,
+ const ASTTemplateArgumentListInfo *ArgsAsWritten,
+ TemplateParameterList *TemplateParams,
+ const NamedDecl *D);
+
+protected:
+ using ExprOrConcept =
+ llvm::PointerUnion<const Expr *, const ConceptReference *>;
+
+ struct AtomicBits {
+ LLVM_PREFERRED_TYPE(ConstraintKind)
+ unsigned Kind : 5;
+ unsigned Placeholder : 1;
+ unsigned PackSubstitutionIndex : 26;
+ OccurenceList Indexes;
+ TemplateArgumentLoc *Args;
+ TemplateParameterList *ParamList;
+ ExprOrConcept ConstraintExpr;
+ const NamedDecl *ConstraintDecl;
+ };
+
+ struct FoldExpandedBits {
+ LLVM_PREFERRED_TYPE(ConstraintKind)
+ unsigned Kind : 5;
+ LLVM_PREFERRED_TYPE(FoldOperatorKind)
+ unsigned FoldOperator : 1;
+ unsigned Placeholder : 26;
+ OccurenceList Indexes;
+ TemplateArgumentLoc *Args;
+ TemplateParameterList *ParamList;
+ const Expr *Pattern;
+ const NamedDecl *ConstraintDecl;
+ NormalizedConstraint *Constraint;
+ };
+
+ struct ConceptIdBits : AtomicBits {
+ NormalizedConstraint *Sub;
+
+ // Only used for parameter mapping.
+ const ConceptSpecializationExpr *CSE;
+ };
+
+ struct CompoundBits {
+ LLVM_PREFERRED_TYPE(ConstraintKind)
+ unsigned Kind : 5;
+ LLVM_PREFERRED_TYPE(CompoundConstraintKind)
+ unsigned CCK : 1;
+ NormalizedConstraint *LHS;
+ NormalizedConstraint *RHS;
+ };
+
+ union {
+ AtomicBits Atomic;
+ FoldExpandedBits FoldExpanded;
+ ConceptIdBits ConceptId;
+ CompoundBits Compound;
+ };
+
+ ~NormalizedConstraint() {
+ if (getKind() != ConstraintKind::Compound)
+ Atomic.Indexes.llvm::SmallBitVector::~SmallBitVector();
+ }
+
+ NormalizedConstraint(const Expr *ConstraintExpr,
+ const NamedDecl *ConstraintDecl,
+ UnsignedOrNone PackIndex)
+ : Atomic{llvm::to_underlying(ConstraintKind::Atomic),
+ /*Placeholder=*/0,
+ PackIndex.toInternalRepresentation(),
+ /*Indexes=*/{},
+ /*Args=*/nullptr,
+ /*ParamList=*/nullptr,
+ ConstraintExpr,
+ ConstraintDecl} {}
+
+ NormalizedConstraint(const Expr *Pattern, FoldOperatorKind OpKind,
+ NormalizedConstraint *Constraint,
+ const NamedDecl *ConstraintDecl)
+ : FoldExpanded{llvm::to_underlying(ConstraintKind::FoldExpanded),
+ llvm::to_underlying(OpKind),
+ /*Placeholder=*/0,
+ /*Indexes=*/{},
+ /*Args=*/nullptr,
+ /*ParamList=*/nullptr,
+ Pattern,
+ ConstraintDecl,
+ Constraint} {}
+
+ NormalizedConstraint(const ConceptReference *ConceptId,
+ const NamedDecl *ConstraintDecl,
+ NormalizedConstraint *SubConstraint,
+ const ConceptSpecializationExpr *CSE,
+ UnsignedOrNone PackIndex)
+ : ConceptId{{llvm::to_underlying(ConstraintKind::ConceptId),
+ /*Placeholder=*/0, PackIndex.toInternalRepresentation(),
+ /*Indexes=*/{},
+ /*Args=*/nullptr, /*ParamList=*/nullptr, ConceptId,
+ ConstraintDecl},
+ SubConstraint,
+ CSE} {}
+
+ NormalizedConstraint(NormalizedConstraint *LHS, CompoundConstraintKind CCK,
+ NormalizedConstraint *RHS)
+ : Compound{llvm::to_underlying(ConstraintKind::Compound), CCK, LHS, RHS} {
+ }
-struct alignas(ConstraintAlignment) AtomicConstraint {
- const Expr *ConstraintExpr;
- const NamedDecl *ConstraintDecl;
- std::optional<ArrayRef<TemplateArgumentLoc>> ParameterMapping;
+ bool hasParameterMapping() const {
+ return getKind() != ConstraintKind::Compound
+ && Atomic.Args != nullptr;
+ }
+
+ const OccurenceList &mappingOccurenceList() const {
+ assert(hasParameterMapping() && "Th...
[truncated]
|
llvmbot
added
the
libc++
cor3ntin
force-pushed
the
corentin/use_normalization_for_satisfaction
branch
from
c4e0ddb to
6f514b2
Compare
zyn0217
reviewed
Sep 9, 2025
There was a problem hiding this comment.
Thank you very much for putting it up and the incredible work!
I left some notes, mostly are those that I didn't get around to yet (and I think they should be useful); so maybe we want to resolve them before we move forward.
Also let's not forget the release note :)
clang/include/clang/AST/ASTContext.h
Outdated
| struct fltSemantics; | ||
| template <typename T, unsigned N> class SmallPtrSet; | ||
|
|
||
| template <> struct DenseMapInfo<llvm::FoldingSetNodeID> { |
There was a problem hiding this comment.
Maybe split it into an NFC PR? I'm not sure if downstream users want it
There was a problem hiding this comment.
This patch is in desperate need of 'shrinking', I like the idea of splitting out any/everything we can.
zyn0217
reviewed
Sep 9, 2025
zyn0217
reviewed
Sep 9, 2025
|
Update: still needs cleaning but the test pass |
zyn0217
reviewed
Sep 10, 2025
These parameters should not affect subsumption. This force us to maintain two occurence list for each atomic constraint.
zyn0217
reviewed
Sep 24, 2025
| const Expr *E, unsigned Depth, llvm::SmallBitVector &Used) { | ||
| MarkUsedTemplateParameterVisitor(Used, Depth, /*VisitDeclRefTypes=*/false) | ||
| .TraverseStmt(const_cast<Expr *>(E)); | ||
| return; |
zyn0217
reviewed
Sep 24, 2025
Comment on lines
+1929
to
+1933
|
|
||
| SemaRef.MarkUsedTemplateParametersForSubsumptionParameterMapping( | ||
| static_cast<AtomicConstraint &>(N).getConstraintExpr(), | ||
| /*Depth=*/0, OccurringIndicesForSubsumption); | ||
|
|
There was a problem hiding this comment.
Do you think this is applicable to FoldExprs?
There was a problem hiding this comment.
I did some testing and i can't get it to failed without it, so I removed it!
…n/use_normalization_for_satisfaction
…n/use_normalization_for_satisfaction
…n/use_normalization_for_satisfaction
|
There have not been any comments in a while - let's see if this sticks :) |
|
Looks like it has a memory leak: https://lab.llvm.org/buildbot/#/builders/169/builds/15562. @cor3ntin please take a look. |
Thanks. I reverted the patch shortly after landing it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 [email protected]