Hi Jason,
Thanks for the reply and review. I've attached an updated patch with the
change log and sign off.
The change made in gcc/testsuite/g++.dg/warn/pr83054.C is because I think
there is no more warning since we have already devirtualized the
destruction for the array.
Apologies for the poor formatting. It is my first time contributing. Do let
me know if there's any stuff I've missed and feel free to modify the patch
where you deem necessary.
Thanks!
On Wed, Jul 26, 2023 at 12:25 PM Jason Merrill <ja...@redhat.com> wrote:
> On 7/12/23 10:10, Ng YongXiang via Gcc-patches wrote:
> > Component:
> > c++
> >
> > Bug ID:
> > 110057
> >
> > Bugzilla link:
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110057
> >
> > Description:
> > Array should not call virtual destructor of object when array is
> destructed
> >
> > ChangeLog:
> >
> > 2023-07-12 Ng YongXiang <yongxian...@gmail.com> PR c++
> > * Devirtualize auto generated destructor calls of arraycp/ *
> > init.c: Call non virtual destructor of objects in arraytestsuite/
> > * g++.dg/devirt-array-destructor-1.C: New. *
> > g++.dg/devirt-array-destructor-2.C: New.
> >
> >
> > On Wed, Jul 12, 2023 at 5:02 PM Xi Ruoyao <xry...@xry111.site> wrote:
> >
> >> On Wed, 2023-07-12 at 16:58 +0800, Ng YongXiang via Gcc-patches wrote:
> >>> I'm writing to seek for a review for an issue I filed some time ago.
> >>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110057 . A proposed patch
> >> is
> >>> attached in the bug tracker as well.
> >>
> >> You should send the patch to gcc-patches@gcc.gnu.org for a review, see
> >> https://gcc.gnu.org/contribute.html for the details. Generally we
> >> consider patches attached in bugzilla as drafts.
>
> Thanks! The change makes sense under
> https://eel.is/c++draft/expr.delete#3.sentence-2 , but please look again
> at contribute.html.
>
> In particular, the Legal section; you don't seem to have a copyright
> assignment with the FSF, nor do I see a DCO certification
> (https://gcc.gnu.org/dco.html) in your patch.
>
> Like the examples in contribute.html, the subject line should be more
> like "[PATCH] c++: devirtualization of array destruction [PR110057]"
>
> The ChangeLog entry should be in the commit message.
>
> > * g++.dg/warn/pr83054.C: Change expected number of
> devirtualized calls
>
> This isn't just changing the expected number, it's also changing the
> array from a local variable to dynamically allocated, which is a big
> change to what's being tested. If you want to test the dynamic case,
> please add a new test instead of making this change.
>
> > diff --git a/gcc/testsuite/g++.dg/warn/pr83054.C
> b/gcc/testsuite/g++.dg/warn/pr83054.C
> > index 5285f94acee..7cd0951713d 100644
> > --- a/gcc/testsuite/g++.dg/warn/pr83054.C
> > +++ b/gcc/testsuite/g++.dg/warn/pr83054.C
> > @@ -10,7 +10,7 @@
> > #endif
> >
> > extern "C" int printf (const char *, ...);
> > -struct foo // { dg-warning "final would enable devirtualization of 5
> calls" }
> > +struct foo // { dg-warning "final would enable devirtualization of 1
> call" }
> > {
> > static int count;
> > void print (int i, int j) { printf ("foo[%d][%d] = %d\n", i, j, x); }
> > @@ -29,19 +29,15 @@ int foo::count;
> >
> > int main ()
> > {
> > - {
> > - foo array[3][3];
> > - for (int i = 0; i < 3; i++)
> > - {
> > - for (int j = 0; j < 3; j++)
> > - {
> > - printf("&a[%d][%d] = %x\n", i, j, (void *)&array[i][j]);
> > - }
> > - }
> > - // The count should be nine, if not, fail the test.
> > - if (foo::count != 9)
> > - return 1;
> > - }
> > + foo* arr[9];
> > + for (int i = 0; i < 9; ++i)
> > + arr[i] = new foo();
> > + if (foo::count != 9)
> > + return 1;
> > + for (int i = 0; i < 9; ++i)
> > + arr[i]->print(i / 3, i % 3);
> > + for (int i = 0; i < 9; ++i)
> > + delete arr[i];
>
>
>
From 2b7cbd8e0787c8e20f0464dbf610908a9f3c68f7 Mon Sep 17 00:00:00 2001
From: yongxiangng <yongxiangng@gmail.com>
Date: Wed, 26 Jul 2023 23:45:25 +0800
Subject: [PATCH 1/1] [PATCH] c++: devirtualization of array destruction
[PR110057]
cp/ChangeLog:
* init.c: Call non virtual destructor of objects in array
testsuite/ChangeLog:
* g++.dg/devirt-array-destructor-1.C: New.
* g++.dg/devirt-array-destructor-2.C: New.
* g++.dg/warn/pr83054.C: Remove expected warnings caused by devirtualization
Signed-off-by: Ng Yong Xiang <yongxiangng@gmail.com>
---
gcc/cp/init.cc | 8 +++---
.../g++.dg/devirt-array-destructor-1.C | 27 ++++++++++++++++++
.../g++.dg/devirt-array-destructor-2.C | 28 +++++++++++++++++++
gcc/testsuite/g++.dg/warn/pr83054.C | 2 +-
4 files changed, 60 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/devirt-array-destructor-1.C
create mode 100644 gcc/testsuite/g++.dg/devirt-array-destructor-2.C
diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
index 6ccda365b04..69ab51d0a4b 100644
--- a/gcc/cp/init.cc
+++ b/gcc/cp/init.cc
@@ -4112,8 +4112,8 @@ build_vec_delete_1 (location_t loc, tree base, tree maxindex, tree type,
if (type_build_dtor_call (type))
{
tmp = build_delete (loc, ptype, base, sfk_complete_destructor,
- LOOKUP_NORMAL|LOOKUP_DESTRUCTOR, 1,
- complain);
+ LOOKUP_NORMAL|LOOKUP_DESTRUCTOR|LOOKUP_NONVIRTUAL,
+ 1, complain);
if (tmp == error_mark_node)
return error_mark_node;
}
@@ -4143,8 +4143,8 @@ build_vec_delete_1 (location_t loc, tree base, tree maxindex, tree type,
return error_mark_node;
body = build_compound_expr (loc, body, tmp);
tmp = build_delete (loc, ptype, tbase, sfk_complete_destructor,
- LOOKUP_NORMAL|LOOKUP_DESTRUCTOR, 1,
- complain);
+ LOOKUP_NORMAL|LOOKUP_DESTRUCTOR|LOOKUP_NONVIRTUAL,
+ 1, complain);
if (tmp == error_mark_node)
return error_mark_node;
body = build_compound_expr (loc, body, tmp);
diff --git a/gcc/testsuite/g++.dg/devirt-array-destructor-1.C b/gcc/testsuite/g++.dg/devirt-array-destructor-1.C
new file mode 100644
index 00000000000..be2d16ae761
--- /dev/null
+++ b/gcc/testsuite/g++.dg/devirt-array-destructor-1.C
@@ -0,0 +1,27 @@
+/* { dg-do run } */
+/* Virtual calls should be devirtualized because we know dynamic type of object in array at compile time */
+/* { dg-options "-O3 -fdump-tree-optimized -fno-inline" } */
+
+class A
+{
+public:
+ virtual ~A()
+ {
+ }
+};
+
+class B : public A
+{
+public:
+ virtual ~B()
+ {
+ }
+};
+
+int main()
+{
+ B b[10];
+ return 0;
+}
+
+/* { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "optimized"} } */
diff --git a/gcc/testsuite/g++.dg/devirt-array-destructor-2.C b/gcc/testsuite/g++.dg/devirt-array-destructor-2.C
new file mode 100644
index 00000000000..0b3ab2ca9d0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/devirt-array-destructor-2.C
@@ -0,0 +1,28 @@
+/* { dg-do run } */
+/* Virtual calls should be devirtualized because we know dynamic type of object in array at compile time */
+/* { dg-options "-O3 -fdump-tree-optimized -fno-inline" } */
+
+class A
+{
+public:
+ virtual ~A()
+ {
+ }
+};
+
+class B : public A
+{
+public:
+ virtual ~B()
+ {
+ }
+};
+
+int main()
+{
+ B* ptr = new B[10];
+ delete[] ptr;
+ return 0;
+}
+
+/* { dg-final { scan-tree-dump-times "OBJ_TYPE_REF" 0 "optimized"} } */
diff --git a/gcc/testsuite/g++.dg/warn/pr83054.C b/gcc/testsuite/g++.dg/warn/pr83054.C
index 5285f94acee..5a4a6abe248 100644
--- a/gcc/testsuite/g++.dg/warn/pr83054.C
+++ b/gcc/testsuite/g++.dg/warn/pr83054.C
@@ -10,7 +10,7 @@
#endif
extern "C" int printf (const char *, ...);
-struct foo // { dg-warning "final would enable devirtualization of 5 calls" }
+struct foo
{
static int count;
void print (int i, int j) { printf ("foo[%d][%d] = %d\n", i, j, x); }
--
2.34.1
