[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
Alejandro Álvarez Ayllón via cfe-commits
cfe-commits at lists.llvm.org
Wed Feb 12 04:04:54 PST 2025
https://github.com/alejandro-alvarez-sonarsource updated https://github.com/llvm/llvm-project/pull/121768
>From da2bbf99b8430d8b6aa6bf7969c9825b4d94219b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alvarez at sonarsource.com>
Date: Mon, 18 Nov 2024 11:36:03 +0100
Subject: [PATCH 1/8] [clang][Sema] Fix initialization of
`NonTypeTemplateParmDecl`...
...when there are invalid constraints.
When attaching a `TypeConstraint`, in case of error, the trailing
pointer that is supposed to point to the constraint is left
uninitialized.
Sometimes the uninitialized value will be a `nullptr`, but at other
times it will not. If we traverse the AST (for instance, dumping it,
or when writing the BMI), we may get a crash depending on the value
that was left. The serialization may also contain a bogus value.
With this commit, we always initialize this trailing pointer, using a
`RecoveryExpr` in case of failure to parse.
---
clang/lib/Sema/SemaTemplate.cpp | 18 ++++++-
...constraint-template-non-type-parm-decl.cpp | 54 +++++++++++++++++++
2 files changed, 70 insertions(+), 2 deletions(-)
create mode 100644 clang/test/Modules/malformed-constraint-template-non-type-parm-decl.cpp
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 20ec2fbeaa6a8..9b51f973fb2bb 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -1235,6 +1235,10 @@ bool Sema::AttachTypeConstraint(AutoTypeLoc TL,
<< NewConstrainedParm->getTypeSourceInfo()
->getTypeLoc()
.getSourceRange();
+ NewConstrainedParm->setPlaceholderTypeConstraint(
+ RecoveryExpr::Create(Context, OrigConstrainedParm->getType(),
+ OrigConstrainedParm->getBeginLoc(),
+ OrigConstrainedParm->getEndLoc(), {}));
return true;
}
// FIXME: Concepts: This should be the type of the placeholder, but this is
@@ -1242,8 +1246,13 @@ bool Sema::AttachTypeConstraint(AutoTypeLoc TL,
DeclRefExpr *Ref =
BuildDeclRefExpr(OrigConstrainedParm, OrigConstrainedParm->getType(),
VK_PRValue, OrigConstrainedParm->getLocation());
- if (!Ref)
+ if (!Ref) {
+ NewConstrainedParm->setPlaceholderTypeConstraint(
+ RecoveryExpr::Create(Context, OrigConstrainedParm->getType(),
+ OrigConstrainedParm->getBeginLoc(),
+ OrigConstrainedParm->getEndLoc(), {}));
return true;
+ }
ExprResult ImmediatelyDeclaredConstraint = formImmediatelyDeclaredConstraint(
*this, TL.getNestedNameSpecifierLoc(), TL.getConceptNameInfo(),
TL.getNamedConcept(), /*FoundDecl=*/TL.getFoundDecl(), TL.getLAngleLoc(),
@@ -1255,8 +1264,13 @@ bool Sema::AttachTypeConstraint(AutoTypeLoc TL,
},
EllipsisLoc);
if (ImmediatelyDeclaredConstraint.isInvalid() ||
- !ImmediatelyDeclaredConstraint.isUsable())
+ !ImmediatelyDeclaredConstraint.isUsable()) {
+ NewConstrainedParm->setPlaceholderTypeConstraint(
+ RecoveryExpr::Create(Context, OrigConstrainedParm->getType(),
+ OrigConstrainedParm->getBeginLoc(),
+ OrigConstrainedParm->getEndLoc(), {}));
return true;
+ }
NewConstrainedParm->setPlaceholderTypeConstraint(
ImmediatelyDeclaredConstraint.get());
diff --git a/clang/test/Modules/malformed-constraint-template-non-type-parm-decl.cpp b/clang/test/Modules/malformed-constraint-template-non-type-parm-decl.cpp
new file mode 100644
index 0000000000000..cea6404bbebd2
--- /dev/null
+++ b/clang/test/Modules/malformed-constraint-template-non-type-parm-decl.cpp
@@ -0,0 +1,54 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 mod.cppm -emit-module-interface -o mod.pcm -fallow-pcm-with-compiler-errors -verify
+// RUN: %clang_cc1 -std=c++20 main.cpp -fmodule-file=mod=mod.pcm -verify -fallow-pcm-with-compiler-errors -fsyntax-only -ast-dump-all | FileCheck %s
+
+//--- mod.cppm
+export module mod;
+
+template <typename T, auto Q>
+concept ReferenceOf = Q;
+
+// expected-error at +2 {{unknown type name 'AngleIsInvalidNow'}}
+// expected-error at +1 {{constexpr variable 'angle' must be initialized by a constant expression}}
+constexpr struct angle {AngleIsInvalidNow e;} angle;
+
+// expected-error at +1 {{non-type template argument is not a constant expression}}
+template<ReferenceOf<angle> auto R, typename Rep> requires requires(Rep v) {cos(v);}
+auto cos(const Rep& q);
+
+// expected-error at +1 {{non-type template argument is not a constant expression}}
+template<ReferenceOf<angle> auto R, typename Rep> requires requires(Rep v) {tan(v);}
+auto tan(const Rep& q);
+
+//--- main.cpp
+// expected-no-diagnostics
+import mod;
+
+// CHECK: |-FunctionTemplateDecl {{.*}} <line:11:1, line:12:22> col:6 imported in mod hidden invalid cos
+// CHECK-NEXT: | |-NonTypeTemplateParmDecl {{.*}} <line:11:10, col:34> col:34 imported in mod hidden referenced invalid 'ReferenceOf<angle> auto' depth 0 index 0 R
+// CHECK-NEXT: | | `-RecoveryExpr {{.*}} <col:10, col:34> 'ReferenceOf<angle> auto' contains-errors lvalue
+// CHECK-NEXT: | |-TemplateTypeParmDecl {{.*}} <col:37, col:46> col:46 imported in mod hidden referenced typename depth 0 index 1 Rep
+// CHECK-NEXT: | |-RequiresExpr {{.*}} <col:60, col:84> 'bool'
+// CHECK-NEXT: | | |-ParmVarDecl {{.*}} <col:69, col:73> col:73 imported in mod hidden referenced v 'Rep'
+// CHECK-NEXT: | | `-SimpleRequirement {{.*}} dependent
+// CHECK-NEXT: | | `-CallExpr {{.*}} <col:77, col:82> '<dependent type>'
+// CHECK-NEXT: | | |-UnresolvedLookupExpr {{.*}} <col:77> '<overloaded function type>' lvalue (ADL) = 'cos' empty
+// CHECK-NEXT: | | `-DeclRefExpr {{.*}} <col:81> 'Rep' lvalue ParmVar {{.*}} 'v' 'Rep' non_odr_use_unevaluated
+// CHECK-NEXT: | `-FunctionDecl {{.*}} <line:12:1, col:22> col:6 imported in mod hidden cos 'auto (const Rep &)'
+// CHECK-NEXT: | `-ParmVarDecl {{.*}} <col:10, col:21> col:21 imported in mod hidden q 'const Rep &'
+
+// CHECK: |-FunctionTemplateDecl {{.*}} <line:15:1, line:16:22> col:6 imported in mod hidden invalid tan
+// CHECK-NEXT: | |-NonTypeTemplateParmDecl {{.*}} <line:15:10, col:34> col:34 imported in mod hidden referenced invalid 'ReferenceOf<angle> auto' depth 0 index 0 R
+// CHECK-NEXT: | | `-RecoveryExpr {{.*}} <col:10, col:34> 'ReferenceOf<angle> auto' contains-errors lvalue
+// CHECK-NEXT: | |-TemplateTypeParmDecl {{.*}} <col:37, col:46> col:46 imported in mod hidden referenced typename depth 0 index 1 Rep
+// CHECK-NEXT: | |-RequiresExpr {{.*}} <col:60, col:84> 'bool'
+// CHECK-NEXT: | | |-ParmVarDecl {{.*}} <col:69, col:73> col:73 imported in mod hidden referenced v 'Rep'
+// CHECK-NEXT: | | `-SimpleRequirement {{.*}} dependent
+// CHECK-NEXT: | | `-CallExpr {{.*}} <col:77, col:82> '<dependent type>'
+// CHECK-NEXT: | | |-UnresolvedLookupExpr {{.*}} <col:77> '<overloaded function type>' lvalue (ADL) = 'tan' empty
+// CHECK-NEXT: | | `-DeclRefExpr {{.*}} <col:81> 'Rep' lvalue ParmVar {{.*}} 'v' 'Rep' non_odr_use_unevaluated
+// CHECK-NEXT: | `-FunctionDecl {{.*}} <line:16:1, col:22> col:6 imported in mod hidden tan 'auto (const Rep &)'
+// CHECK-NEXT: | `-ParmVarDecl {{.*}} <col:10, col:21> col:21 imported in mod hidden q 'const Rep &'
>From fc140834694f45faf9ba72653fe76f05974dd89f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alvarez at sonarsource.com>
Date: Mon, 6 Jan 2025 16:18:42 +0100
Subject: [PATCH 2/8] `BuildDeclRefExpr` should not fail
---
clang/lib/Sema/SemaTemplate.cpp | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 9b51f973fb2bb..a6e701141cb09 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -1246,13 +1246,8 @@ bool Sema::AttachTypeConstraint(AutoTypeLoc TL,
DeclRefExpr *Ref =
BuildDeclRefExpr(OrigConstrainedParm, OrigConstrainedParm->getType(),
VK_PRValue, OrigConstrainedParm->getLocation());
- if (!Ref) {
- NewConstrainedParm->setPlaceholderTypeConstraint(
- RecoveryExpr::Create(Context, OrigConstrainedParm->getType(),
- OrigConstrainedParm->getBeginLoc(),
- OrigConstrainedParm->getEndLoc(), {}));
- return true;
- }
+ assert(Ref != nullptr && "Unexpected nullptr!");
+
ExprResult ImmediatelyDeclaredConstraint = formImmediatelyDeclaredConstraint(
*this, TL.getNestedNameSpecifierLoc(), TL.getConceptNameInfo(),
TL.getNamedConcept(), /*FoundDecl=*/TL.getFoundDecl(), TL.getLAngleLoc(),
>From ab1616f4238852d5efa44b4aab67e72048af9738 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alvarez at sonarsource.com>
Date: Mon, 6 Jan 2025 17:02:54 +0100
Subject: [PATCH 3/8] Refactor to avoid duplication
---
clang/lib/Sema/SemaTemplate.cpp | 59 +++++++++++++++++----------------
1 file changed, 30 insertions(+), 29 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index a6e701141cb09..336a71b151b57 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -1228,36 +1228,37 @@ bool Sema::AttachTypeConstraint(AutoTypeLoc TL,
NonTypeTemplateParmDecl *NewConstrainedParm,
NonTypeTemplateParmDecl *OrigConstrainedParm,
SourceLocation EllipsisLoc) {
- if (NewConstrainedParm->getType().getNonPackExpansionType() != TL.getType() ||
- TL.getAutoKeyword() != AutoTypeKeyword::Auto) {
- Diag(NewConstrainedParm->getTypeSourceInfo()->getTypeLoc().getBeginLoc(),
- diag::err_unsupported_placeholder_constraint)
- << NewConstrainedParm->getTypeSourceInfo()
- ->getTypeLoc()
- .getSourceRange();
- NewConstrainedParm->setPlaceholderTypeConstraint(
- RecoveryExpr::Create(Context, OrigConstrainedParm->getType(),
- OrigConstrainedParm->getBeginLoc(),
- OrigConstrainedParm->getEndLoc(), {}));
- return true;
- }
- // FIXME: Concepts: This should be the type of the placeholder, but this is
- // unclear in the wording right now.
- DeclRefExpr *Ref =
- BuildDeclRefExpr(OrigConstrainedParm, OrigConstrainedParm->getType(),
- VK_PRValue, OrigConstrainedParm->getLocation());
- assert(Ref != nullptr && "Unexpected nullptr!");
+ ExprResult ImmediatelyDeclaredConstraint = [&] {
+ if (NewConstrainedParm->getType().getNonPackExpansionType() !=
+ TL.getType() ||
+ TL.getAutoKeyword() != AutoTypeKeyword::Auto) {
+ Diag(NewConstrainedParm->getTypeSourceInfo()->getTypeLoc().getBeginLoc(),
+ diag::err_unsupported_placeholder_constraint)
+ << NewConstrainedParm->getTypeSourceInfo()
+ ->getTypeLoc()
+ .getSourceRange();
+ return ExprResult();
+ }
+
+ // FIXME: Concepts: This should be the type of the placeholder, but this is
+ // unclear in the wording right now.
+ DeclRefExpr *Ref =
+ BuildDeclRefExpr(OrigConstrainedParm, OrigConstrainedParm->getType(),
+ VK_PRValue, OrigConstrainedParm->getLocation());
+ assert(Ref != nullptr && "Unexpected nullptr!");
+
+ return formImmediatelyDeclaredConstraint(
+ *this, TL.getNestedNameSpecifierLoc(), TL.getConceptNameInfo(),
+ TL.getNamedConcept(), /*FoundDecl=*/TL.getFoundDecl(),
+ TL.getLAngleLoc(), TL.getRAngleLoc(), BuildDecltypeType(Ref),
+ OrigConstrainedParm->getLocation(),
+ [&](TemplateArgumentListInfo &ConstraintArgs) {
+ for (unsigned I = 0, C = TL.getNumArgs(); I != C; ++I)
+ ConstraintArgs.addArgument(TL.getArgLoc(I));
+ },
+ EllipsisLoc);
+ }();
- ExprResult ImmediatelyDeclaredConstraint = formImmediatelyDeclaredConstraint(
- *this, TL.getNestedNameSpecifierLoc(), TL.getConceptNameInfo(),
- TL.getNamedConcept(), /*FoundDecl=*/TL.getFoundDecl(), TL.getLAngleLoc(),
- TL.getRAngleLoc(), BuildDecltypeType(Ref),
- OrigConstrainedParm->getLocation(),
- [&](TemplateArgumentListInfo &ConstraintArgs) {
- for (unsigned I = 0, C = TL.getNumArgs(); I != C; ++I)
- ConstraintArgs.addArgument(TL.getArgLoc(I));
- },
- EllipsisLoc);
if (ImmediatelyDeclaredConstraint.isInvalid() ||
!ImmediatelyDeclaredConstraint.isUsable()) {
NewConstrainedParm->setPlaceholderTypeConstraint(
>From 12d918ff45689ed286230fa28b456fea24af713d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alvarez at sonarsource.com>
Date: Thu, 9 Jan 2025 11:12:30 +0100
Subject: [PATCH 4/8] Revert `RecoverExpr`
---
clang/lib/Sema/SemaTemplate.cpp | 64 ++++++++++++++-------------------
1 file changed, 27 insertions(+), 37 deletions(-)
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 336a71b151b57..20ec2fbeaa6a8 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -1228,45 +1228,35 @@ bool Sema::AttachTypeConstraint(AutoTypeLoc TL,
NonTypeTemplateParmDecl *NewConstrainedParm,
NonTypeTemplateParmDecl *OrigConstrainedParm,
SourceLocation EllipsisLoc) {
- ExprResult ImmediatelyDeclaredConstraint = [&] {
- if (NewConstrainedParm->getType().getNonPackExpansionType() !=
- TL.getType() ||
- TL.getAutoKeyword() != AutoTypeKeyword::Auto) {
- Diag(NewConstrainedParm->getTypeSourceInfo()->getTypeLoc().getBeginLoc(),
- diag::err_unsupported_placeholder_constraint)
- << NewConstrainedParm->getTypeSourceInfo()
- ->getTypeLoc()
- .getSourceRange();
- return ExprResult();
- }
-
- // FIXME: Concepts: This should be the type of the placeholder, but this is
- // unclear in the wording right now.
- DeclRefExpr *Ref =
- BuildDeclRefExpr(OrigConstrainedParm, OrigConstrainedParm->getType(),
- VK_PRValue, OrigConstrainedParm->getLocation());
- assert(Ref != nullptr && "Unexpected nullptr!");
-
- return formImmediatelyDeclaredConstraint(
- *this, TL.getNestedNameSpecifierLoc(), TL.getConceptNameInfo(),
- TL.getNamedConcept(), /*FoundDecl=*/TL.getFoundDecl(),
- TL.getLAngleLoc(), TL.getRAngleLoc(), BuildDecltypeType(Ref),
- OrigConstrainedParm->getLocation(),
- [&](TemplateArgumentListInfo &ConstraintArgs) {
- for (unsigned I = 0, C = TL.getNumArgs(); I != C; ++I)
- ConstraintArgs.addArgument(TL.getArgLoc(I));
- },
- EllipsisLoc);
- }();
-
- if (ImmediatelyDeclaredConstraint.isInvalid() ||
- !ImmediatelyDeclaredConstraint.isUsable()) {
- NewConstrainedParm->setPlaceholderTypeConstraint(
- RecoveryExpr::Create(Context, OrigConstrainedParm->getType(),
- OrigConstrainedParm->getBeginLoc(),
- OrigConstrainedParm->getEndLoc(), {}));
+ if (NewConstrainedParm->getType().getNonPackExpansionType() != TL.getType() ||
+ TL.getAutoKeyword() != AutoTypeKeyword::Auto) {
+ Diag(NewConstrainedParm->getTypeSourceInfo()->getTypeLoc().getBeginLoc(),
+ diag::err_unsupported_placeholder_constraint)
+ << NewConstrainedParm->getTypeSourceInfo()
+ ->getTypeLoc()
+ .getSourceRange();
return true;
}
+ // FIXME: Concepts: This should be the type of the placeholder, but this is
+ // unclear in the wording right now.
+ DeclRefExpr *Ref =
+ BuildDeclRefExpr(OrigConstrainedParm, OrigConstrainedParm->getType(),
+ VK_PRValue, OrigConstrainedParm->getLocation());
+ if (!Ref)
+ return true;
+ ExprResult ImmediatelyDeclaredConstraint = formImmediatelyDeclaredConstraint(
+ *this, TL.getNestedNameSpecifierLoc(), TL.getConceptNameInfo(),
+ TL.getNamedConcept(), /*FoundDecl=*/TL.getFoundDecl(), TL.getLAngleLoc(),
+ TL.getRAngleLoc(), BuildDecltypeType(Ref),
+ OrigConstrainedParm->getLocation(),
+ [&](TemplateArgumentListInfo &ConstraintArgs) {
+ for (unsigned I = 0, C = TL.getNumArgs(); I != C; ++I)
+ ConstraintArgs.addArgument(TL.getArgLoc(I));
+ },
+ EllipsisLoc);
+ if (ImmediatelyDeclaredConstraint.isInvalid() ||
+ !ImmediatelyDeclaredConstraint.isUsable())
+ return true;
NewConstrainedParm->setPlaceholderTypeConstraint(
ImmediatelyDeclaredConstraint.get());
>From 792fb35e538abfde1f50257b55dc02059a8644c9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alvarez at sonarsource.com>
Date: Thu, 9 Jan 2025 11:48:09 +0100
Subject: [PATCH 5/8] Test only reduced BMI
---
clang/include/clang/AST/DeclTemplate.h | 10 +++++-----
clang/lib/AST/DeclTemplate.cpp | 10 ++++++++++
clang/lib/Serialization/ASTReaderDecl.cpp | 2 +-
clang/lib/Serialization/ASTWriterDecl.cpp | 6 ++++--
...alformed-constraint-template-non-type-parm-decl.cpp | 5 +++--
5 files changed, 23 insertions(+), 10 deletions(-)
diff --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h
index d3a466a8617bb..2778d362fd03c 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -1366,6 +1366,8 @@ class NonTypeTemplateParmDecl final
DefaultArgStorage<NonTypeTemplateParmDecl, TemplateArgumentLoc *>;
DefArgStorage DefaultArgument;
+ bool PlaceholderTypeConstraintInitialized = false;
+
// FIXME: Collapse this into TemplateParamPosition; or, just move depth/index
// down here to save memory.
@@ -1534,13 +1536,11 @@ class NonTypeTemplateParmDecl final
/// Return the constraint introduced by the placeholder type of this non-type
/// template parameter (if any).
Expr *getPlaceholderTypeConstraint() const {
- return hasPlaceholderTypeConstraint() ? *getTrailingObjects<Expr *>() :
- nullptr;
+ return PlaceholderTypeConstraintInitialized ? *getTrailingObjects<Expr *>()
+ : nullptr;
}
- void setPlaceholderTypeConstraint(Expr *E) {
- *getTrailingObjects<Expr *>() = E;
- }
+ void setPlaceholderTypeConstraint(Expr *E);
/// Determine whether this non-type template parameter's type has a
/// placeholder with a type-constraint.
diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index 40ee3753c2422..4027764daabfa 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -853,6 +853,16 @@ void NonTypeTemplateParmDecl::setDefaultArgument(
DefaultArgument.set(new (C) TemplateArgumentLoc(DefArg));
}
+void NonTypeTemplateParmDecl::setPlaceholderTypeConstraint(Expr *E) {
+ assert(hasPlaceholderTypeConstraint() &&
+ "setPlaceholderTypeConstraint called on a NonTypeTemplateParmDecl "
+ "without constraint");
+ assert(!PlaceholderTypeConstraintInitialized && "PlaceholderTypeConstraint "
+ "was already initialized!");
+ *getTrailingObjects<Expr *>() = E;
+ PlaceholderTypeConstraintInitialized = true;
+}
+
//===----------------------------------------------------------------------===//
// TemplateTemplateParmDecl Method Implementations
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 719bc0d06f5b1..0154c97ccb59d 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2684,7 +2684,7 @@ void ASTDeclReader::VisitNonTypeTemplateParmDecl(NonTypeTemplateParmDecl *D) {
// TemplateParmPosition.
D->setDepth(Record.readInt());
D->setPosition(Record.readInt());
- if (D->hasPlaceholderTypeConstraint())
+ if (Record.readBool()) // PlaceholderTypeConstraintInitialized
D->setPlaceholderTypeConstraint(Record.readExpr());
if (D->isExpandedParameterPack()) {
auto TypesAndInfos =
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index 75c1d9a6d438c..d9fae52c17f07 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1983,8 +1983,7 @@ void ASTDeclWriter::VisitNonTypeTemplateParmDecl(NonTypeTemplateParmDecl *D) {
// For an expanded parameter pack, record the number of expansion types here
// so that it's easier for deserialization to allocate the right amount of
// memory.
- Expr *TypeConstraint = D->getPlaceholderTypeConstraint();
- Record.push_back(!!TypeConstraint);
+ Record.push_back(D->hasPlaceholderTypeConstraint());
if (D->isExpandedParameterPack())
Record.push_back(D->getNumExpansionTypes());
@@ -1992,6 +1991,9 @@ void ASTDeclWriter::VisitNonTypeTemplateParmDecl(NonTypeTemplateParmDecl *D) {
// TemplateParmPosition.
Record.push_back(D->getDepth());
Record.push_back(D->getPosition());
+ Expr *TypeConstraint = D->getPlaceholderTypeConstraint();
+ Record.push_back(/*PlaceholderTypeConstraintInitialized=*/TypeConstraint !=
+ nullptr);
if (TypeConstraint)
Record.AddStmt(TypeConstraint);
diff --git a/clang/test/Modules/malformed-constraint-template-non-type-parm-decl.cpp b/clang/test/Modules/malformed-constraint-template-non-type-parm-decl.cpp
index cea6404bbebd2..73dff88e506b4 100644
--- a/clang/test/Modules/malformed-constraint-template-non-type-parm-decl.cpp
+++ b/clang/test/Modules/malformed-constraint-template-non-type-parm-decl.cpp
@@ -5,6 +5,9 @@
// RUN: %clang_cc1 -std=c++20 mod.cppm -emit-module-interface -o mod.pcm -fallow-pcm-with-compiler-errors -verify
// RUN: %clang_cc1 -std=c++20 main.cpp -fmodule-file=mod=mod.pcm -verify -fallow-pcm-with-compiler-errors -fsyntax-only -ast-dump-all | FileCheck %s
+// RUN: %clang_cc1 -std=c++20 mod.cppm -emit-reduced-module-interface -o mod.pcm -fallow-pcm-with-compiler-errors -verify
+// RUN: %clang_cc1 -std=c++20 main.cpp -fmodule-file=mod=mod.pcm -verify -fallow-pcm-with-compiler-errors -fsyntax-only -ast-dump-all | FileCheck %s
+
//--- mod.cppm
export module mod;
@@ -29,7 +32,6 @@ import mod;
// CHECK: |-FunctionTemplateDecl {{.*}} <line:11:1, line:12:22> col:6 imported in mod hidden invalid cos
// CHECK-NEXT: | |-NonTypeTemplateParmDecl {{.*}} <line:11:10, col:34> col:34 imported in mod hidden referenced invalid 'ReferenceOf<angle> auto' depth 0 index 0 R
-// CHECK-NEXT: | | `-RecoveryExpr {{.*}} <col:10, col:34> 'ReferenceOf<angle> auto' contains-errors lvalue
// CHECK-NEXT: | |-TemplateTypeParmDecl {{.*}} <col:37, col:46> col:46 imported in mod hidden referenced typename depth 0 index 1 Rep
// CHECK-NEXT: | |-RequiresExpr {{.*}} <col:60, col:84> 'bool'
// CHECK-NEXT: | | |-ParmVarDecl {{.*}} <col:69, col:73> col:73 imported in mod hidden referenced v 'Rep'
@@ -42,7 +44,6 @@ import mod;
// CHECK: |-FunctionTemplateDecl {{.*}} <line:15:1, line:16:22> col:6 imported in mod hidden invalid tan
// CHECK-NEXT: | |-NonTypeTemplateParmDecl {{.*}} <line:15:10, col:34> col:34 imported in mod hidden referenced invalid 'ReferenceOf<angle> auto' depth 0 index 0 R
-// CHECK-NEXT: | | `-RecoveryExpr {{.*}} <col:10, col:34> 'ReferenceOf<angle> auto' contains-errors lvalue
// CHECK-NEXT: | |-TemplateTypeParmDecl {{.*}} <col:37, col:46> col:46 imported in mod hidden referenced typename depth 0 index 1 Rep
// CHECK-NEXT: | |-RequiresExpr {{.*}} <col:60, col:84> 'bool'
// CHECK-NEXT: | | |-ParmVarDecl {{.*}} <col:69, col:73> col:73 imported in mod hidden referenced v 'Rep'
>From 38d144e38bee202ac4f1c237b0e5c77958820bb9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alvarez at sonarsource.com>
Date: Mon, 20 Jan 2025 17:34:29 +0100
Subject: [PATCH 6/8] Do not serialize PlaceholderTypeConstraintInitialized if
hasPlaceholderTypeConstraint is false
---
clang/lib/Serialization/ASTReaderDecl.cpp | 6 ++++--
clang/lib/Serialization/ASTWriterDecl.cpp | 13 ++++++++-----
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index c4f9cb12ad12f..a031bb18f0044 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2703,8 +2703,10 @@ void ASTDeclReader::VisitNonTypeTemplateParmDecl(NonTypeTemplateParmDecl *D) {
// TemplateParmPosition.
D->setDepth(Record.readInt());
D->setPosition(Record.readInt());
- if (Record.readBool()) // PlaceholderTypeConstraintInitialized
- D->setPlaceholderTypeConstraint(Record.readExpr());
+ if (D->hasPlaceholderTypeConstraint()) {
+ if (Record.readBool()) // PlaceholderTypeConstraintInitialized
+ D->setPlaceholderTypeConstraint(Record.readExpr());
+ }
if (D->isExpandedParameterPack()) {
auto TypesAndInfos =
D->getTrailingObjects<std::pair<QualType, TypeSourceInfo *>>();
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index 77f49af375f56..ec27a197919b0 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1993,11 +1993,14 @@ void ASTDeclWriter::VisitNonTypeTemplateParmDecl(NonTypeTemplateParmDecl *D) {
// TemplateParmPosition.
Record.push_back(D->getDepth());
Record.push_back(D->getPosition());
- Expr *TypeConstraint = D->getPlaceholderTypeConstraint();
- Record.push_back(/*PlaceholderTypeConstraintInitialized=*/TypeConstraint !=
- nullptr);
- if (TypeConstraint)
- Record.AddStmt(TypeConstraint);
+
+ if (D->hasPlaceholderTypeConstraint()) {
+ Expr *TypeConstraint = D->getPlaceholderTypeConstraint();
+ Record.push_back(/*PlaceholderTypeConstraintInitialized=*/TypeConstraint !=
+ nullptr);
+ if (TypeConstraint)
+ Record.AddStmt(TypeConstraint);
+ }
if (D->isExpandedParameterPack()) {
for (unsigned I = 0, N = D->getNumExpansionTypes(); I != N; ++I) {
>From cfd9d69ac3e2fb3778a04395ffd8200bce01e627 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alvarez at sonarsource.com>
Date: Fri, 24 Jan 2025 15:59:52 +0100
Subject: [PATCH 7/8] Simpler approach
---
clang/include/clang/AST/DeclTemplate.h | 10 ++---
clang/lib/AST/DeclTemplate.cpp | 55 ++++++++++++-----------
clang/lib/Serialization/ASTReaderDecl.cpp | 6 +--
clang/lib/Serialization/ASTWriterDecl.cpp | 9 +---
4 files changed, 38 insertions(+), 42 deletions(-)
diff --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h
index 60d8ef7706f32..8c2da97c07a3b 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -1365,8 +1365,6 @@ class NonTypeTemplateParmDecl final
DefaultArgStorage<NonTypeTemplateParmDecl, TemplateArgumentLoc *>;
DefArgStorage DefaultArgument;
- bool PlaceholderTypeConstraintInitialized = false;
-
// FIXME: Collapse this into TemplateParamPosition; or, just move depth/index
// down here to save memory.
@@ -1535,11 +1533,13 @@ class NonTypeTemplateParmDecl final
/// Return the constraint introduced by the placeholder type of this non-type
/// template parameter (if any).
Expr *getPlaceholderTypeConstraint() const {
- return PlaceholderTypeConstraintInitialized ? *getTrailingObjects<Expr *>()
- : nullptr;
+ return hasPlaceholderTypeConstraint() ? *getTrailingObjects<Expr *>() :
+ nullptr;
}
- void setPlaceholderTypeConstraint(Expr *E);
+ void setPlaceholderTypeConstraint(Expr *E) {
+ *getTrailingObjects<Expr *>() = E;
+ }
/// Determine whether this non-type template parameter's type has a
/// placeholder with a type-constraint.
diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index 4027764daabfa..263b24ca8499b 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -786,12 +786,16 @@ NonTypeTemplateParmDecl *NonTypeTemplateParmDecl::Create(
QualType T, bool ParameterPack, TypeSourceInfo *TInfo) {
AutoType *AT =
C.getLangOpts().CPlusPlus20 ? T->getContainedAutoType() : nullptr;
- return new (C, DC,
- additionalSizeToAlloc<std::pair<QualType, TypeSourceInfo *>,
- Expr *>(0,
- AT && AT->isConstrained() ? 1 : 0))
- NonTypeTemplateParmDecl(DC, StartLoc, IdLoc, D, P, Id, T, ParameterPack,
- TInfo);
+ bool const HasConstraint = AT && AT->isConstrained();
+ auto *NTTP =
+ new (C, DC,
+ additionalSizeToAlloc<std::pair<QualType, TypeSourceInfo *>, Expr *>(
+ 0, HasConstraint ? 1 : 0))
+ NonTypeTemplateParmDecl(DC, StartLoc, IdLoc, D, P, Id, T,
+ ParameterPack, TInfo);
+ if (HasConstraint)
+ NTTP->setPlaceholderTypeConstraint(nullptr);
+ return NTTP;
}
NonTypeTemplateParmDecl *NonTypeTemplateParmDecl::Create(
@@ -800,23 +804,30 @@ NonTypeTemplateParmDecl *NonTypeTemplateParmDecl::Create(
QualType T, TypeSourceInfo *TInfo, ArrayRef<QualType> ExpandedTypes,
ArrayRef<TypeSourceInfo *> ExpandedTInfos) {
AutoType *AT = TInfo->getType()->getContainedAutoType();
- return new (C, DC,
- additionalSizeToAlloc<std::pair<QualType, TypeSourceInfo *>,
- Expr *>(
- ExpandedTypes.size(), AT && AT->isConstrained() ? 1 : 0))
- NonTypeTemplateParmDecl(DC, StartLoc, IdLoc, D, P, Id, T, TInfo,
- ExpandedTypes, ExpandedTInfos);
+ bool const HasConstraint = AT && AT->isConstrained();
+ auto *NTTP =
+ new (C, DC,
+ additionalSizeToAlloc<std::pair<QualType, TypeSourceInfo *>, Expr *>(
+ ExpandedTypes.size(), HasConstraint ? 1 : 0))
+ NonTypeTemplateParmDecl(DC, StartLoc, IdLoc, D, P, Id, T, TInfo,
+ ExpandedTypes, ExpandedTInfos);
+ if (HasConstraint)
+ NTTP->setPlaceholderTypeConstraint(nullptr);
+ return NTTP;
}
NonTypeTemplateParmDecl *
NonTypeTemplateParmDecl::CreateDeserialized(ASTContext &C, GlobalDeclID ID,
bool HasTypeConstraint) {
- return new (C, ID, additionalSizeToAlloc<std::pair<QualType,
- TypeSourceInfo *>,
- Expr *>(0,
- HasTypeConstraint ? 1 : 0))
+ auto *NTTP =
+ new (C, ID,
+ additionalSizeToAlloc<std::pair<QualType, TypeSourceInfo *>, Expr *>(
+ 0, HasTypeConstraint ? 1 : 0))
NonTypeTemplateParmDecl(nullptr, SourceLocation(), SourceLocation(),
0, 0, nullptr, QualType(), false, nullptr);
+ if (HasTypeConstraint)
+ NTTP->setPlaceholderTypeConstraint(nullptr);
+ return NTTP;
}
NonTypeTemplateParmDecl *
@@ -830,6 +841,8 @@ NonTypeTemplateParmDecl::CreateDeserialized(ASTContext &C, GlobalDeclID ID,
NonTypeTemplateParmDecl(nullptr, SourceLocation(), SourceLocation(),
0, 0, nullptr, QualType(), nullptr, {}, {});
NTTP->NumExpandedTypes = NumExpandedTypes;
+ if (HasTypeConstraint)
+ NTTP->setPlaceholderTypeConstraint(nullptr);
return NTTP;
}
@@ -853,16 +866,6 @@ void NonTypeTemplateParmDecl::setDefaultArgument(
DefaultArgument.set(new (C) TemplateArgumentLoc(DefArg));
}
-void NonTypeTemplateParmDecl::setPlaceholderTypeConstraint(Expr *E) {
- assert(hasPlaceholderTypeConstraint() &&
- "setPlaceholderTypeConstraint called on a NonTypeTemplateParmDecl "
- "without constraint");
- assert(!PlaceholderTypeConstraintInitialized && "PlaceholderTypeConstraint "
- "was already initialized!");
- *getTrailingObjects<Expr *>() = E;
- PlaceholderTypeConstraintInitialized = true;
-}
-
//===----------------------------------------------------------------------===//
// TemplateTemplateParmDecl Method Implementations
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 2227e15a2f8e1..0b75468a94103 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2713,10 +2713,8 @@ void ASTDeclReader::VisitNonTypeTemplateParmDecl(NonTypeTemplateParmDecl *D) {
// TemplateParmPosition.
D->setDepth(Record.readInt());
D->setPosition(Record.readInt());
- if (D->hasPlaceholderTypeConstraint()) {
- if (Record.readBool()) // PlaceholderTypeConstraintInitialized
- D->setPlaceholderTypeConstraint(Record.readExpr());
- }
+ if (D->hasPlaceholderTypeConstraint())
+ D->setPlaceholderTypeConstraint(Record.readExpr());
if (D->isExpandedParameterPack()) {
auto TypesAndInfos =
D->getTrailingObjects<std::pair<QualType, TypeSourceInfo *>>();
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index d6bce66939ff0..ac6f19f8f7125 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2020,13 +2020,8 @@ void ASTDeclWriter::VisitNonTypeTemplateParmDecl(NonTypeTemplateParmDecl *D) {
Record.push_back(D->getDepth());
Record.push_back(D->getPosition());
- if (D->hasPlaceholderTypeConstraint()) {
- Expr *TypeConstraint = D->getPlaceholderTypeConstraint();
- Record.push_back(/*PlaceholderTypeConstraintInitialized=*/TypeConstraint !=
- nullptr);
- if (TypeConstraint)
- Record.AddStmt(TypeConstraint);
- }
+ if (D->hasPlaceholderTypeConstraint())
+ Record.AddStmt(D->getPlaceholderTypeConstraint());
if (D->isExpandedParameterPack()) {
for (unsigned I = 0, N = D->getNumExpansionTypes(); I != N; ++I) {
>From 786d7bded7b818a15212a715ba8c34687a446f18 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alvarez at sonarsource.com>
Date: Wed, 12 Feb 2025 12:55:30 +0100
Subject: [PATCH 8/8] Change order of const
---
clang/lib/AST/DeclTemplate.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/AST/DeclTemplate.cpp b/clang/lib/AST/DeclTemplate.cpp
index 0cdf146fda7c9..e6f9e84290b68 100644
--- a/clang/lib/AST/DeclTemplate.cpp
+++ b/clang/lib/AST/DeclTemplate.cpp
@@ -786,7 +786,7 @@ NonTypeTemplateParmDecl *NonTypeTemplateParmDecl::Create(
QualType T, bool ParameterPack, TypeSourceInfo *TInfo) {
AutoType *AT =
C.getLangOpts().CPlusPlus20 ? T->getContainedAutoType() : nullptr;
- bool const HasConstraint = AT && AT->isConstrained();
+ const bool HasConstraint = AT && AT->isConstrained();
auto *NTTP =
new (C, DC,
additionalSizeToAlloc<std::pair<QualType, TypeSourceInfo *>, Expr *>(
More information about the cfe-commits
mailing list