diff mbox series

[C++] Tweak check_previous_goto_1 to emit hard errors instead of permerrors in some cases

Message ID ec57d3c8-469d-b0a7-60e8-42256ac55ac3@oracle.com
State New
Headers show
Series [C++] Tweak check_previous_goto_1 to emit hard errors instead of permerrors in some cases | expand

Commit Message

Paolo Carlini Aug. 9, 2018, 12:07 p.m. UTC 
Hi,

over the years we reworked and improved the code in decl.c checking 
gotos quite a bit. Lately, in some specific unsafe cases, identify_goto 
issues upfront an error instead of a permerror, whereas it used to 
always issue a permerror. Over the last weeks a few colleagues of mine 
noticed that we don't do that, escalating a permerror to a plain error, 
in a case which is certainly unsafe - decl_jump_unsafe returns 2 - thus, 
if the user passes -fpermissive we end up emitting assembly completely 
missing labels. The straightforward patchlet below passes testing on 
x86_64-linux.

Thanks, Paolo.

/////////////////////
/cp
2018-08-09  Paolo Carlini  <paolo.carlini@oracle.com>

	* decl.c (check_previous_goto_1): When decl_jump_unsafe returns 2
	emit an error instead of a permerror.

/testsuite
2018-08-09  Paolo Carlini  <paolo.carlini@oracle.com>

	* g++.dg/init/goto3.C: Adjust for error intead of permerror.

Comments

Paolo Carlini Aug. 10, 2018, 9:23 a.m. UTC | #1 
.. an additional clarification (I told you that over the years we 
changed this code quite a bit...): I originally added the testcase that 
I'm adjusting here, I did that when, back in 2014, I worked on 63558: 
the test uses -fpermissive -w and was meant to check, as requested by 
Manuel in the bug, that we didn't issue further unsuppressible 
explanatory diagnostic after the primary permerror. I would argue that 
that first clean-up back 2014 was an improvement but note that, in 
practice, before it we rejected (with a confusing permerror + error) 
this kind of broken code and we don't anymore. That's bad. We don't 
really emit sensible assembly for it, as I already explained.

Paolo.
Nathan Sidwell Aug. 14, 2018, 8:54 p.m. UTC | #2 
On 08/09/2018 08:07 AM, Paolo Carlini wrote:
> Hi,
> 
> over the years we reworked and improved the code in decl.c checking gotos quite 
> a bit. Lately, in some specific unsafe cases, identify_goto issues upfront an 
> error instead of a permerror, whereas it used to always issue a permerror. Over 
> the last weeks a few colleagues of mine noticed that we don't do that, 
> escalating a permerror to a plain error, in a case which is certainly unsafe - 
> decl_jump_unsafe returns 2 - thus, if the user passes -fpermissive we end up 
> emitting assembly completely missing labels. The straightforward patchlet below 
> passes testing on x86_64-linux.
> 
> Thanks, Paolo.
> 
> /////////////////////
> 

ok, thanks
diff mbox series

Patch

Index: cp/decl.c
===================================================================
--- cp/decl.c	(revision 263443)
+++ cp/decl.c	(working copy)
@@ -3191,7 +3191,8 @@  check_previous_goto_1 (tree decl, cp_binding_level
 	  if (!identified)
 	    {
 	      complained = identify_goto (decl, input_location, locus,
-					  DK_PERMERROR);
+					  problem > 1
+					  ? DK_ERROR : DK_PERMERROR);
 	      identified = 1;
 	    }
 	  if (complained)
Index: testsuite/g++.dg/init/goto3.C
===================================================================
--- testsuite/g++.dg/init/goto3.C	(revision 263443)
+++ testsuite/g++.dg/init/goto3.C	(working copy)
@@ -15,11 +15,11 @@  adapt_parameters_next_iteration(void)
     case VAR_NONE: break;
 
     case VAR_DELTA:
-        int trunc_n_ants = 0;
+        int trunc_n_ants = 0;  // { dg-message "initialization" }
         n_ants += trunc_n_ants;
         break;
-    case VAR_SWITCH:
+    case VAR_SWITCH:  // { dg-error "jump" }
         break;
-      default: break;
+      default: break;  // { dg-error "jump" }
     }
 }