Consensus on goto's into catch blocks

[Iain Buclaw]

On Thursday, 13 June 2013 at 17:51:51 UTC, H. S. Teoh wrote:
> On Thu, Jun 13, 2013 at 07:35:39PM +0200, Iain Buclaw wrote:
>> Can someone remind me again what was last agreed when I 
>> brought this
>> up?
>> 
>> I seem to recall that this should be disallowed as is 
>> practically
>> always a bug, also, and it skips any initialisation of the 
>> exception
>> object. (See: http://dlang.org/statement.html - "It is illegal 
>> for a
>> GotoStatement to be used to skip initializations.")
>> 
>> Current failing test I want to have removed from the test 
>> suite.
>> 
>> test/runnable/eh.d:
>> void test8()
>> {
>>   int a;
>>   goto L2;    // gdc Error: cannot goto into catch block
>> 
>>   try {
>>       a += 2;
>>   }
>>   catch (Exception e) {
>>       a += 3;
>> L2: ;
>>       a += 100;
>>   }
>>   assert(a == 100);
>> }
> [...]
>
> Hmph, how did this test end up in test/runnable? What if after 
> L2, we
> try to deference e? I can't see any way this code can even be 
> remotely
> valid. How did this end up in the test suite?? (Or is this 
> supposed to
> test whether the compiler is able to ascertain that e is not 
> referenced
> after L2? Does DMD even do that kind of analysis?)
>


I think the test is for the analysis of the try block.  The 
frontend says: "Right, this try block will not throw, so all 
catches can be removed.", or "This try body is empty, so this 
entire try/catch statement can be removed"  -  However this is 
plain wrong in that the backend will require to know the bodies 
of all the catches in case they are used in some way.

In GDC's case, the compiler will ICE if it finds a goto to a 
label that doesn't exist, whereas DMD will happily accept this as 
valid and instead die when you try to run the program (oops!).  
So since around pre-2.060 I have always #ifdef'd out this 
frontend optimisation.  But since then analysis has been improved 
somewhat, but still misses this edge case.

void test8()
{
   int a;
   goto L2;    // BOOM!

   try { }
   catch (Exception e) {
L2: ;
       a += 100;
   }
   assert(a == 100);
}

But I have made alterations to fix that. :-)


Where GDC also differs from DMD is that the glue-layer part of 
the front-end also has a rudimentary goto/label analyser that 
makes sure that you:
- Can't goto into a try block (though testing just now, the try 
block requires a throw statement, so the frontend must wrongly 
unravel the try {} statement somehow).

- Can't goto into a catch block

- Can't goto into or out of a finally block.

- Can't have case/default in switches nested inside a try block,


Future improvements will be to also include skipped 
initialisations.

{
     goto L2;
     Object o = new Object;

   L2:
     writeln (o);
}

This also has the benefit of fleshing out these sorts of bugs in 
phobos (there are a few, I've already pre-tested it).

(I'm stalling)

But back to the point, to make this analysis work and be valid, 
the front-end must either stop unraveling/removing try/catch 
blocks, or improve the way it analyses code before doing this 
sort of optimisation.   I'm sure Walter would prefer the latter 
with patches welcome.  :o)

Regards
Iain