[PATCH] D143334: [clang][Interp] Fix diagnosing uninitialized ctor record arrays
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG40c26ec48c8a: [clang][Interp] Fix diagnosing uninitialized
ctor record arrays (authored by tbaeder).
Changed prior to commit:
https://reviews.llvm.org/D143334?vs=494879=526970#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143334/new/
https://reviews.llvm.org/D143334
Files:
clang/lib/AST/Interp/Interp.cpp
clang/test/AST/Interp/cxx20.cpp
Index: clang/test/AST/Interp/cxx20.cpp
===
--- clang/test/AST/Interp/cxx20.cpp
+++ clang/test/AST/Interp/cxx20.cpp
@@ -138,8 +138,8 @@
namespace UninitializedFields {
class A {
public:
-int a; // expected-note 2{{subobject declared here}} \
- // ref-note 2{{subobject declared here}}
+int a; // expected-note 3{{subobject declared here}} \
+ // ref-note 3{{subobject declared here}}
constexpr A() {}
};
constexpr A a; // expected-error {{must be initialized by a constant
expression}} \
@@ -174,19 +174,15 @@
// ref-error {{must be initialized by a constant
expression}} \
// ref-note {{subobject 'a' is not initialized}}
-
- // FIXME: These two are currently disabled because the array fields
- // cannot be initialized.
-#if 0
class C3 {
public:
A a[2];
constexpr C3() {}
};
constexpr C3 c3; // expected-error {{must be initialized by a constant
expression}} \
- // expected-note {{subobject of type 'int' is not
initialized}} \
+ // expected-note {{subobject 'a' is not initialized}} \
// ref-error {{must be initialized by a constant
expression}} \
- // ref-note {{subobject of type 'int' is not initialized}}
+ // ref-note {{subobject 'a' is not initialized}}
class C4 {
public:
@@ -195,10 +191,9 @@
constexpr C4(){}
};
constexpr C4 c4; // expected-error {{must be initialized by a constant
expression}} \
- // expected-note {{subobject of type 'bool' is not
initialized}} \
+ // expected-note {{subobject 'B' is not initialized}} \
// ref-error {{must be initialized by a constant
expression}} \
- // ref-note {{subobject of type 'bool' is not initialized}}
-#endif
+ // ref-note {{subobject 'B' is not initialized}}
};
namespace ConstThis {
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -386,7 +386,7 @@
size_t NumElems = CAT->getSize().getZExtValue();
QualType ElemType = CAT->getElementType();
- if (isa(ElemType.getTypePtr())) {
+ if (ElemType->isRecordType()) {
const Record *R = BasePtr.getElemRecord();
for (size_t I = 0; I != NumElems; ++I) {
Pointer ElemPtr = BasePtr.atIndex(I).narrow();
Index: clang/test/AST/Interp/cxx20.cpp
===
--- clang/test/AST/Interp/cxx20.cpp
+++ clang/test/AST/Interp/cxx20.cpp
@@ -138,8 +138,8 @@
namespace UninitializedFields {
class A {
public:
-int a; // expected-note 2{{subobject declared here}} \
- // ref-note 2{{subobject declared here}}
+int a; // expected-note 3{{subobject declared here}} \
+ // ref-note 3{{subobject declared here}}
constexpr A() {}
};
constexpr A a; // expected-error {{must be initialized by a constant expression}} \
@@ -174,19 +174,15 @@
// ref-error {{must be initialized by a constant expression}} \
// ref-note {{subobject 'a' is not initialized}}
-
- // FIXME: These two are currently disabled because the array fields
- // cannot be initialized.
-#if 0
class C3 {
public:
A a[2];
constexpr C3() {}
};
constexpr C3 c3; // expected-error {{must be initialized by a constant expression}} \
- // expected-note {{subobject of type 'int' is not initialized}} \
+ // expected-note {{subobject 'a' is not initialized}} \
// ref-error {{must be initialized by a constant expression}} \
- // ref-note {{subobject of type 'int' is not initialized}}
+ // ref-note {{subobject 'a' is not initialized}}
class C4 {
public:
@@ -195,10 +191,9 @@
constexpr C4(){}
};
constexpr C4 c4; // expected-error {{must be initialized by a constant expression}} \
- // expected-note {{subobject of type 'bool' is not initialized}} \
+ // expected-note {{subobject 'B' is not initialized}} \
// ref-error {{must be initialized by a constant expression}} \
- // ref-note
[PATCH] D143334: [clang][Interp] Fix diagnosing uninitialized ctor record arrays
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LG with a request for an additional test.
Comment at: clang/lib/AST/Interp/Interp.cpp:390
- if (isa(ElemType.getTypePtr())) {
+ if (ElemType->isRecordType()) {
const Record *R = BasePtr.getElemRecord();
tbaeder wrote:
> shafik wrote:
> > aaron.ballman wrote:
> > > The difference between these two is that `isRecordType()` is looking at
> > > the canonical type whereas `isa<>` is looking at the type under
> > > inspection rather than the canonical type. I'd expect these to have the
> > > same behavior in most cases, but only matter for cases involving typedefs.
> > >
> > > I think you're correct about the test case below not needing these
> > > particular changes -- at least, I'm not seeing what's changed that should
> > > impact the test. Should this be split into two changes? 1) Expose the
> > > test, 2) Make this functional change + add a new test where the canonical
> > > type is different to demonstrate the fix.
> > +1
> Can you come up with a small test case that would show the difference? You
> mentioned typedefs, but if the array is of a typedef type, the old `isa<>`
> version doesn't work either.
Ah, I see what you mean, yeah. With a typedef type, the `isa` would have also
failed. That's a good test case to add to this patch as well.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143334/new/
https://reviews.llvm.org/D143334
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D143334: [clang][Interp] Fix diagnosing uninitialized ctor record arrays
tbaeder added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143334/new/ https://reviews.llvm.org/D143334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D143334: [clang][Interp] Fix diagnosing uninitialized ctor record arrays
tbaeder added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143334/new/ https://reviews.llvm.org/D143334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D143334: [clang][Interp] Fix diagnosing uninitialized ctor record arrays
tbaeder added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143334/new/ https://reviews.llvm.org/D143334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D143334: [clang][Interp] Fix diagnosing uninitialized ctor record arrays
tbaeder added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143334/new/ https://reviews.llvm.org/D143334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D143334: [clang][Interp] Fix diagnosing uninitialized ctor record arrays
tbaeder added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143334/new/ https://reviews.llvm.org/D143334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D143334: [clang][Interp] Fix diagnosing uninitialized ctor record arrays
tbaeder added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143334/new/ https://reviews.llvm.org/D143334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D143334: [clang][Interp] Fix diagnosing uninitialized ctor record arrays
tbaeder added inline comments.
Comment at: clang/lib/AST/Interp/Interp.cpp:390
- if (isa(ElemType.getTypePtr())) {
+ if (ElemType->isRecordType()) {
const Record *R = BasePtr.getElemRecord();
shafik wrote:
> aaron.ballman wrote:
> > The difference between these two is that `isRecordType()` is looking at the
> > canonical type whereas `isa<>` is looking at the type under inspection
> > rather than the canonical type. I'd expect these to have the same behavior
> > in most cases, but only matter for cases involving typedefs.
> >
> > I think you're correct about the test case below not needing these
> > particular changes -- at least, I'm not seeing what's changed that should
> > impact the test. Should this be split into two changes? 1) Expose the test,
> > 2) Make this functional change + add a new test where the canonical type is
> > different to demonstrate the fix.
> +1
Can you come up with a small test case that would show the difference? You
mentioned typedefs, but if the array is of a typedef type, the old `isa<>`
version doesn't work either.
Comment at: clang/test/AST/Interp/cxx20.cpp:182
};
constexpr C3 c3; // expected-error {{must be initialized by a constant
expression}} \
// expected-note {{subobject of type 'int' is not
initialized}} \
This line actually needs the changes in this patch, the array is of record type.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143334/new/
https://reviews.llvm.org/D143334
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D143334: [clang][Interp] Fix diagnosing uninitialized ctor record arrays
shafik added inline comments.
Comment at: clang/lib/AST/Interp/Interp.cpp:390
- if (isa(ElemType.getTypePtr())) {
+ if (ElemType->isRecordType()) {
const Record *R = BasePtr.getElemRecord();
aaron.ballman wrote:
> The difference between these two is that `isRecordType()` is looking at the
> canonical type whereas `isa<>` is looking at the type under inspection rather
> than the canonical type. I'd expect these to have the same behavior in most
> cases, but only matter for cases involving typedefs.
>
> I think you're correct about the test case below not needing these particular
> changes -- at least, I'm not seeing what's changed that should impact the
> test. Should this be split into two changes? 1) Expose the test, 2) Make this
> functional change + add a new test where the canonical type is different to
> demonstrate the fix.
+1
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143334/new/
https://reviews.llvm.org/D143334
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D143334: [clang][Interp] Fix diagnosing uninitialized ctor record arrays
aaron.ballman added inline comments.
Comment at: clang/lib/AST/Interp/Interp.cpp:390
- if (isa(ElemType.getTypePtr())) {
+ if (ElemType->isRecordType()) {
const Record *R = BasePtr.getElemRecord();
The difference between these two is that `isRecordType()` is looking at the
canonical type whereas `isa<>` is looking at the type under inspection rather
than the canonical type. I'd expect these to have the same behavior in most
cases, but only matter for cases involving typedefs.
I think you're correct about the test case below not needing these particular
changes -- at least, I'm not seeing what's changed that should impact the test.
Should this be split into two changes? 1) Expose the test, 2) Make this
functional change + add a new test where the canonical type is different to
demonstrate the fix.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143334/new/
https://reviews.llvm.org/D143334
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D143334: [clang][Interp] Fix diagnosing uninitialized ctor record arrays
tbaeder added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143334/new/ https://reviews.llvm.org/D143334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D143334: [clang][Interp] Fix diagnosing uninitialized ctor record arrays
tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
No idea why I used `isa(ElemType.getTypePtr())` here, but it broke
detecting that the element type is of record type.
Other than that, I think a previous patch made it possible to enable those two
tests that have been disabled so far.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D143334
Files:
clang/lib/AST/Interp/Interp.cpp
clang/test/AST/Interp/cxx20.cpp
Index: clang/test/AST/Interp/cxx20.cpp
===
--- clang/test/AST/Interp/cxx20.cpp
+++ clang/test/AST/Interp/cxx20.cpp
@@ -138,8 +138,8 @@
namespace UninitializedFields {
class A {
public:
-int a; // expected-note 2{{subobject declared here}} \
- // ref-note 2{{subobject declared here}}
+int a; // expected-note 3{{subobject declared here}} \
+ // ref-note 3{{subobject declared here}}
constexpr A() {}
};
constexpr A a; // expected-error {{must be initialized by a constant
expression}} \
@@ -174,10 +174,6 @@
// ref-error {{must be initialized by a constant
expression}} \
// ref-note {{subobject of type 'int' is not initialized}}
-
- // FIXME: These two are currently disabled because the array fields
- // cannot be initialized.
-#if 0
class C3 {
public:
A a[2];
@@ -198,7 +194,6 @@
// expected-note {{subobject of type 'bool' is not
initialized}} \
// ref-error {{must be initialized by a constant
expression}} \
// ref-note {{subobject of type 'bool' is not initialized}}
-#endif
};
namespace ConstThis {
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -387,7 +387,7 @@
size_t NumElems = CAT->getSize().getZExtValue();
QualType ElemType = CAT->getElementType();
- if (isa(ElemType.getTypePtr())) {
+ if (ElemType->isRecordType()) {
const Record *R = BasePtr.getElemRecord();
for (size_t I = 0; I != NumElems; ++I) {
Pointer ElemPtr = BasePtr.atIndex(I).narrow();
Index: clang/test/AST/Interp/cxx20.cpp
===
--- clang/test/AST/Interp/cxx20.cpp
+++ clang/test/AST/Interp/cxx20.cpp
@@ -138,8 +138,8 @@
namespace UninitializedFields {
class A {
public:
-int a; // expected-note 2{{subobject declared here}} \
- // ref-note 2{{subobject declared here}}
+int a; // expected-note 3{{subobject declared here}} \
+ // ref-note 3{{subobject declared here}}
constexpr A() {}
};
constexpr A a; // expected-error {{must be initialized by a constant expression}} \
@@ -174,10 +174,6 @@
// ref-error {{must be initialized by a constant expression}} \
// ref-note {{subobject of type 'int' is not initialized}}
-
- // FIXME: These two are currently disabled because the array fields
- // cannot be initialized.
-#if 0
class C3 {
public:
A a[2];
@@ -198,7 +194,6 @@
// expected-note {{subobject of type 'bool' is not initialized}} \
// ref-error {{must be initialized by a constant expression}} \
// ref-note {{subobject of type 'bool' is not initialized}}
-#endif
};
namespace ConstThis {
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -387,7 +387,7 @@
size_t NumElems = CAT->getSize().getZExtValue();
QualType ElemType = CAT->getElementType();
- if (isa(ElemType.getTypePtr())) {
+ if (ElemType->isRecordType()) {
const Record *R = BasePtr.getElemRecord();
for (size_t I = 0; I != NumElems; ++I) {
Pointer ElemPtr = BasePtr.atIndex(I).narrow();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
