diff mbox series

[v2] C++ concepts: fix ICE with requires on dtors (PR c++/89036)

Message ID 1548446549-54085-1-git-send-email-dmalcolm@redhat.com
State New
Headers show
Series [v2] C++ concepts: fix ICE with requires on dtors (PR c++/89036) | expand

Commit Message

David Malcolm Jan. 25, 2019, 8:02 p.m. UTC 
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

Comments

David Malcolm Feb. 8, 2019, 5:03 p.m. UTC | #1 
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) {}
> +};
David Malcolm Feb. 13, 2019, 2:36 p.m. UTC | #2 
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) {}
> > +};
Nathan Sidwell Feb. 13, 2019, 2:39 p.m. UTC | #3 
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) {}
>>> +};
diff mbox series

Patch

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) {}
+};