* [PATCH] c++: Diagnose or avoid constexpr dtors in classes with virtual bases [PR114426]
@ 2024-04-12 13:12 Jakub Jelinek
2024-04-12 17:45 ` Jason Merrill
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2024-04-12 13:12 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
Hi!
I had another look at this P1 PR today.
You said in the "c++: fix in-charge parm in constexpr" mail back in December
(as well as in the r14-6507 commit message):
"Since a class with vbases can't have constexpr 'tors there isn't actually
a need for an in-charge parameter in a destructor" but the ICE is because
the destructor is marked implicitly constexpr.
https://eel.is/c++draft/dcl.constexpr#3.2 says that a destructor of a class
with virtual bases is not constexpr-suitable, but we were actually
implementing this just for constructors, so clearly my fault from the
https://wg21.link/P0784R7 implementation. That paper clearly added that
sentence in there and removed similar sentence just from the constructor case.
So, the following patch makes sure the
else if (CLASSTYPE_VBASECLASSES (DECL_CONTEXT (fun)))
{
ret = false;
if (complain)
error ("%q#T has virtual base classes", DECL_CONTEXT (fun));
}
hunk is done no just for DECL_CONSTRUCTOR_P (fun), but also
DECL_DESTRUCTOR_P (fun) - in that case just for cxx_dialect >= cxx20,
as for cxx_dialect < cxx20 we already set ret = false; and diagnose
a different error, so no need to diagnose two.
Bootstrapped/regtested on x86_64-linux and i686-linux, and checked it fixes
the testcase in a cross to armv7hl-linux-gnueabi, ok for trunk?
2024-04-12 Jakub Jelinek <jakub@redhat.com>
PR c++/114426
* constexpr.cc (is_valid_constexpr_fn): Return false/diagnose with
complain destructors in classes with virtual bases.
* g++.dg/cpp2a/pr114426.C: New test.
* g++.dg/cpp2a/constexpr-dtor16.C: New test.
--- gcc/cp/constexpr.cc.jj 2024-04-09 09:29:04.708521907 +0200
+++ gcc/cp/constexpr.cc 2024-04-12 11:45:08.845476718 +0200
@@ -262,18 +262,15 @@ is_valid_constexpr_fn (tree fun, bool co
inform (DECL_SOURCE_LOCATION (fun),
"lambdas are implicitly %<constexpr%> only in C++17 and later");
}
- else if (DECL_DESTRUCTOR_P (fun))
+ else if (DECL_DESTRUCTOR_P (fun) && cxx_dialect < cxx20)
{
- if (cxx_dialect < cxx20)
- {
- ret = false;
- if (complain)
- error_at (DECL_SOURCE_LOCATION (fun),
- "%<constexpr%> destructors only available"
- " with %<-std=c++20%> or %<-std=gnu++20%>");
- }
+ ret = false;
+ if (complain)
+ error_at (DECL_SOURCE_LOCATION (fun),
+ "%<constexpr%> destructors only available with "
+ "%<-std=c++20%> or %<-std=gnu++20%>");
}
- else if (!DECL_CONSTRUCTOR_P (fun))
+ else if (!DECL_CONSTRUCTOR_P (fun) && !DECL_DESTRUCTOR_P (fun))
{
tree rettype = TREE_TYPE (TREE_TYPE (fun));
if (!literal_type_p (rettype))
--- gcc/testsuite/g++.dg/cpp2a/pr114426.C.jj 2024-04-12 12:05:07.443891700 +0200
+++ gcc/testsuite/g++.dg/cpp2a/pr114426.C 2024-04-12 12:05:07.443891700 +0200
@@ -0,0 +1,7 @@
+// PR c++/114426
+// { dg-do compile }
+// { dg-additional-options "-O2" }
+
+struct A { virtual ~A (); };
+struct B : virtual A { virtual void foo () = 0; };
+struct C : B { C () {} };
--- gcc/testsuite/g++.dg/cpp2a/constexpr-dtor16.C.jj 2024-04-12 12:05:35.398505976 +0200
+++ gcc/testsuite/g++.dg/cpp2a/constexpr-dtor16.C 2024-04-12 12:08:31.771072322 +0200
@@ -0,0 +1,7 @@
+// PR c++/114426
+// { dg-do compile { target c++11 } }
+
+struct A { virtual ~A (); };
+struct B : virtual A { constexpr ~B () {} };
+// { dg-error "'struct B' has virtual base classes" "" { target c++20 } .-1 }
+// { dg-error "'constexpr' destructors only available with" "" { target c++17_down } .-2 }
Jakub
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] c++: Diagnose or avoid constexpr dtors in classes with virtual bases [PR114426]
2024-04-12 13:12 [PATCH] c++: Diagnose or avoid constexpr dtors in classes with virtual bases [PR114426] Jakub Jelinek
@ 2024-04-12 17:45 ` Jason Merrill
0 siblings, 0 replies; 2+ messages in thread
From: Jason Merrill @ 2024-04-12 17:45 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On 4/12/24 09:12, Jakub Jelinek wrote:
> Hi!
>
> I had another look at this P1 PR today.
> You said in the "c++: fix in-charge parm in constexpr" mail back in December
> (as well as in the r14-6507 commit message):
> "Since a class with vbases can't have constexpr 'tors there isn't actually
> a need for an in-charge parameter in a destructor" but the ICE is because
> the destructor is marked implicitly constexpr.
> https://eel.is/c++draft/dcl.constexpr#3.2 says that a destructor of a class
> with virtual bases is not constexpr-suitable, but we were actually
> implementing this just for constructors, so clearly my fault from the
> https://wg21.link/P0784R7 implementation. That paper clearly added that
> sentence in there and removed similar sentence just from the constructor case.
>
> So, the following patch makes sure the
> else if (CLASSTYPE_VBASECLASSES (DECL_CONTEXT (fun)))
> {
> ret = false;
> if (complain)
> error ("%q#T has virtual base classes", DECL_CONTEXT (fun));
> }
> hunk is done no just for DECL_CONSTRUCTOR_P (fun), but also
> DECL_DESTRUCTOR_P (fun) - in that case just for cxx_dialect >= cxx20,
> as for cxx_dialect < cxx20 we already set ret = false; and diagnose
> a different error, so no need to diagnose two.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, and checked it fixes
> the testcase in a cross to armv7hl-linux-gnueabi, ok for trunk?
OK.
> 2024-04-12 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/114426
> * constexpr.cc (is_valid_constexpr_fn): Return false/diagnose with
> complain destructors in classes with virtual bases.
>
> * g++.dg/cpp2a/pr114426.C: New test.
> * g++.dg/cpp2a/constexpr-dtor16.C: New test.
>
> --- gcc/cp/constexpr.cc.jj 2024-04-09 09:29:04.708521907 +0200
> +++ gcc/cp/constexpr.cc 2024-04-12 11:45:08.845476718 +0200
> @@ -262,18 +262,15 @@ is_valid_constexpr_fn (tree fun, bool co
> inform (DECL_SOURCE_LOCATION (fun),
> "lambdas are implicitly %<constexpr%> only in C++17 and later");
> }
> - else if (DECL_DESTRUCTOR_P (fun))
> + else if (DECL_DESTRUCTOR_P (fun) && cxx_dialect < cxx20)
> {
> - if (cxx_dialect < cxx20)
> - {
> - ret = false;
> - if (complain)
> - error_at (DECL_SOURCE_LOCATION (fun),
> - "%<constexpr%> destructors only available"
> - " with %<-std=c++20%> or %<-std=gnu++20%>");
> - }
> + ret = false;
> + if (complain)
> + error_at (DECL_SOURCE_LOCATION (fun),
> + "%<constexpr%> destructors only available with "
> + "%<-std=c++20%> or %<-std=gnu++20%>");
> }
> - else if (!DECL_CONSTRUCTOR_P (fun))
> + else if (!DECL_CONSTRUCTOR_P (fun) && !DECL_DESTRUCTOR_P (fun))
> {
> tree rettype = TREE_TYPE (TREE_TYPE (fun));
> if (!literal_type_p (rettype))
> --- gcc/testsuite/g++.dg/cpp2a/pr114426.C.jj 2024-04-12 12:05:07.443891700 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/pr114426.C 2024-04-12 12:05:07.443891700 +0200
> @@ -0,0 +1,7 @@
> +// PR c++/114426
> +// { dg-do compile }
> +// { dg-additional-options "-O2" }
> +
> +struct A { virtual ~A (); };
> +struct B : virtual A { virtual void foo () = 0; };
> +struct C : B { C () {} };
> --- gcc/testsuite/g++.dg/cpp2a/constexpr-dtor16.C.jj 2024-04-12 12:05:35.398505976 +0200
> +++ gcc/testsuite/g++.dg/cpp2a/constexpr-dtor16.C 2024-04-12 12:08:31.771072322 +0200
> @@ -0,0 +1,7 @@
> +// PR c++/114426
> +// { dg-do compile { target c++11 } }
> +
> +struct A { virtual ~A (); };
> +struct B : virtual A { constexpr ~B () {} };
> +// { dg-error "'struct B' has virtual base classes" "" { target c++20 } .-1 }
> +// { dg-error "'constexpr' destructors only available with" "" { target c++17_down } .-2 }
>
> Jakub
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-04-12 17:45 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-12 13:12 [PATCH] c++: Diagnose or avoid constexpr dtors in classes with virtual bases [PR114426] Jakub Jelinek
2024-04-12 17:45 ` 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).