diff mbox

PATCH to disable the canonical types check in verify_type (PR c++/70029)

Message ID 20160414143058.GA28445@redhat.com
State New
Headers show

Commit Message

Marek Polacek April 14, 2016, 2:30 p.m. UTC
Looking at this PR again, seems we have reached conclusion that the way forward
for GCC 6 is to temporarily disable the check, so I'm posting a patch for that,
so as to finally resolve this PR.  The problem is that the C++ FE violates the
check when it sets FUNCTION_*_QUALIFIED flags.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2016-04-14  Marek Polacek  <polacek@redhat.com>
	    Jan Hubicka  <hubicka@ucw.cz>

	PR c++/70029
	* tree.c (verify_type): Disable the canonical type of main variant
	check.

	* g++.dg/torture/pr70029.C: New test.


	Marek

Comments

Jason Merrill April 14, 2016, 3:01 p.m. UTC | #1
On 04/14/2016 10:30 AM, Marek Polacek wrote:
> +  /* FIXME: this is violated by the C++ FE as discussed in PR70029, when
> +     FUNCTION_*_QUALIFIED flags are set.  */
> +  if (0 && TYPE_MAIN_VARIANT (t) == t && ct && TYPE_MAIN_VARIANT (ct) != ct)

How about guarding this check with flag_checking rather than disabling 
it entirely?  That way it won't affect released compilers, and we can 
downgrade the PR from P1, but doesn't hide the bug.

Jason
Jakub Jelinek April 14, 2016, 3:05 p.m. UTC | #2
On Thu, Apr 14, 2016 at 11:01:02AM -0400, Jason Merrill wrote:
> On 04/14/2016 10:30 AM, Marek Polacek wrote:
> >+  /* FIXME: this is violated by the C++ FE as discussed in PR70029, when
> >+     FUNCTION_*_QUALIFIED flags are set.  */
> >+  if (0 && TYPE_MAIN_VARIANT (t) == t && ct && TYPE_MAIN_VARIANT (ct) != ct)
> 
> How about guarding this check with flag_checking rather than disabling it
> entirely?  That way it won't affect released compilers, and we can downgrade
> the PR from P1, but doesn't hide the bug.

That will still mean people who use -fchecking will keep reporting such
ICEs.  I think it is better to disable it and reenable after GCC 6 branches.

	Jakub
Jason Merrill April 14, 2016, 3:18 p.m. UTC | #3
On 04/14/2016 11:05 AM, Jakub Jelinek wrote:
> On Thu, Apr 14, 2016 at 11:01:02AM -0400, Jason Merrill wrote:
>> On 04/14/2016 10:30 AM, Marek Polacek wrote:
>>> +  /* FIXME: this is violated by the C++ FE as discussed in PR70029, when
>>> +     FUNCTION_*_QUALIFIED flags are set.  */
>>> +  if (0 && TYPE_MAIN_VARIANT (t) == t && ct && TYPE_MAIN_VARIANT (ct) != ct)
>>
>> How about guarding this check with flag_checking rather than disabling it
>> entirely?  That way it won't affect released compilers, and we can downgrade
>> the PR from P1, but doesn't hide the bug.
>
> That will still mean people who use -fchecking will keep reporting such
> ICEs.  I think it is better to disable it and reenable after GCC 6 branches.

OK.

Jason
Marek Polacek April 14, 2016, 4:12 p.m. UTC | #4
On Thu, Apr 14, 2016 at 11:18:59AM -0400, Jason Merrill wrote:
> On 04/14/2016 11:05 AM, Jakub Jelinek wrote:
> >On Thu, Apr 14, 2016 at 11:01:02AM -0400, Jason Merrill wrote:
> >>On 04/14/2016 10:30 AM, Marek Polacek wrote:
> >>>+  /* FIXME: this is violated by the C++ FE as discussed in PR70029, when
> >>>+     FUNCTION_*_QUALIFIED flags are set.  */
> >>>+  if (0 && TYPE_MAIN_VARIANT (t) == t && ct && TYPE_MAIN_VARIANT (ct) != ct)
> >>
> >>How about guarding this check with flag_checking rather than disabling it
> >>entirely?  That way it won't affect released compilers, and we can downgrade
> >>the PR from P1, but doesn't hide the bug.
> >
> >That will still mean people who use -fchecking will keep reporting such
> >ICEs.  I think it is better to disable it and reenable after GCC 6 branches.
> 
> OK.

Should I consider the patch approved or do we want to hear from Honza?

	Marek
Jeff Law April 14, 2016, 4:40 p.m. UTC | #5
On 04/14/2016 10:12 AM, Marek Polacek wrote:
> On Thu, Apr 14, 2016 at 11:18:59AM -0400, Jason Merrill wrote:
>> On 04/14/2016 11:05 AM, Jakub Jelinek wrote:
>>> On Thu, Apr 14, 2016 at 11:01:02AM -0400, Jason Merrill wrote:
>>>> On 04/14/2016 10:30 AM, Marek Polacek wrote:
>>>>> +  /* FIXME: this is violated by the C++ FE as discussed in PR70029, when
>>>>> +     FUNCTION_*_QUALIFIED flags are set.  */
>>>>> +  if (0 && TYPE_MAIN_VARIANT (t) == t && ct && TYPE_MAIN_VARIANT (ct) != ct)
>>>>
>>>> How about guarding this check with flag_checking rather than disabling it
>>>> entirely?  That way it won't affect released compilers, and we can downgrade
>>>> the PR from P1, but doesn't hide the bug.
>>>
>>> That will still mean people who use -fchecking will keep reporting such
>>> ICEs.  I think it is better to disable it and reenable after GCC 6 branches.
>>
>> OK.
>
> Should I consider the patch approved or do we want to hear from Honza?
Given the BZ & list discussion, I'd consider the patch approved.

I *think* the way to deal with the BZ is to change the regression marker 
to 7 and the target milestone as well.  I think leaving it as a P1 would 
be fine as that'll force revisiting in the gcc-7 timeframe.

jeff
Marek Polacek April 14, 2016, 4:53 p.m. UTC | #6
On Thu, Apr 14, 2016 at 10:40:43AM -0600, Jeff Law wrote:
> Given the BZ & list discussion, I'd consider the patch approved.
> 
> I *think* the way to deal with the BZ is to change the regression marker to
> 7 and the target milestone as well.  I think leaving it as a P1 would be
> fine as that'll force revisiting in the gcc-7 timeframe.

Thanks, I've done so now.

	Marek
Jan Hubicka April 15, 2016, 11:24 a.m. UTC | #7
> On Thu, Apr 14, 2016 at 10:40:43AM -0600, Jeff Law wrote:
> > Given the BZ & list discussion, I'd consider the patch approved.
> > 
> > I *think* the way to deal with the BZ is to change the regression marker to
> > 7 and the target milestone as well.  I think leaving it as a P1 would be
> > fine as that'll force revisiting in the gcc-7 timeframe.
> 
> Thanks, I've done so now.
Thanks for doing that! I am indeed happy with deferring this to GCC 7

Honza
> 
> 	Marek
diff mbox

Patch

diff --git gcc/testsuite/g++.dg/torture/pr70029.C gcc/testsuite/g++.dg/torture/pr70029.C
index e69de29..9592f0c 100644
--- gcc/testsuite/g++.dg/torture/pr70029.C
+++ gcc/testsuite/g++.dg/torture/pr70029.C
@@ -0,0 +1,12 @@ 
+// PR c++/70029
+// { dg-do compile }
+// { dg-options "-std=c++11 -g -flto" }
+// { dg-require-effective-target lto }
+
+struct A
+{
+  A();
+  int foo() && __attribute__ ((__warn_unused_result__)) { return 0; }
+};
+
+A a;
diff --git gcc/tree.c gcc/tree.c
index ed28429..c64d720 100644
--- gcc/tree.c
+++ gcc/tree.c
@@ -13584,7 +13584,9 @@  verify_type (const_tree t)
       debug_tree (ct);
       error_found = true;
     }
-  if (TYPE_MAIN_VARIANT (t) == t && ct && TYPE_MAIN_VARIANT (ct) != ct)
+  /* FIXME: this is violated by the C++ FE as discussed in PR70029, when
+     FUNCTION_*_QUALIFIED flags are set.  */
+  if (0 && TYPE_MAIN_VARIANT (t) == t && ct && TYPE_MAIN_VARIANT (ct) != ct)
    {
       error ("TYPE_CANONICAL of main variant is not main variant");
       debug_tree (ct);