From: Tobias Burnus <tobias.burnus@physik.fu-berlin.de>
To: gcc-patches@gcc.gnu.org
Subject: PR C++/88114 - patch for destructor not generated for "virtual ~destructor() = default"
Date: Wed, 21 Nov 2018 12:19:00 -0000 [thread overview]
Message-ID: <20181121121935.tm4y2wyie6bc72sn@physik.fu-berlin.de> (raw)
[-- 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() { }
next reply other threads:[~2018-11-21 12:19 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-21 12:19 Tobias Burnus [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181121121935.tm4y2wyie6bc72sn@physik.fu-berlin.de \
--to=tobias.burnus@physik.fu-berlin.de \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).