diff mbox series

[C++] PR 89488 ("[9 Regression] ICE in merge_exception_specifiers, at cp/typeck2.c:2395")

Message ID 27d0753f-39f8-b80b-d92d-86ce7163ea23@oracle.com
State New
Headers show
Series [C++] PR 89488 ("[9 Regression] ICE in merge_exception_specifiers, at cp/typeck2.c:2395") | expand

Commit Message

Paolo Carlini Feb. 25, 2019, 3:27 p.m. UTC
Hi,

this error recovery regression has to do with the recent changes 
committed by Jason for c++/88368. What happens is that 
maybe_instantiate_noexcept fails the hard way, thus, toward the end of 
the function, doesn't update TREE_TYPE (fn) and just returns false. 
process_subob_fn doesn't notice and proceeds to call 
merge_exception_specifiers anyway where of course the gcc_assert 
(!DEFERRED_NOEXCEPT_SPEC_P (add)) triggers, because 
maybe_instantiate_noexcept has not done its normal job. To improve 
error-recovery I think we can simply leave *spec_p alone in such cases, 
because we would merge the *spec_p with a TYPE_RAISES_EXCEPTIONS 
(TREE_TYPE (fn)) where TREE_TYPE (fn) has not been normally computed. I 
tried a few other things which prima facie looked sensible but nothing 
else worked - eg, returning false from maybe_instantiate_noexcept and 
also updating TREE_TYPE (fn) to a noexcept_false_spec variant causes 
regressions exactly for the testcases of c++/88368.

Tested x86_64-linux.

Thanks, Paolo.

////////////////////////
/cp
2019-02-25  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/89488
	* method.c (process_subob_fn): When maybe_instantiate_noexcept returns
	false don't call merge_exception_specifiers.

/testsuite
2019-02-25  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/89488
	* g++.dg/cpp0x/nsdmi15.C: New.

Comments

Jason Merrill Feb. 26, 2019, 2:28 p.m. UTC | #1
On 2/25/19 10:27 AM, Paolo Carlini wrote:
> Hi,
> 
> this error recovery regression has to do with the recent changes 
> committed by Jason for c++/88368. What happens is that 
> maybe_instantiate_noexcept fails the hard way, thus, toward the end of 
> the function, doesn't update TREE_TYPE (fn) and just returns false. 
> process_subob_fn doesn't notice and proceeds to call 
> merge_exception_specifiers anyway where of course the gcc_assert 
> (!DEFERRED_NOEXCEPT_SPEC_P (add)) triggers, because 
> maybe_instantiate_noexcept has not done its normal job. To improve 
> error-recovery I think we can simply leave *spec_p alone in such cases, 
> because we would merge the *spec_p with a TYPE_RAISES_EXCEPTIONS 
> (TREE_TYPE (fn)) where TREE_TYPE (fn) has not been normally computed. I 
> tried a few other things which prima facie looked sensible but nothing 
> else worked - eg, returning false from maybe_instantiate_noexcept and 
> also updating TREE_TYPE (fn) to a noexcept_false_spec variant causes 
> regressions exactly for the testcases of c++/88368.

If maybe_instantiate_noexcept returns false, I think we should set 
*spec_p to error_mark_node.

Jason
Paolo Carlini Feb. 26, 2019, 4:02 p.m. UTC | #2
Hi,

On 26/02/19 15:28, Jason Merrill wrote:
> On 2/25/19 10:27 AM, Paolo Carlini wrote:
>> Hi,
>>
>> this error recovery regression has to do with the recent changes 
>> committed by Jason for c++/88368. What happens is that 
>> maybe_instantiate_noexcept fails the hard way, thus, toward the end 
>> of the function, doesn't update TREE_TYPE (fn) and just returns 
>> false. process_subob_fn doesn't notice and proceeds to call 
>> merge_exception_specifiers anyway where of course the gcc_assert 
>> (!DEFERRED_NOEXCEPT_SPEC_P (add)) triggers, because 
>> maybe_instantiate_noexcept has not done its normal job. To improve 
>> error-recovery I think we can simply leave *spec_p alone in such 
>> cases, because we would merge the *spec_p with a 
>> TYPE_RAISES_EXCEPTIONS (TREE_TYPE (fn)) where TREE_TYPE (fn) has not 
>> been normally computed. I tried a few other things which prima facie 
>> looked sensible but nothing else worked - eg, returning false from 
>> maybe_instantiate_noexcept and also updating TREE_TYPE (fn) to a 
>> noexcept_false_spec variant causes regressions exactly for the 
>> testcases of c++/88368.
>
> If maybe_instantiate_noexcept returns false, I think we should set 
> *spec_p to error_mark_node.

Sure, that certainly works, I tested it a couple of days ago and I'm 
finishing testing the below now. The only difference is that during 
error-recovery 'zl ()' is seen as seriously broken and we don't give the 
second,  "cannot convert", error message, which we used to give.

Thanks, Paolo.

/////////////////////
Index: cp/method.c
===================================================================
--- cp/method.c	(revision 269187)
+++ cp/method.c	(working copy)
@@ -1256,9 +1256,13 @@ process_subob_fn (tree fn, tree *spec_p, bool *tri
 
   if (spec_p)
     {
-      maybe_instantiate_noexcept (fn);
-      tree raises = TYPE_RAISES_EXCEPTIONS (TREE_TYPE (fn));
-      *spec_p = merge_exception_specifiers (*spec_p, raises);
+      if (!maybe_instantiate_noexcept (fn))
+	*spec_p = error_mark_node;
+      else
+	{
+	  tree raises = TYPE_RAISES_EXCEPTIONS (TREE_TYPE (fn));
+	  *spec_p = merge_exception_specifiers (*spec_p, raises);
+	}
     }
 
   if (!trivial_fn_p (fn) && !dtor_from_ctor)
Index: testsuite/g++.dg/cpp0x/nsdmi15.C
===================================================================
--- testsuite/g++.dg/cpp0x/nsdmi15.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/nsdmi15.C	(working copy)
@@ -0,0 +1,8 @@
+// PR c++/89488
+// { dg-do compile { target c++11 } }
+
+struct zl {
+  struct {
+    int x2 = zl ();  // { dg-error "default member" }
+  } fx;
+};
Jason Merrill Feb. 26, 2019, 9:19 p.m. UTC | #3
On 2/26/19 11:02 AM, Paolo Carlini wrote:
> Hi,
> 
> On 26/02/19 15:28, Jason Merrill wrote:
>> On 2/25/19 10:27 AM, Paolo Carlini wrote:
>>> Hi,
>>>
>>> this error recovery regression has to do with the recent changes 
>>> committed by Jason for c++/88368. What happens is that 
>>> maybe_instantiate_noexcept fails the hard way, thus, toward the end 
>>> of the function, doesn't update TREE_TYPE (fn) and just returns 
>>> false. process_subob_fn doesn't notice and proceeds to call 
>>> merge_exception_specifiers anyway where of course the gcc_assert 
>>> (!DEFERRED_NOEXCEPT_SPEC_P (add)) triggers, because 
>>> maybe_instantiate_noexcept has not done its normal job. To improve 
>>> error-recovery I think we can simply leave *spec_p alone in such 
>>> cases, because we would merge the *spec_p with a 
>>> TYPE_RAISES_EXCEPTIONS (TREE_TYPE (fn)) where TREE_TYPE (fn) has not 
>>> been normally computed. I tried a few other things which prima facie 
>>> looked sensible but nothing else worked - eg, returning false from 
>>> maybe_instantiate_noexcept and also updating TREE_TYPE (fn) to a 
>>> noexcept_false_spec variant causes regressions exactly for the 
>>> testcases of c++/88368.
>>
>> If maybe_instantiate_noexcept returns false, I think we should set 
>> *spec_p to error_mark_node.
> 
> Sure, that certainly works, I tested it a couple of days ago and I'm 
> finishing testing the below now. The only difference is that during 
> error-recovery 'zl ()' is seen as seriously broken and we don't give the 
> second,  "cannot convert", error message, which we used to give.

OK if it passes.

Jason
diff mbox series

Patch

Index: cp/method.c
===================================================================
--- cp/method.c	(revision 269187)
+++ cp/method.c	(working copy)
@@ -1254,9 +1254,8 @@  process_subob_fn (tree fn, tree *spec_p, bool *tri
       return;
     }
 
-  if (spec_p)
+  if (spec_p && maybe_instantiate_noexcept (fn))
     {
-      maybe_instantiate_noexcept (fn);
       tree raises = TYPE_RAISES_EXCEPTIONS (TREE_TYPE (fn));
       *spec_p = merge_exception_specifiers (*spec_p, raises);
     }
Index: testsuite/g++.dg/cpp0x/nsdmi15.C
===================================================================
--- testsuite/g++.dg/cpp0x/nsdmi15.C	(nonexistent)
+++ testsuite/g++.dg/cpp0x/nsdmi15.C	(working copy)
@@ -0,0 +1,8 @@ 
+// PR c++/89488
+// { dg-do compile { target c++11 } }
+
+struct zl {
+  struct {
+    int x2 = zl ();  // { dg-error "default member|cannot convert" }
+  } fx;
+};