* [PATCH] C++ concepts: fix ICE with requires on dtors (PR c++/89036)
@ 2019-01-25 16:07 David Malcolm
2019-01-25 17:10 ` Nathan Sidwell
0 siblings, 1 reply; 6+ messages in thread
From: David Malcolm @ 2019-01-25 16:07 UTC (permalink / raw)
To: jason, Nathan Sidwell, gcc-patches; +Cc: David Malcolm
PR c++/89036 reports an ICE due to this assertion failing
1136 /* A class should never have more than one destructor. */
1137 gcc_assert (!current_fns || via_using || !DECL_DESTRUCTOR_P (method));
on this template with a pair of dtors, with
mutually exclusive "requires" clauses:
template<typename T>
struct Y {
~Y() requires(true) = default;
~Y() requires(false) {}
};
(is this valid? my knowledge of this part of C++ is fairly hazy)
Nathan introduced this assertion as part of:
ca9219bf18c68a001d62ecb981bc9176b0feaf12 (aka r251340):
2017-08-24 Nathan Sidwell <nathan@acm.org>
Conversion operators kept on single overload set
which, amongst other changes to add_method had this:
/* A class should never have more than one destructor. */
- if (current_fns && DECL_MAYBE_IN_CHARGE_DESTRUCTOR_P (method))
- return false;
+ gcc_assert (!current_fns || !DECL_DESTRUCTOR_P (method));
The following patch generalizes the assertion to allow for multiple
dtors if they have "requires" clauses. (I already generalized
the assertion in another way in r268041 to fix PR c++/88699;
alternatively, is this too much like whack-a-mole, and would
dropping the assertion altogether be better?)
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/cp/ChangeLog:
PR c++/89036
* class.c (add_method): Generalize assertion to allow for
multiple dtors if they have "requires" clauses.
gcc/testsuite/ChangeLog:
PR c++/89036
* g++.dg/concepts/pr89036.C: New test.
---
gcc/cp/class.c | 11 +++++++++--
gcc/testsuite/g++.dg/concepts/pr89036.C | 8 ++++++++
2 files changed, 17 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/concepts/pr89036.C
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index e8773c2..fb46f92 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -1133,8 +1133,15 @@ add_method (tree type, tree method, bool via_using)
}
}
- /* A class should never have more than one destructor. */
- gcc_assert (!current_fns || via_using || !DECL_DESTRUCTOR_P (method));
+ /* A class should never have more than one destructor... */
+ gcc_assert (!current_fns
+ || via_using
+ || !DECL_DESTRUCTOR_P (method)
+ /* ...unless the destructors are constrained by "requires"
+ clauses. */
+ || (flag_concepts
+ && get_constraints (method)
+ && CI_DECLARATOR_REQS (get_constraints (method))));
current_fns = ovl_insert (method, current_fns, via_using);
diff --git a/gcc/testsuite/g++.dg/concepts/pr89036.C b/gcc/testsuite/g++.dg/concepts/pr89036.C
new file mode 100644
index 0000000..f83ef8b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr89036.C
@@ -0,0 +1,8 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-fconcepts" }
+
+template<typename T>
+struct Y {
+ ~Y() requires(true) = default;
+ ~Y() requires(false) {}
+};
--
1.8.5.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] C++ concepts: fix ICE with requires on dtors (PR c++/89036)
2019-01-25 16:07 [PATCH] C++ concepts: fix ICE with requires on dtors (PR c++/89036) David Malcolm
@ 2019-01-25 17:10 ` Nathan Sidwell
2019-01-25 21:20 ` [PATCH v2] " David Malcolm
0 siblings, 1 reply; 6+ messages in thread
From: Nathan Sidwell @ 2019-01-25 17:10 UTC (permalink / raw)
To: gcc-patches
On 1/25/19 8:48 AM, David Malcolm wrote:
> PR c++/89036 reports an ICE due to this assertion failing
>
> 1136 /* A class should never have more than one destructor. */
> 1137 gcc_assert (!current_fns || via_using || !DECL_DESTRUCTOR_P (method));
>
> on this template with a pair of dtors, with
> mutually exclusive "requires" clauses:
>
> template<typename T>
> struct Y {
> ~Y() requires(true) = default;
> ~Y() requires(false) {}
> };
>
> (is this valid? my knowledge of this part of C++ is fairly hazy)
Yes. A more sensible example would have 'true' and 'false' replaced by
something determined from the template parms.
>
> Nathan introduced this assertion as part of:
>
> ca9219bf18c68a001d62ecb981bc9176b0feaf12 (aka r251340):
> 2017-08-24 Nathan Sidwell <nathan@acm.org>
> Conversion operators kept on single overload set
I'd just drop the assert at this point.
nathan
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] C++ concepts: fix ICE with requires on dtors (PR c++/89036)
2019-01-25 17:10 ` Nathan Sidwell
@ 2019-01-25 21:20 ` David Malcolm
2019-02-08 17:03 ` PING " David Malcolm
0 siblings, 1 reply; 6+ messages in thread
From: David Malcolm @ 2019-01-25 21:20 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: gcc-patches, David Malcolm
On Fri, 2019-01-25 at 08:59 -0800, Nathan Sidwell wrote:
> On 1/25/19 8:48 AM, David Malcolm wrote:
> > PR c++/89036 reports an ICE due to this assertion failing
> >
> > 1136 /* A class should never have more than one
> > destructor. */
> > 1137 gcc_assert (!current_fns || via_using ||
> > !DECL_DESTRUCTOR_P (method));
> >
> > on this template with a pair of dtors, with
> > mutually exclusive "requires" clauses:
> >
> > template<typename T>
> > struct Y {
> > ~Y() requires(true) = default;
> > ~Y() requires(false) {}
> > };
> >
> > (is this valid? my knowledge of this part of C++ is fairly hazy)
>
> Yes. A more sensible example would have 'true' and 'false' replaced
> by
> something determined from the template parms.
>
> >
> > Nathan introduced this assertion as part of:
> >
> > ca9219bf18c68a001d62ecb981bc9176b0feaf12 (aka r251340):
> > 2017-08-24 Nathan Sidwell <nathan@acm.org>
> > Conversion operators kept on single overload set
>
> I'd just drop the assert at this point.
>
> nathan
Thanks. Here's a version of the patch which drops the assertion.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/cp/ChangeLog:
PR c++/89036
* class.c (add_method): Drop destructor assertion.
gcc/testsuite/ChangeLog:
PR c++/89036
* g++.dg/concepts/pr89036.C: New test.
---
gcc/cp/class.c | 3 ---
gcc/testsuite/g++.dg/concepts/pr89036.C | 8 ++++++++
2 files changed, 8 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/concepts/pr89036.C
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index e8773c2..6e9dac9 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -1133,9 +1133,6 @@ add_method (tree type, tree method, bool via_using)
}
}
- /* A class should never have more than one destructor. */
- gcc_assert (!current_fns || via_using || !DECL_DESTRUCTOR_P (method));
-
current_fns = ovl_insert (method, current_fns, via_using);
if (!COMPLETE_TYPE_P (type) && !DECL_CONV_FN_P (method)
diff --git a/gcc/testsuite/g++.dg/concepts/pr89036.C b/gcc/testsuite/g++.dg/concepts/pr89036.C
new file mode 100644
index 0000000..f83ef8b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/concepts/pr89036.C
@@ -0,0 +1,8 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-fconcepts" }
+
+template<typename T>
+struct Y {
+ ~Y() requires(true) = default;
+ ~Y() requires(false) {}
+};
--
1.8.5.3
^ permalink raw reply [flat|nested] 6+ messages in thread
* PING Re: [PATCH v2] C++ concepts: fix ICE with requires on dtors (PR c++/89036)
2019-01-25 21:20 ` [PATCH v2] " David Malcolm
@ 2019-02-08 17:03 ` David Malcolm
2019-02-13 14:36 ` PING^2 " David Malcolm
0 siblings, 1 reply; 6+ messages in thread
From: David Malcolm @ 2019-02-08 17:03 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: gcc-patches
Ping
On Fri, 2019-01-25 at 15:02 -0500, David Malcolm wrote:
> On Fri, 2019-01-25 at 08:59 -0800, Nathan Sidwell wrote:
> > On 1/25/19 8:48 AM, David Malcolm wrote:
> > > PR c++/89036 reports an ICE due to this assertion failing
> > >
> > > 1136 /* A class should never have more than one
> > > destructor. */
> > > 1137 gcc_assert (!current_fns || via_using ||
> > > !DECL_DESTRUCTOR_P (method));
> > >
> > > on this template with a pair of dtors, with
> > > mutually exclusive "requires" clauses:
> > >
> > > template<typename T>
> > > struct Y {
> > > ~Y() requires(true) = default;
> > > ~Y() requires(false) {}
> > > };
> > >
> > > (is this valid? my knowledge of this part of C++ is fairly hazy)
> >
> > Yes. A more sensible example would have 'true' and 'false' replaced
> > by
> > something determined from the template parms.
> >
> > >
> > > Nathan introduced this assertion as part of:
> > >
> > > ca9219bf18c68a001d62ecb981bc9176b0feaf12 (aka r251340):
> > > 2017-08-24 Nathan Sidwell <nathan@acm.org>
> > > Conversion operators kept on single overload set
> >
> > I'd just drop the assert at this point.
> >
> > nathan
>
> Thanks. Here's a version of the patch which drops the assertion.
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> gcc/cp/ChangeLog:
> PR c++/89036
> * class.c (add_method): Drop destructor assertion.
>
> gcc/testsuite/ChangeLog:
> PR c++/89036
> * g++.dg/concepts/pr89036.C: New test.
> ---
> gcc/cp/class.c | 3 ---
> gcc/testsuite/g++.dg/concepts/pr89036.C | 8 ++++++++
> 2 files changed, 8 insertions(+), 3 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/concepts/pr89036.C
>
> diff --git a/gcc/cp/class.c b/gcc/cp/class.c
> index e8773c2..6e9dac9 100644
> --- a/gcc/cp/class.c
> +++ b/gcc/cp/class.c
> @@ -1133,9 +1133,6 @@ add_method (tree type, tree method, bool
> via_using)
> }
> }
>
> - /* A class should never have more than one destructor. */
> - gcc_assert (!current_fns || via_using || !DECL_DESTRUCTOR_P
> (method));
> -
> current_fns = ovl_insert (method, current_fns, via_using);
>
> if (!COMPLETE_TYPE_P (type) && !DECL_CONV_FN_P (method)
> diff --git a/gcc/testsuite/g++.dg/concepts/pr89036.C
> b/gcc/testsuite/g++.dg/concepts/pr89036.C
> new file mode 100644
> index 0000000..f83ef8b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/concepts/pr89036.C
> @@ -0,0 +1,8 @@
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-fconcepts" }
> +
> +template<typename T>
> +struct Y {
> + ~Y() requires(true) = default;
> + ~Y() requires(false) {}
> +};
^ permalink raw reply [flat|nested] 6+ messages in thread
* PING^2 Re: [PATCH v2] C++ concepts: fix ICE with requires on dtors (PR c++/89036)
2019-02-08 17:03 ` PING " David Malcolm
@ 2019-02-13 14:36 ` David Malcolm
2019-02-13 14:39 ` Nathan Sidwell
0 siblings, 1 reply; 6+ messages in thread
From: David Malcolm @ 2019-02-13 14:36 UTC (permalink / raw)
To: Nathan Sidwell; +Cc: gcc-patches
Ping
On Fri, 2019-02-08 at 12:03 -0500, David Malcolm wrote:
> Ping
>
> On Fri, 2019-01-25 at 15:02 -0500, David Malcolm wrote:
> > On Fri, 2019-01-25 at 08:59 -0800, Nathan Sidwell wrote:
> > > On 1/25/19 8:48 AM, David Malcolm wrote:
> > > > PR c++/89036 reports an ICE due to this assertion failing
> > > >
> > > > 1136 /* A class should never have more than one
> > > > destructor. */
> > > > 1137 gcc_assert (!current_fns || via_using ||
> > > > !DECL_DESTRUCTOR_P (method));
> > > >
> > > > on this template with a pair of dtors, with
> > > > mutually exclusive "requires" clauses:
> > > >
> > > > template<typename T>
> > > > struct Y {
> > > > ~Y() requires(true) = default;
> > > > ~Y() requires(false) {}
> > > > };
> > > >
> > > > (is this valid? my knowledge of this part of C++ is fairly
> > > > hazy)
> > >
> > > Yes. A more sensible example would have 'true' and 'false'
> > > replaced
> > > by
> > > something determined from the template parms.
> > >
> > > >
> > > > Nathan introduced this assertion as part of:
> > > >
> > > > ca9219bf18c68a001d62ecb981bc9176b0feaf12 (aka r251340):
> > > > 2017-08-24 Nathan Sidwell <nathan@acm.org>
> > > > Conversion operators kept on single overload set
> > >
> > > I'd just drop the assert at this point.
> > >
> > > nathan
> >
> > Thanks. Here's a version of the patch which drops the assertion.
> >
> > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> >
> > OK for trunk?
> >
> > gcc/cp/ChangeLog:
> > PR c++/89036
> > * class.c (add_method): Drop destructor assertion.
> >
> > gcc/testsuite/ChangeLog:
> > PR c++/89036
> > * g++.dg/concepts/pr89036.C: New test.
> > ---
> > gcc/cp/class.c | 3 ---
> > gcc/testsuite/g++.dg/concepts/pr89036.C | 8 ++++++++
> > 2 files changed, 8 insertions(+), 3 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/concepts/pr89036.C
> >
> > diff --git a/gcc/cp/class.c b/gcc/cp/class.c
> > index e8773c2..6e9dac9 100644
> > --- a/gcc/cp/class.c
> > +++ b/gcc/cp/class.c
> > @@ -1133,9 +1133,6 @@ add_method (tree type, tree method, bool
> > via_using)
> > }
> > }
> >
> > - /* A class should never have more than one destructor. */
> > - gcc_assert (!current_fns || via_using || !DECL_DESTRUCTOR_P
> > (method));
> > -
> > current_fns = ovl_insert (method, current_fns, via_using);
> >
> > if (!COMPLETE_TYPE_P (type) && !DECL_CONV_FN_P (method)
> > diff --git a/gcc/testsuite/g++.dg/concepts/pr89036.C
> > b/gcc/testsuite/g++.dg/concepts/pr89036.C
> > new file mode 100644
> > index 0000000..f83ef8b
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/concepts/pr89036.C
> > @@ -0,0 +1,8 @@
> > +// { dg-do compile { target c++11 } }
> > +// { dg-options "-fconcepts" }
> > +
> > +template<typename T>
> > +struct Y {
> > + ~Y() requires(true) = default;
> > + ~Y() requires(false) {}
> > +};
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: PING^2 Re: [PATCH v2] C++ concepts: fix ICE with requires on dtors (PR c++/89036)
2019-02-13 14:36 ` PING^2 " David Malcolm
@ 2019-02-13 14:39 ` Nathan Sidwell
0 siblings, 0 replies; 6+ messages in thread
From: Nathan Sidwell @ 2019-02-13 14:39 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches
On 2/13/19 9:36 AM, David Malcolm wrote:
> Ping
yes, sorry didn't realize you were waiting on me.
> On Fri, 2019-02-08 at 12:03 -0500, David Malcolm wrote:
>> Ping
>>
>> On Fri, 2019-01-25 at 15:02 -0500, David Malcolm wrote:
>>> On Fri, 2019-01-25 at 08:59 -0800, Nathan Sidwell wrote:
>>>> On 1/25/19 8:48 AM, David Malcolm wrote:
>>>>> PR c++/89036 reports an ICE due to this assertion failing
>>>>>
>>>>> 1136 /* A class should never have more than one
>>>>> destructor. */
>>>>> 1137 gcc_assert (!current_fns || via_using ||
>>>>> !DECL_DESTRUCTOR_P (method));
>>>>>
>>>>> on this template with a pair of dtors, with
>>>>> mutually exclusive "requires" clauses:
>>>>>
>>>>> template<typename T>
>>>>> struct Y {
>>>>> ~Y() requires(true) = default;
>>>>> ~Y() requires(false) {}
>>>>> };
>>>>>
>>>>> (is this valid? my knowledge of this part of C++ is fairly
>>>>> hazy)
>>>>
>>>> Yes. A more sensible example would have 'true' and 'false'
>>>> replaced
>>>> by
>>>> something determined from the template parms.
>>>>
>>>>>
>>>>> Nathan introduced this assertion as part of:
>>>>>
>>>>> ca9219bf18c68a001d62ecb981bc9176b0feaf12 (aka r251340):
>>>>> 2017-08-24 Nathan Sidwell <nathan@acm.org>
>>>>> Conversion operators kept on single overload set
>>>>
>>>> I'd just drop the assert at this point.
>>>>
>>>> nathan
>>>
>>> Thanks. Here's a version of the patch which drops the assertion.
>>>
>>> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>>>
>>> OK for trunk?
>>>
>>> gcc/cp/ChangeLog:
>>> PR c++/89036
>>> * class.c (add_method): Drop destructor assertion.
>>>
>>> gcc/testsuite/ChangeLog:
>>> PR c++/89036
>>> * g++.dg/concepts/pr89036.C: New test.
>>> ---
>>> gcc/cp/class.c | 3 ---
>>> gcc/testsuite/g++.dg/concepts/pr89036.C | 8 ++++++++
>>> 2 files changed, 8 insertions(+), 3 deletions(-)
>>> create mode 100644 gcc/testsuite/g++.dg/concepts/pr89036.C
>>>
>>> diff --git a/gcc/cp/class.c b/gcc/cp/class.c
>>> index e8773c2..6e9dac9 100644
>>> --- a/gcc/cp/class.c
>>> +++ b/gcc/cp/class.c
>>> @@ -1133,9 +1133,6 @@ add_method (tree type, tree method, bool
>>> via_using)
>>> }
>>> }
>>>
>>> - /* A class should never have more than one destructor. */
>>> - gcc_assert (!current_fns || via_using || !DECL_DESTRUCTOR_P
>>> (method));
>>> -
>>> current_fns = ovl_insert (method, current_fns, via_using);
>>>
>>> if (!COMPLETE_TYPE_P (type) && !DECL_CONV_FN_P (method)
>>> diff --git a/gcc/testsuite/g++.dg/concepts/pr89036.C
>>> b/gcc/testsuite/g++.dg/concepts/pr89036.C
>>> new file mode 100644
>>> index 0000000..f83ef8b
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/concepts/pr89036.C
>>> @@ -0,0 +1,8 @@
>>> +// { dg-do compile { target c++11 } }
>>> +// { dg-options "-fconcepts" }
>>> +
>>> +template<typename T>
>>> +struct Y {
>>> + ~Y() requires(true) = default;
>>> + ~Y() requires(false) {}
>>> +};
--
Nathan Sidwell
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-02-13 14:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-25 16:07 [PATCH] C++ concepts: fix ICE with requires on dtors (PR c++/89036) David Malcolm
2019-01-25 17:10 ` Nathan Sidwell
2019-01-25 21:20 ` [PATCH v2] " David Malcolm
2019-02-08 17:03 ` PING " David Malcolm
2019-02-13 14:36 ` PING^2 " David Malcolm
2019-02-13 14:39 ` Nathan Sidwell
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).