[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -0,0 +1,55 @@
+// 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
+
+// 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;
+
+template
+concept ReferenceOf = Q;
+
+// expected-error@+2 {{unknown type name 'AngleIsInvalidNow'}}
+// expected-error@+1 {{constexpr variable 'angle' must be initialized by a
constant expression}}
+constexpr struct angle {AngleIsInvalidNow e;} angle;
+
+// expected-error@+1 {{non-type template argument is not a constant
expression}}
+template auto R, typename Rep> requires requires(Rep v)
{cos(v);}
+auto cos(const Rep& q);
+
+// expected-error@+1 {{non-type template argument is not a constant
expression}}
+template auto R, typename Rep> requires requires(Rep v)
{tan(v);}
+auto tan(const Rep& q);
+
+//--- main.cpp
+// expected-no-diagnostics
+import mod;
+
+// CHECK: |-FunctionTemplateDecl {{.*}} col:6
imported in mod hidden invalid cos
+// CHECK-NEXT: | |-NonTypeTemplateParmDecl {{.*}} col:34
imported in mod hidden referenced invalid 'ReferenceOf auto' depth 0
index 0 R
+// CHECK-NEXT: | |-TemplateTypeParmDecl {{.*}} col:46
imported in mod hidden referenced typename depth 0 index 1 Rep
+// CHECK-NEXT: | |-RequiresExpr {{.*}} 'bool'
+// CHECK-NEXT: | | |-ParmVarDecl {{.*}} col:73 imported in
mod hidden referenced v 'Rep'
+// CHECK-NEXT: | | `-SimpleRequirement {{.*}} dependent
+// CHECK-NEXT: | | `-CallExpr {{.*}} ''
+// CHECK-NEXT: | | |-UnresolvedLookupExpr {{.*}} '' lvalue (ADL) = 'cos' empty
+// CHECK-NEXT: | | `-DeclRefExpr {{.*}} 'Rep' lvalue ParmVar
{{.*}} 'v' 'Rep' non_odr_use_unevaluated
+// CHECK-NEXT: | `-FunctionDecl {{.*}} col:6 imported in
mod hidden cos 'auto (const Rep &)'
+// CHECK-NEXT: | `-ParmVarDecl {{.*}} col:21 imported in
mod hidden q 'const Rep &'
+
+// CHECK: |-FunctionTemplateDecl {{.*}} col:6
imported in mod hidden invalid tan
+// CHECK-NEXT: | |-NonTypeTemplateParmDecl {{.*}} col:34
imported in mod hidden referenced invalid 'ReferenceOf auto' depth 0
index 0 R
+// CHECK-NEXT: | |-TemplateTypeParmDecl {{.*}} col:46
imported in mod hidden referenced typename depth 0 index 1 Rep
+// CHECK-NEXT: | |-RequiresExpr {{.*}} 'bool'
+// CHECK-NEXT: | | |-ParmVarDecl {{.*}} col:73 imported in
mod hidden referenced v 'Rep'
+// CHECK-NEXT: | | `-SimpleRequirement {{.*}} dependent
+// CHECK-NEXT: | | `-CallExpr {{.*}} ''
+// CHECK-NEXT: | | |-UnresolvedLookupExpr {{.*}} '' lvalue (ADL) = 'tan' empty
+// CHECK-NEXT: | | `-DeclRefExpr {{.*}} 'Rep' lvalue ParmVar
{{.*}} 'v' 'Rep' non_odr_use_unevaluated
+// CHECK-NEXT: | `-FunctionDecl {{.*}} col:6 imported in
mod hidden tan 'auto (const Rep &)'
+// CHECK-NEXT: | `-ParmVarDecl {{.*}} col:21 imported in
mod hidden q 'const Rep &'
mizvekov wrote:
Please filter out line numbers on the AST dump match, otherwise this makes
these tests hard to update.
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID: In-Reply-To: https://github.com/erichkeane closed https://github.com/llvm/llvm-project/pull/121768 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
Alejandro =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?=, Alejandro =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?=, Alejandro =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?=, Alejandro =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?=, Alejandro =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?=, Alejandro =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?=, Alejandro =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?=, Alejandro =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?= Message-ID: In-Reply-To: https://github.com/erichkeane approved this pull request. https://github.com/llvm/llvm-project/pull/121768 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
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?=
Date: Mon, 18 Nov 2024 11:36:03 +0100
Subject: [PATCH 1/9] [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 0..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
+concept ReferenceOf = Q;
+
+// expected-error@+2 {{unknown type name 'AngleIsInvalidNow'}}
+// expected-error@+1 {{constexpr variable 'angle' must be initialized by a
constant expression}}
+constexpr struct angle {AngleIsInvalidNow e;} angle;
+
+// expected-error@+1 {{non-type template argument is not a constant
expression}}
+template auto R, typename Rep> requires requires(Rep v)
{cos(v);}
+auto cos(const Rep& q);
+
+// expected-error@+1 {{non-type template argument is not a constant
expression}}
+template auto R, typename Rep> requires requires(Rep v)
{tan(v);}
+auto tan(const Rep& q);
+
+//--- main.cpp
+// expected-no-diagnostics
+import mod;
+
+// CHECK: |-FunctionTemplateDecl {{.*}} col:6
imported in mod hidden invalid cos
+// CHECK-NEXT: | |-NonTypeTemplateParmDecl {{.*}} col:34
imported in mod hidden referenced invalid 'ReferenceOf auto' depth 0
index 0 R
+// CHECK-
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
@@ -2016,17 +2016,17 @@ 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();
alejandro-alvarez-sonarsource wrote:
There is a difference: `D->hasPlaceholderTypeConstraint()` may be true, but
`D->getPlaceholderTypeConstraint` may return `nullptr`.
IIUC, before this patch there was a underlying assumption that the former is
true if and only if the latter is not `nullptr`.
This is not aligned when there are parsing errors.
See [my comment
here](https://github.com/llvm/llvm-project/pull/121768#discussion_r1908541893).
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -2016,17 +2016,17 @@ 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();
erichkeane wrote:
Changes here are all NFC, right? Or am I missing a purpose to this change?
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -800,23 +804,30 @@ NonTypeTemplateParmDecl *NonTypeTemplateParmDecl::Create(
QualType T, TypeSourceInfo *TInfo, ArrayRef ExpandedTypes,
ArrayRef ExpandedTInfos) {
AutoType *AT = TInfo->getType()->getContainedAutoType();
- return new (C, DC,
- additionalSizeToAlloc,
-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();
erichkeane wrote:
```suggestion
const bool HasConstraint = AT && AT->isConstrained();
```
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
@@ -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,
-Expr *>(0,
-AT && AT->isConstrained() ? 1 : 0))
- NonTypeTemplateParmDecl(DC, StartLoc, IdLoc, D, P, Id, T, ParameterPack,
- TInfo);
+ bool const HasConstraint = AT && AT->isConstrained();
alejandro-alvarez-sonarsource wrote:
Thanks! Swapped the order of the cost. Would you mind doing the merge?
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
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?=
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 0..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
+concept ReferenceOf = Q;
+
+// expected-error@+2 {{unknown type name 'AngleIsInvalidNow'}}
+// expected-error@+1 {{constexpr variable 'angle' must be initialized by a
constant expression}}
+constexpr struct angle {AngleIsInvalidNow e;} angle;
+
+// expected-error@+1 {{non-type template argument is not a constant
expression}}
+template auto R, typename Rep> requires requires(Rep v)
{cos(v);}
+auto cos(const Rep& q);
+
+// expected-error@+1 {{non-type template argument is not a constant
expression}}
+template auto R, typename Rep> requires requires(Rep v)
{tan(v);}
+auto tan(const Rep& q);
+
+//--- main.cpp
+// expected-no-diagnostics
+import mod;
+
+// CHECK: |-FunctionTemplateDecl {{.*}} col:6
imported in mod hidden invalid cos
+// CHECK-NEXT: | |-NonTypeTemplateParmDecl {{.*}} col:34
imported in mod hidden referenced invalid 'ReferenceOf auto' depth 0
index 0 R
+// CHECK-
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID: In-Reply-To: https://github.com/zyn0217 edited https://github.com/llvm/llvm-project/pull/121768 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
Alejandro =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?=, Alejandro =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?=, Alejandro =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?=, Alejandro =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?=, Alejandro =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?=, Alejandro =?utf-8?q?=C3=81lvarez_Ayll=C3=B3n?= Message-ID: In-Reply-To: https://github.com/zyn0217 approved this pull request. LGTM modulo a nit https://github.com/llvm/llvm-project/pull/121768 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -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,
-Expr *>(0,
-AT && AT->isConstrained() ? 1 : 0))
- NonTypeTemplateParmDecl(DC, StartLoc, IdLoc, D, P, Id, T, ParameterPack,
- TInfo);
+ bool const HasConstraint = AT && AT->isConstrained();
zyn0217 wrote:
We prefer `const bool` afaik
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
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?=
Date: Mon, 18 Nov 2024 11:36:03 +0100
Subject: [PATCH 1/7] [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 20ec2fbeaa6a8b..9b51f973fb2bb8 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 00..cea6404bbebd28
--- /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
+concept ReferenceOf = Q;
+
+// expected-error@+2 {{unknown type name 'AngleIsInvalidNow'}}
+// expected-error@+1 {{constexpr variable 'angle' must be initialized by a
constant expression}}
+constexpr struct angle {AngleIsInvalidNow e;} angle;
+
+// expected-error@+1 {{non-type template argument is not a constant
expression}}
+template auto R, typename Rep> requires requires(Rep v)
{cos(v);}
+auto cos(const Rep& q);
+
+// expected-error@+1 {{non-type template argument is not a constant
expression}}
+template auto R, typename Rep> requires requires(Rep v)
{tan(v);}
+auto tan(const Rep& q);
+
+//--- main.cpp
+// expected-no-diagnostics
+import mod;
+
+// CHECK: |-FunctionTemplateDecl {{.*}} col:6
imported in mod hidden invalid cos
+// CHECK-NEXT: | |-NonTypeTemplateParmDecl {{.*}} col:34
imported in mod hidden referenced invalid 'ReferenceOf auto' depth 0
index 0 R
+// CH
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -1533,13 +1535,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() :
-nullptr;
+return PlaceholderTypeConstraintInitialized ? *getTrailingObjects()
+: nullptr;
}
- void setPlaceholderTypeConstraint(Expr *E) {
-*getTrailingObjects() = E;
erichkeane wrote:
Ok, then we're on the same page. I do not see a difference between "explicitly
null" and "not initialized" here. I think that is a differentiation without a
cause.
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
@@ -1533,13 +1535,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() :
-nullptr;
+return PlaceholderTypeConstraintInitialized ? *getTrailingObjects()
+: nullptr;
}
- void setPlaceholderTypeConstraint(Expr *E) {
-*getTrailingObjects() = E;
alejandro-alvarez-sonarsource wrote:
The flag would allow to discern between "was not initialized" and "was
explicitly initialized to null".
Now, I have to admit I do not know if the latter is meaningful for a
constrained NTTP.
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -1533,13 +1535,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() :
-nullptr;
+return PlaceholderTypeConstraintInitialized ? *getTrailingObjects()
+: nullptr;
}
- void setPlaceholderTypeConstraint(Expr *E) {
-*getTrailingObjects() = E;
erichkeane wrote:
I'm not complaining about the 'set' method, just that the concern is that it is
uninitialized. I'm saying, why not just initialize it to nullptr? Or am I
misunderstanding the concern.
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
https://github.com/alejandro-alvarez-sonarsource edited https://github.com/llvm/llvm-project/pull/121768 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
@@ -1533,13 +1535,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() :
-nullptr;
+return PlaceholderTypeConstraintInitialized ? *getTrailingObjects()
+: nullptr;
}
- void setPlaceholderTypeConstraint(Expr *E) {
-*getTrailingObjects() = E;
alejandro-alvarez-sonarsource wrote:
Both inside `Sema` and `ASTReaderDecl` the `NonTypeTemplateParmDecl` is created
relatively far from the parsing / deserialization of the constraint.
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?=, Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID: In-Reply-To: erichkeane wrote: Also, can you please update the commit message? it isn't accurate anymore, re `RecoveryExpr`. https://github.com/llvm/llvm-project/pull/121768 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -1533,13 +1535,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() :
-nullptr;
+return PlaceholderTypeConstraintInitialized ? *getTrailingObjects()
+: nullptr;
}
- void setPlaceholderTypeConstraint(Expr *E) {
-*getTrailingObjects() = E;
erichkeane wrote:
While I like the assert-safety that we get from the
`PlaceholderTypeConstraintInitialized`, I'm not sure it is worth the
rigamarole. Why not just have the `ctor` do `setPlaceholderTypeConstraint`?
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
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?=
Date: Mon, 18 Nov 2024 11:36:03 +0100
Subject: [PATCH 1/6] [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 20ec2fbeaa6a8b..9b51f973fb2bb8 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 00..cea6404bbebd28
--- /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
+concept ReferenceOf = Q;
+
+// expected-error@+2 {{unknown type name 'AngleIsInvalidNow'}}
+// expected-error@+1 {{constexpr variable 'angle' must be initialized by a
constant expression}}
+constexpr struct angle {AngleIsInvalidNow e;} angle;
+
+// expected-error@+1 {{non-type template argument is not a constant
expression}}
+template auto R, typename Rep> requires requires(Rep v)
{cos(v);}
+auto cos(const Rep& q);
+
+// expected-error@+1 {{non-type template argument is not a constant
expression}}
+template auto R, typename Rep> requires requires(Rep v)
{tan(v);}
+auto tan(const Rep& q);
+
+//--- main.cpp
+// expected-no-diagnostics
+import mod;
+
+// CHECK: |-FunctionTemplateDecl {{.*}} col:6
imported in mod hidden invalid cos
+// CHECK-NEXT: | |-NonTypeTemplateParmDecl {{.*}} col:34
imported in mod hidden referenced invalid 'ReferenceOf auto' depth 0
index 0 R
+// CH
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -1983,15 +1983,17 @@ 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());
VisitDeclaratorDecl(D);
// TemplateParmPosition.
Record.push_back(D->getDepth());
Record.push_back(D->getPosition());
+ Expr *TypeConstraint = D->getPlaceholderTypeConstraint();
+ Record.push_back(/*PlaceholderTypeConstraintInitialized=*/TypeConstraint !=
+ nullptr);
zyn0217 wrote:
Can we only serialize `PlaceholderTypeConstraintInitialized` only when
`D->hasPlaceholderTypeConstraint()` so that we can save up a bit for a number
of cases?
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
alejandro-alvarez-sonarsource wrote: Ping? https://github.com/llvm/llvm-project/pull/121768 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
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?=
Date: Mon, 18 Nov 2024 11:36:03 +0100
Subject: [PATCH 1/5] [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 20ec2fbeaa6a8b..9b51f973fb2bb8 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 00..cea6404bbebd28
--- /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
+concept ReferenceOf = Q;
+
+// expected-error@+2 {{unknown type name 'AngleIsInvalidNow'}}
+// expected-error@+1 {{constexpr variable 'angle' must be initialized by a
constant expression}}
+constexpr struct angle {AngleIsInvalidNow e;} angle;
+
+// expected-error@+1 {{non-type template argument is not a constant
expression}}
+template auto R, typename Rep> requires requires(Rep v)
{cos(v);}
+auto cos(const Rep& q);
+
+// expected-error@+1 {{non-type template argument is not a constant
expression}}
+template auto R, typename Rep> requires requires(Rep v)
{tan(v);}
+auto tan(const Rep& q);
+
+//--- main.cpp
+// expected-no-diagnostics
+import mod;
+
+// CHECK: |-FunctionTemplateDecl {{.*}} col:6
imported in mod hidden invalid cos
+// CHECK-NEXT: | |-NonTypeTemplateParmDecl {{.*}} col:34
imported in mod hidden referenced invalid 'ReferenceOf auto' depth 0
index 0 R
+// CH
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
@@ -1228,35 +1228,45 @@ 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();
-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);
+ 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())
+ !ImmediatelyDeclaredConstraint.isUsable()) {
+NewConstrainedParm->setPlaceholderTypeConstraint(
+RecoveryExpr::Create(Context, OrigConstrainedParm->getType(),
alejandro-alvarez-sonarsource wrote:
Ok, I have followed a similar approach to #113182. There is no `RecoveryExpr`
but `invalid` is set, which should work to easily identify something went
mising.
Note that I had to patch AstWriterDecl and AstReaderDecl too: there is an extra
boolean to flag the initialization of the underlying pointer.
I also changed
```cpp
Expr *TypeConstraint = D->getPlaceholderTypeConstraint();
Record.push_back(!!TypeConstraint);
```
to
```cpp
Record.push_back(D->hasPlaceholderTypeConstraint());
```
Which I think it is the intended meaning from looking at the reader
```cpp
bool HasTypeConstraint = Record.readInt();
D = NonTypeTemplateParmDecl::CreateDeserialized(Context, ID,
HasTypeConstraint);
break;
```
Otherwise there will be no space for the pointer on the trailing object, but
`hasPlaceholderTypeConstraint` would be returning `true` and that looks
inconsistent to me, even if (I imagine) it is unlikely
`setPlaceholderTypeConstraint` would be called on a deserialized
`NonTypeTemplateParmDecl`
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -1228,35 +1228,45 @@ 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();
-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);
+ 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())
+ !ImmediatelyDeclaredConstraint.isUsable()) {
+NewConstrainedParm->setPlaceholderTypeConstraint(
+RecoveryExpr::Create(Context, OrigConstrainedParm->getType(),
zyn0217 wrote:
> and the trailing object should probably be initialized to null on the
> constructor of NonTypeTemplateParmDecl to catch any other accidental
> uninitialized value.
We have a bit flag `TypeConstraintInitialized` in `TemplateTypeParmDecl` to
represent a state of "having constraint attached but uninitialized (because of
invalid constraint expression)". I think it'd be better to do the same thing in
NonTypeTemplateParmDecl, or the reverse for consistency.
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
@@ -1228,35 +1228,45 @@ 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();
-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);
+ 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())
+ !ImmediatelyDeclaredConstraint.isUsable()) {
+NewConstrainedParm->setPlaceholderTypeConstraint(
+RecoveryExpr::Create(Context, OrigConstrainedParm->getType(),
alejandro-alvarez-sonarsource wrote:
The reasons for the `RecoveryExpr` is, indeed, like @erichkeane said, to be
able to identify on the AST places where things were missing during parsing.
For this particular use case, the frontend (an analyzers) can easily tell apart
1. There is no type constraint (`nullptr`)
2. There should be one but we could not parse it (`RecoveryExpr`)
The second point, IIUC [the
documentation](https://clang.llvm.org/doxygen/classclang_1_1RecoveryExpr.html#details),
is one of the reasons to use `RecoveryExpr`. Option 1 fixes the crash but
loses information that can be used for better diagnosis.
> which was found during reduction of an unrelated bug
Not really, and I apologize if I have given this impression.
We have had few crashes and assertions when parsing `mp-units` with missing
headers. This was not accidentally found because of #118288 (which is my other
PR based on errors from this project).
> I mostly say this because always initializing the placeholder pointer to
> null would have been a much simpler fix, which would potentially cover other
> instances of a similar bug.
I agree with you here, and the trailing object should probably be initialized
to null on the constructor of `NonTypeTemplateParmDecl` to catch any other
accidental uninitialized value. I am happy to do that too if no one objects.
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -1228,35 +1228,45 @@ 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();
-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);
+ 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())
+ !ImmediatelyDeclaredConstraint.isUsable()) {
+NewConstrainedParm->setPlaceholderTypeConstraint(
+RecoveryExpr::Create(Context, OrigConstrainedParm->getType(),
mizvekov wrote:
Right, but the original bug was about fixing a crash-on-invalid, which was
found during reduction of an unrelated bug. It seems this 'improvement' is
being preempted, without an actual motivating case.
The RecoveryExpr doesn't even always have the correct type the expression would
have, so it's not clear what kind of compromise would be achieved here.
I mostly say this because always initializing the placeholder pointer to null
would have been a much simpler fix, which would potentially cover other
instances of a similar bug.
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
Alejandro =?utf-8?q?Álvarez_Ayllón?=,
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -1228,35 +1228,45 @@ 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();
-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);
+ 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())
+ !ImmediatelyDeclaredConstraint.isUsable()) {
+NewConstrainedParm->setPlaceholderTypeConstraint(
+RecoveryExpr::Create(Context, OrigConstrainedParm->getType(),
mizvekov wrote:
It's not immediately clear to me, nor explained in the commit message, what
this RecoveryExpr is expected to buy, versus always initializing to nullptr,
which is the status quo (most of the time, by accident).
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
@@ -1235,15 +1235,24 @@ 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
// unclear in the wording right now.
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(), {}));
alejandro-alvarez-sonarsource wrote:
Refactored into a lambda so there is only one place to handle failures and
introduce the `RecoveryExpr`.
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
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?=
Date: Mon, 18 Nov 2024 11:36:03 +0100
Subject: [PATCH 1/3] [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 20ec2fbeaa6a8b..9b51f973fb2bb8 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 00..cea6404bbebd28
--- /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
+concept ReferenceOf = Q;
+
+// expected-error@+2 {{unknown type name 'AngleIsInvalidNow'}}
+// expected-error@+1 {{constexpr variable 'angle' must be initialized by a
constant expression}}
+constexpr struct angle {AngleIsInvalidNow e;} angle;
+
+// expected-error@+1 {{non-type template argument is not a constant
expression}}
+template auto R, typename Rep> requires requires(Rep v)
{cos(v);}
+auto cos(const Rep& q);
+
+// expected-error@+1 {{non-type template argument is not a constant
expression}}
+template auto R, typename Rep> requires requires(Rep v)
{tan(v);}
+auto tan(const Rep& q);
+
+//--- main.cpp
+// expected-no-diagnostics
+import mod;
+
+// CHECK: |-FunctionTemplateDecl {{.*}} col:6
imported in mod hidden invalid cos
+// CHECK-NEXT: | |-NonTypeTemplateParmDecl {{.*}} col:34
imported in mod hidden referenced invalid 'ReferenceOf auto' depth 0
index 0 R
+// CH
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
Alejandro =?utf-8?q?Álvarez_Ayllón?Message-ID:
In-Reply-To:
@@ -1235,15 +1235,24 @@ 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
// unclear in the wording right now.
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(), {}));
cor3ntin wrote:
After chatting with @erichkeane, the recovery expression is probably fine.
However maybe we could find a way (lambda?) to avoid the repetition here and
line 1263
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
@@ -1235,15 +1235,24 @@ 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
// unclear in the wording right now.
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(), {}));
alejandro-alvarez-sonarsource wrote:
To avoid the crash it should. For our use case we prefer a `RecoveryExpr`
because it is obvious from the AST that something went amiss, so if it is not a
blocker, I'd prefer to keep it.
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
@@ -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 alejandro-alvarez-sonarsource wrote: We don't even need serialization. IIRC it can be triggered with an ast dump, but only if the uninitialized value happens not to be a 0, which is the tricky part to trigger (not using sanitizers, that is). https://github.com/llvm/llvm-project/pull/121768 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
@@ -1235,15 +1235,24 @@ 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
// unclear in the wording right now.
DeclRefExpr *Ref =
BuildDeclRefExpr(OrigConstrainedParm, OrigConstrainedParm->getType(),
VK_PRValue, OrigConstrainedParm->getLocation());
- if (!Ref)
+ if (!Ref) {
alejandro-alvarez-sonarsource wrote:
I took the existing `if`, but looking at `BuildDeclRefExpr` it doesn't seem
like it could return `nullptr`.
I have dropped the condition and added an assert instead.
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
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?=
Date: Mon, 18 Nov 2024 11:36:03 +0100
Subject: [PATCH 1/2] [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 20ec2fbeaa6a8b..9b51f973fb2bb8 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 00..cea6404bbebd28
--- /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
+concept ReferenceOf = Q;
+
+// expected-error@+2 {{unknown type name 'AngleIsInvalidNow'}}
+// expected-error@+1 {{constexpr variable 'angle' must be initialized by a
constant expression}}
+constexpr struct angle {AngleIsInvalidNow e;} angle;
+
+// expected-error@+1 {{non-type template argument is not a constant
expression}}
+template auto R, typename Rep> requires requires(Rep v)
{cos(v);}
+auto cos(const Rep& q);
+
+// expected-error@+1 {{non-type template argument is not a constant
expression}}
+template auto R, typename Rep> requires requires(Rep v)
{tan(v);}
+auto tan(const Rep& q);
+
+//--- main.cpp
+// expected-no-diagnostics
+import mod;
+
+// CHECK: |-FunctionTemplateDecl {{.*}} col:6
imported in mod hidden invalid cos
+// CHECK-NEXT: | |-NonTypeTemplateParmDecl {{.*}} col:34
imported in mod hidden referenced invalid 'ReferenceOf auto' depth 0
index 0 R
+// CH
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
@@ -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 zyn0217 wrote: I'm wondering if this also manifests with just pch serialization - it would always be great if we can simplify the test https://github.com/llvm/llvm-project/pull/121768 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
@@ -1235,15 +1235,24 @@ 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
// unclear in the wording right now.
DeclRefExpr *Ref =
BuildDeclRefExpr(OrigConstrainedParm, OrigConstrainedParm->getType(),
VK_PRValue, OrigConstrainedParm->getLocation());
- if (!Ref)
+ if (!Ref) {
zyn0217 wrote:
+1, `BuildDeclRefExpr` wouldn't fail with a nullptr
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
@@ -1235,15 +1235,24 @@ 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
// unclear in the wording right now.
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(), {}));
cor3ntin wrote:
I think just setting it to nullptr would be sufficient? (maybe with a comment)
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
@@ -1235,15 +1235,24 @@ 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
// unclear in the wording right now.
DeclRefExpr *Ref =
BuildDeclRefExpr(OrigConstrainedParm, OrigConstrainedParm->getType(),
VK_PRValue, OrigConstrainedParm->getLocation());
- if (!Ref)
+ if (!Ref) {
cor3ntin wrote:
Can that actually fail? I wonder if we should not just assert
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
@@ -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(), {}));
cor3ntin wrote:
Same here
https://github.com/llvm/llvm-project/pull/121768
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
llvmbot wrote:
@llvm/pr-subscribers-clang-modules
@llvm/pr-subscribers-clang
Author: Alejandro Álvarez Ayllón (alejandro-alvarez-sonarsource)
Changes
...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.
This does not affect only modules, but it causes a segfault more consistently
when they are involved.
The test case was reduced from mp-units.
---
Full diff: https://github.com/llvm/llvm-project/pull/121768.diff
2 Files Affected:
- (modified) clang/lib/Sema/SemaTemplate.cpp (+16-2)
- (added)
clang/test/Modules/malformed-constraint-template-non-type-parm-decl.cpp (+54)
``diff
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 20ec2fbeaa6a8b..9b51f973fb2bb8 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 00..cea6404bbebd28
--- /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
+concept ReferenceOf = Q;
+
+// expected-error@+2 {{unknown type name 'AngleIsInvalidNow'}}
+// expected-error@+1 {{constexpr variable 'angle' must be initialized by a
constant expression}}
+constexpr struct angle {AngleIsInvalidNow e;} angle;
+
+// expected-error@+1 {{non-type template argument is not a constant
expression}}
+template auto R, typename Rep> requires requires(Rep v)
{cos(v);}
+auto cos(const Rep& q);
+
+// expected-error@+1 {{non-type template argument is not a constant
expression}}
+template auto R, typename Rep> requires requires(Rep v)
{tan(v);}
+auto tan(const Rep& q);
+
+//--- main.cpp
+// expected-no-diagnostics
+import mod;
+
+// CHECK: |-FunctionTemplateDecl {{.*}} col:6
imported in mod hidden invalid cos
+// CHECK-NEXT: | |-NonTypeTemplateParmDecl {{.*}} col:34
imported in mod hidden referenced invalid 'ReferenceOf auto' depth 0
index 0 R
+// CHECK-NEXT: | | `-RecoveryExpr {{.*}} 'ReferenceOf
auto' contains-errors lvalue
+//
[clang] [clang][Sema] Fix initialization of `NonTypeTemplateParmDecl`... (PR #121768)
https://github.com/alejandro-alvarez-sonarsource created
https://github.com/llvm/llvm-project/pull/121768
...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.
This does not affect only modules, but it causes a segfault more consistently
when they are involved.
The test case was reduced from mp-units.
From da2bbf99b8430d8b6aa6bf7969c9825b4d94219b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
Date: Mon, 18 Nov 2024 11:36:03 +0100
Subject: [PATCH] [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 20ec2fbeaa6a8b..9b51f973fb2bb8 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 00..cea6404bbebd28
--- /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
+concept ReferenceOf = Q;
+
+// expected-error@+2 {{unknown type name 'AngleIsInvalidNow'}}
+// expected-error@+1 {{constexpr variable 'angle' must be initialized by a
constant expre
