* PR C++/88114 - patch for destructor not generated for "virtual ~destructor() = default"
@ 2018-11-21 12:19 Tobias Burnus
2018-11-25 8:44 ` *ping* – PR C++/88114 - PATCH " Tobias Burnus
2018-12-05 22:50 ` PR C++/88114 - patch " Jason Merrill
0 siblings, 2 replies; 7+ messages in thread
From: Tobias Burnus @ 2018-11-21 12:19 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 1946 bytes --]
Hello all,
if a class contains any 'virtual ... = 0', it's an abstract class and for an
abstract class, the destructor not added to the vtable.
For a normal
virtual ~class() { }
that's not a problem as the class::~class() destructor will be generated during
the parsing of the function.
But for
virtual ~class() = default;
the destructor will be generated via mark_used via the vtable.
If one now declares a derived class and uses it, the class::~class() is generated
in that translation unit. Unless, #pragma interface/implementation is used.
In that case, the 'default' destructor will never be generated.
The following code seems to work both for the big code and for the example;
without '#pragma implementation', the destructor is not generated for the example,
only with.
The patch survived boostrapping GCC with default languages on x86-64-gnu-linux
and "make check-g++".*
[One probably could get rid of some of the conditions for generating the code,
e.g. TREE_USED and DECL_DEFAULTED_FN are probably not both needed; one might
want to set some additional DECL to the fn decl.]
Does the patch and the test case make sense? Or is something else/in addition
needed?
Tobias
*I do get the following failures on this CentOS6 system:
FAIL: g++.dg/pr83239.C -std=gnu++98 (test for excess errors)
Excess errors:
cc1plus: warning: 'void* __builtin_memset(void*, int, long unsigned int)' specified size 18446744073709551608 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
cc1plus: warning: 'void* __builtin_memset(void*, int, long unsigned int)' specified size 18446744073709551600 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
FAIL: g++.dg/tls/thread_local-order2.C -std=c++14 execution test
FAIL: g++.dg/tls/thread_local-order2.C -std=c++17 execution test
plus each 32 times:
FAIL: guality/guality.h: 0 PASS, 1 FAIL, 0 UNRESOLVED
FAIL: guality/guality.h: varl is -1, not 6
[-- Attachment #2: pr88114-destructor-default.diff --]
[-- Type: text/x-diff, Size: 2363 bytes --]
PR C++/88114
* decl2.c (c_parse_final_cleanups): If needed, generate code for
the destructor of an abstract class.
(mark_used): Update comment for older function-name change.
PR C++/88114
* g++.dg/cpp0x/defaulted61.C: New.
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index ffc0d0d6ec4..056e49ad88a 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -4782,6 +4782,18 @@ c_parse_final_cleanups (void)
{
reconsider = true;
keyed_classes->unordered_remove (i);
+
+ /* For abstract classes, the destructor has been removed from the
+ vtable (in class.c's build_vtbl_initializer). For a compiler-
+ generated destructor, it hence might not have been generated in
+ this translation unit - and with '#pragma interface' it might
+ never get generated. */
+ if (CLASSTYPE_PURE_VIRTUALS (t)
+ && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (t))
+ for (tree x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
+ if (DECL_DECLARES_FUNCTION_P (x) && DECL_DESTRUCTOR_P (x)
+ && !TREE_USED (x) && DECL_DEFAULTED_FN (x))
+ note_vague_linkage_fn (x);
}
/* The input_location may have been changed during marking of
vtable entries. */
@@ -5465,7 +5477,7 @@ mark_used (tree decl, tsubst_flags_t complain)
within the body of a function so as to avoid collecting live data
on the stack (such as overload resolution candidates).
- We could just let cp_write_global_declarations handle synthesizing
+ We could just let c_parse_final_cleanups handle synthesizing
this function by adding it to deferred_fns, but doing
it at the use site produces better error messages. */
++function_depth;
diff --git a/gcc/testsuite/g++.dg/cpp0x/defaulted61.C b/gcc/testsuite/g++.dg/cpp0x/defaulted61.C
new file mode 100644
index 00000000000..e7e0a486292
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/defaulted61.C
@@ -0,0 +1,22 @@
+// { dg-do compile { target c++11 } }
+// { dg-final { scan-assembler "_ZN3OneD0Ev" } }
+
+// PR C++/88114
+// Destructor of an abstract class was never generated
+// when compiling the class - nor later due to the
+// '#pragma interface'
+
+#pragma implementation
+#pragma interface
+
+class One
+{
+ public:
+ virtual ~One() = default;
+ void some_fn();
+ virtual void later() = 0;
+ private:
+ int m_int;
+};
+
+void One::some_fn() { }
^ permalink raw reply [flat|nested] 7+ messages in thread
* *ping* – PR C++/88114 - PATCH for destructor not generated for "virtual ~destructor() = default"
2018-11-21 12:19 PR C++/88114 - patch for destructor not generated for "virtual ~destructor() = default" Tobias Burnus
@ 2018-11-25 8:44 ` Tobias Burnus
2018-11-28 20:36 ` *ping*^2 " Tobias Burnus
2018-12-05 22:50 ` PR C++/88114 - patch " Jason Merrill
1 sibling, 1 reply; 7+ messages in thread
From: Tobias Burnus @ 2018-11-25 8:44 UTC (permalink / raw)
To: Tobias Burnus, gcc-patches
On 21 November 2018, Tobias Burnus wrote:
> Hello all,
>
> if a class contains any 'virtual ... = 0', it's an abstract class and for an
> abstract class, the destructor not added to the vtable.
>
> For a normal
> virtual ~class() { }
> that's not a problem as the class::~class() destructor will be generated during
> the parsing of the function.
>
> But for
> virtual ~class() = default;
> the destructor will be generated via mark_used via the vtable.
>
>
> If one now declares a derived class and uses it, the class::~class() is generated
> in that translation unit. Unless, #pragma interface/implementation is used.
>
> In that case, the 'default' destructor will never be generated.
>
>
> The following code seems to work both for the big code and for the example;
> without '#pragma implementation', the destructor is not generated for the example,
> only with.
>
> The patch survived boostrapping GCC with default languages on x86-64-gnu-linux
> and "make check-g++".*
>
> [One probably could get rid of some of the conditions for generating the code,
> e.g. TREE_USED and DECL_DEFAULTED_FN are probably not both needed; one might
> want to set some additional DECL to the fn decl.]
>
> Does the patch and the test case make sense? Or is something else/in addition
> needed?
>
> Tobias
>
>
> *I do get the following failures on this CentOS6 system:
>
> FAIL: g++.dg/pr83239.C -std=gnu++98 (test for excess errors)
> Excess errors:
> cc1plus: warning: 'void* __builtin_memset(void*, int, long unsigned int)' specified size 18446744073709551608 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
> cc1plus: warning: 'void* __builtin_memset(void*, int, long unsigned int)' specified size 18446744073709551600 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
>
> FAIL: g++.dg/tls/thread_local-order2.C -std=c++14 execution test
> FAIL: g++.dg/tls/thread_local-order2.C -std=c++17 execution test
>
> plus each 32 times:
> FAIL: guality/guality.h: 0 PASS, 1 FAIL, 0 UNRESOLVED
> FAIL: guality/guality.h: varl is -1, not 6
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: *ping*^2 – PR C++/88114 - PATCH for destructor not generated for "virtual ~destructor() = default"
2018-11-25 8:44 ` *ping* – PR C++/88114 - PATCH " Tobias Burnus
@ 2018-11-28 20:36 ` Tobias Burnus
2018-12-04 12:42 ` *PING*^3 [C++ Patch] " Tobias Burnus
0 siblings, 1 reply; 7+ messages in thread
From: Tobias Burnus @ 2018-11-28 20:36 UTC (permalink / raw)
To: Tobias Burnus, gcc-patches
On the 25th November 2018, schrieb Tobias Burnus wrote:
> On 21 November 2018, Tobias Burnus wrote:
>> Hello all,
>>
>> if a class contains any 'virtual ... = 0', it's an abstract class and
>> for an
>> abstract class, the destructor not added to the vtable.
>>
>> For a normal
>> Â Â virtual ~class() { }
>> that's not a problem as the class::~class() destructor will be
>> generated during
>> the parsing of the function.
>>
>> But for
>> Â Â virtual ~class() = default;
>> the destructor will be generated via mark_used via the vtable.
>>
>>
>> If one now declares a derived class and uses it, the class::~class()
>> is generated
>> in that translation unit. Unless, #pragma interface/implementation
>> is used.
>>
>> In that case, the 'default' destructor will never be generated.
>>
>>
>> The following code seems to work both for the big code and for the
>> example;
>> without '#pragma implementation', the destructor is not generated for
>> the example,
>> only with.
>>
>> The patch survived boostrapping GCC with default languages on
>> x86-64-gnu-linux
>> and "make check-g++".*
>>
>> [One probably could get rid of some of the conditions for generating
>> the code,
>> e.g. TREE_USED and DECL_DEFAULTED_FN are probably not both needed;
>> one might
>> want to set some additional DECL to the fn decl.]
>>
>> Does the patch and the test case make sense? Or is something else/in
>> addition
>> needed?
>>
>> Tobias
>>
>>
>> *I do get the following failures on this CentOS6 system:
>>
>> FAIL: g++.dg/pr83239.CÂ -std=gnu++98 (test for excess errors)
>> Excess errors:
>> cc1plus: warning: 'void* __builtin_memset(void*, int, long unsigned
>> int)' specified size 18446744073709551608 exceeds maximum object size
>> 9223372036854775807 [-Wstringop-overflow=]
>> cc1plus: warning: 'void* __builtin_memset(void*, int, long unsigned
>> int)' specified size 18446744073709551600 exceeds maximum object size
>> 9223372036854775807 [-Wstringop-overflow=]
>>
>> FAIL: g++.dg/tls/thread_local-order2.CÂ -std=c++14 execution test
>> FAIL: g++.dg/tls/thread_local-order2.CÂ -std=c++17 execution test
>>
>> plus each 32 times:
>> FAIL: guality/guality.h: 0 PASS, 1 FAIL, 0 UNRESOLVED
>> FAIL: guality/guality.h: varl is -1, not 6
^ permalink raw reply [flat|nested] 7+ messages in thread
* *PING*^3 [C++ Patch] – PR C++/88114 - PATCH for destructor not generated for "virtual ~destructor() = default"
2018-11-28 20:36 ` *ping*^2 " Tobias Burnus
@ 2018-12-04 12:42 ` Tobias Burnus
0 siblings, 0 replies; 7+ messages in thread
From: Tobias Burnus @ 2018-12-04 12:42 UTC (permalink / raw)
To: Tobias Burnus; +Cc: gcc-patches
On Wed, Nov 28, 2018 at 09:35:53PM +0100, Tobias Burnus wrote:
> On the 25th November 2018, schrieb Tobias Burnus wrote:
> > On 21 November 2018, Tobias Burnus wrote:
> > > Hello all,
> > >
> > > if a class contains any 'virtual ... = 0', it's an abstract class
> > > and for an
> > > abstract class, the destructor not added to the vtable.
> > >
> > > For a normal
> > > Â Â virtual ~class() { }
> > > that's not a problem as the class::~class() destructor will be
> > > generated during
> > > the parsing of the function.
> > >
> > > But for
> > > Â Â virtual ~class() = default;
> > > the destructor will be generated via mark_used via the vtable.
> > >
> > >
> > > If one now declares a derived class and uses it, the class::~class()
> > > is generated
> > > in that translation unit. Unless, #pragma interface/implementation
> > > is used.
> > >
> > > In that case, the 'default' destructor will never be generated.
> > >
> > >
> > > The following code seems to work both for the big code and for the
> > > example;
> > > without '#pragma implementation', the destructor is not generated
> > > for the example,
> > > only with.
> > >
> > > The patch survived boostrapping GCC with default languages on
> > > x86-64-gnu-linux
> > > and "make check-g++".*
> > >
> > > [One probably could get rid of some of the conditions for generating
> > > the code,
> > > e.g. TREE_USED and DECL_DEFAULTED_FN are probably not both needed;
> > > one might
> > > want to set some additional DECL to the fn decl.]
> > >
> > > Does the patch and the test case make sense? Or is something else/in
> > > addition
> > > needed?
> > >
> > > Tobias
> > >
> > >
> > > *I do get the following failures on this CentOS6 system:
> > >
> > > FAIL: g++.dg/pr83239.CÂ -std=gnu++98 (test for excess errors)
> > > Excess errors:
> > > cc1plus: warning: 'void* __builtin_memset(void*, int, long unsigned
> > > int)' specified size 18446744073709551608 exceeds maximum object
> > > size 9223372036854775807 [-Wstringop-overflow=]
> > > cc1plus: warning: 'void* __builtin_memset(void*, int, long unsigned
> > > int)' specified size 18446744073709551600 exceeds maximum object
> > > size 9223372036854775807 [-Wstringop-overflow=]
> > >
> > > FAIL: g++.dg/tls/thread_local-order2.CÂ -std=c++14 execution test
> > > FAIL: g++.dg/tls/thread_local-order2.CÂ -std=c++17 execution test
> > >
> > > plus each 32 times:
> > > FAIL: guality/guality.h: 0 PASS, 1 FAIL, 0 UNRESOLVED
> > > FAIL: guality/guality.h: varl is -1, not 6
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PR C++/88114 - patch for destructor not generated for "virtual ~destructor() = default"
2018-11-21 12:19 PR C++/88114 - patch for destructor not generated for "virtual ~destructor() = default" Tobias Burnus
2018-11-25 8:44 ` *ping* – PR C++/88114 - PATCH " Tobias Burnus
@ 2018-12-05 22:50 ` Jason Merrill
2019-01-11 18:36 ` Tobias Burnus
1 sibling, 1 reply; 7+ messages in thread
From: Jason Merrill @ 2018-12-05 22:50 UTC (permalink / raw)
To: Tobias Burnus, gcc-patches
On 11/21/18 7:19 AM, Tobias Burnus wrote:
> Hello all,
>
> if a class contains any 'virtual ... = 0', it's an abstract class and for an
> abstract class, the destructor not added to the vtable.
>
> For a normal
> virtual ~class() { }
> that's not a problem as the class::~class() destructor will be generated during
> the parsing of the function.
>
> But for
> virtual ~class() = default;
> the destructor will be generated via mark_used via the vtable.
>
>
> If one now declares a derived class and uses it, the class::~class() is generated
> in that translation unit. Unless, #pragma interface/implementation is used.
>
> In that case, the 'default' destructor will never be generated.
>
>
> The following code seems to work both for the big code and for the example;
> without '#pragma implementation', the destructor is not generated for the example,
> only with.
>
> The patch survived boostrapping GCC with default languages on x86-64-gnu-linux
> and "make check-g++".*
>
> [One probably could get rid of some of the conditions for generating the code,
> e.g. TREE_USED and DECL_DEFAULTED_FN are probably not both needed; one might
> want to set some additional DECL to the fn decl.]
You can get at the destructor with CLASSTYPE_DESTRUCTOR rather than
walking through TYPE_FIELDS. I'd also check DECL_DEFAULTED_IN_CLASS_P.
I'd also do this in maybe_emit_vtables rather than here, so that it only
happens once per class.
Jason
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PR C++/88114 - patch for destructor not generated for "virtual ~destructor() = default"
2018-12-05 22:50 ` PR C++/88114 - patch " Jason Merrill
@ 2019-01-11 18:36 ` Tobias Burnus
2019-01-11 18:57 ` Jason Merrill
0 siblings, 1 reply; 7+ messages in thread
From: Tobias Burnus @ 2019-01-11 18:36 UTC (permalink / raw)
To: gcc-patches, Jason Merrill
[-- Attachment #1: Type: text/plain, Size: 1223 bytes --]
Dear Jason, dear all,
Jason Merrill wrote on 5 Dec 2018:
> You can get at the destructor with CLASSTYPE_DESTRUCTOR rather than walking through TYPE_FIELDS. I'd also check DECL_DEFAULTED_IN_CLASS_P.
> I'd also do this in maybe_emit_vtables rather than here, so that it only happens once per class.
Updated patch. I also added a test case which checks that the destructor
is not generated.
[ Original patch: https://gcc.gnu.org/ml/gcc-patches/2018-11/msg01824.html ]
Background again:
If a class contains any 'virtual ... = 0', it's an abstract class and for an
abstract class, the destructor not added to the vtable.
For a normal
virtual ~class() { }
that's not a problem as the class::~class() destructor will be generated during
the parsing of the function.
But for
virtual ~class() = default;
the destructor will be generated via mark_used via the vtable.
If one now declares a derived class and uses it, the class::~class() is generated
in that translation unit. Unless, #pragma interface/implementation is used.
In that case, the 'default' destructor will never be generated.
Build and regtested on x86_64-gnu-linux.
OK? Or do you have more suggested changes?
Tobias
[-- Attachment #2: pr88114-destructor-default-v2.diff --]
[-- Type: text/x-diff, Size: 3190 bytes --]
2019-01-11 Tobias Burnus <burnus@net-b.de>
PR C++/88114
* decl2.c (maybe_emit_vtables): If needed, generate code for
the destructor of an abstract class.
(mark_used): Update comment for older function-name change.
PR C++/88114
* g++.dg/cpp0x/defaulted61.C: New.
* g++.dg/cpp0x/defaulted62.C: New.
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index dbab95fbc96..2e7ecdaa01c 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -2220,6 +2220,17 @@ maybe_emit_vtables (tree ctype)
}
}
+ /* For abstract classes, the destructor has been removed from the
+ vtable (in class.c's build_vtbl_initializer). For a compiler-
+ generated destructor, it hence might not have been generated in
+ this translation unit - and with '#pragma interface' it might
+ never get generated. */
+ if (CLASSTYPE_PURE_VIRTUALS (ctype)
+ && TYPE_HAS_NONTRIVIAL_DESTRUCTOR (ctype)
+ && DECL_DEFAULTED_IN_CLASS_P(CLASSTYPE_DESTRUCTOR(ctype))
+ && DECL_DEFAULTED_FN (CLASSTYPE_DESTRUCTOR(ctype)))
+ note_vague_linkage_fn (CLASSTYPE_DESTRUCTOR(ctype));
+
/* Since we're writing out the vtable here, also write the debug
info. */
note_debug_info_needed (ctype);
@@ -5497,7 +5508,7 @@ mark_used (tree decl, tsubst_flags_t complain)
within the body of a function so as to avoid collecting live data
on the stack (such as overload resolution candidates).
- We could just let cp_write_global_declarations handle synthesizing
+ We could just let c_parse_final_cleanups handle synthesizing
this function by adding it to deferred_fns, but doing
it at the use site produces better error messages. */
++function_depth;
diff --git a/gcc/testsuite/g++.dg/cpp0x/defaulted61.C b/gcc/testsuite/g++.dg/cpp0x/defaulted61.C
new file mode 100644
index 00000000000..e7e0a486292
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/defaulted61.C
@@ -0,0 +1,22 @@
+// { dg-do compile { target c++11 } }
+// { dg-final { scan-assembler "_ZN3OneD0Ev" } }
+
+// PR C++/88114
+// Destructor of an abstract class was never generated
+// when compiling the class - nor later due to the
+// '#pragma interface'
+
+#pragma implementation
+#pragma interface
+
+class One
+{
+ public:
+ virtual ~One() = default;
+ void some_fn();
+ virtual void later() = 0;
+ private:
+ int m_int;
+};
+
+void One::some_fn() { }
diff --git a/gcc/testsuite/g++.dg/cpp0x/defaulted62.C b/gcc/testsuite/g++.dg/cpp0x/defaulted62.C
new file mode 100644
index 00000000000..d8dab608816
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/defaulted62.C
@@ -0,0 +1,25 @@
+// { dg-do compile { target c++11 } }
+// { dg-final { scan-assembler-not "_ZN3OneD0Ev" } }
+
+// PR C++/88114
+// Destructor of an abstract class was never generated
+// when compiling the class - nor later due to the
+// '#pragma interface'
+// -> g++.dg/cpp0x/defaulted61.C
+
+// HERE, in g++.dg/cpp0x/defaulted62.C:
+// As we have commented the pragmas, it should NOT be created
+// #pragma implementation
+// #pragma interface
+
+class One
+{
+ public:
+ virtual ~One() = default;
+ void some_fn();
+ virtual void later() = 0;
+ private:
+ int m_int;
+};
+
+void One::some_fn() { }
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: PR C++/88114 - patch for destructor not generated for "virtual ~destructor() = default"
2019-01-11 18:36 ` Tobias Burnus
@ 2019-01-11 18:57 ` Jason Merrill
0 siblings, 0 replies; 7+ messages in thread
From: Jason Merrill @ 2019-01-11 18:57 UTC (permalink / raw)
To: Tobias Burnus, gcc-patches
On 1/11/19 1:36 PM, Tobias Burnus wrote:
> Dear Jason, dear all,
>
> Jason Merrill wrote on 5 Dec 2018:
>> You can get at the destructor with CLASSTYPE_DESTRUCTOR rather than walking through TYPE_FIELDS. I'd also check DECL_DEFAULTED_IN_CLASS_P.
>> I'd also do this in maybe_emit_vtables rather than here, so that it only happens once per class.
>
> Updated patch. I also added a test case which checks that the destructor
> is not generated.
>
> [ Original patch: https://gcc.gnu.org/ml/gcc-patches/2018-11/msg01824.html ]
>
> Background again:
> If a class contains any 'virtual ... = 0', it's an abstract class and for an
> abstract class, the destructor not added to the vtable.
>
> For a normal
> virtual ~class() { }
> that's not a problem as the class::~class() destructor will be generated during
> the parsing of the function.
>
> But for
> virtual ~class() = default;
> the destructor will be generated via mark_used via the vtable.
>
> If one now declares a derived class and uses it, the class::~class() is generated
> in that translation unit. Unless, #pragma interface/implementation is used.
>
> In that case, the 'default' destructor will never be generated.
>
> Build and regtested on x86_64-gnu-linux.
> OK? Or do you have more suggested changes?
> + && DECL_DEFAULTED_IN_CLASS_P(CLASSTYPE_DESTRUCTOR(ctype))
> + && DECL_DEFAULTED_FN (CLASSTYPE_DESTRUCTOR(ctype)))
Checking DECL_DEFAULTED_FN again is redundant, it's checked by
DECL_DEFAULTED_IN_CLASS_P. OK with that last condition removed.
Jason
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-01-11 18:57 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-21 12:19 PR C++/88114 - patch for destructor not generated for "virtual ~destructor() = default" Tobias Burnus
2018-11-25 8:44 ` *ping* – PR C++/88114 - PATCH " Tobias Burnus
2018-11-28 20:36 ` *ping*^2 " Tobias Burnus
2018-12-04 12:42 ` *PING*^3 [C++ Patch] " Tobias Burnus
2018-12-05 22:50 ` PR C++/88114 - patch " Jason Merrill
2019-01-11 18:36 ` Tobias Burnus
2019-01-11 18:57 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).