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


Thanks
Iain.

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?)


T

-- 
"Holy war is an oxymoron." -- Lazarus Long

On Thursday, 13 June 2013 at 17:35:41 UTC, 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);
> }
>
>
> Thanks
> Iain.

For what it's worth, this C++ program with GCC:

int main()
{
   goto L2;
   try {
       throw 5;
   }
   catch (int e) {
L2: ;
   }
}

produces:
main.cpp|| In function 'int main()':|
main.cpp|8|error: jump to label 'L2' [-fpermissive]|
main.cpp|3|error:   from here [-fpermissive]|
main.cpp|7|error:   crosses initialization of 'int e'|
main.cpp|8|error:   enters catch block|

I know you can add -permissive for GCC, or by default with VS*,
and crossed initliazers will be default constructed. That said, I
think it is mostly a workaround for legacy C, and *really* bad
practice to do it anyways.

So I think bottom line is: Invalid according to strict grammar,
and for a good reason.

*Amonst other things, like passing temporaries by ref. :puke: I
hate VS so much. PS: I'm responsible for cross compiling in my
company. The amount of shit VS allows is crazy scary.

On 6/13/13 10:35 AM, 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);
> }
>
>
> Thanks
> Iain.

I think it should be illegal, but not because it's a catch block but because of the initialization.  If the catch was just "catch (Exception)" then it shouldn't be illegal.

On Thursday, 13 June 2013 at 18:51:01 UTC, Brad Roberts wrote:
> On 6/13/13 10:35 AM, Iain Buclaw wrote:
>
> I think it should be illegal, but not because it's a catch block but because of the initialization.
>  If the catch was just "catch (Exception)" then it shouldn't be illegal.

Again for what it's worth, the C++ program with "catch(...)" also fails.

Even if the variable is unnamed though, doesn't the code still cross the declaration, which is enough to make a mess of things? I mean, the caught exception is still on the stack somewhere, right?

And still, if, by some hoops, we could make it work, do we really want to add that functionality? I mean, would it really even be useful?

That, and I'm not a fan to having code that breaks just because you later decided to name a variable.

Eg:
before:
catch(Exception) //OK
after
catch(Exception e) //Nope, that's illegal now. Good luck finding out why!

On Thursday, 13 June 2013 at 19:08:40 UTC, monarch_dodra wrote:
> On Thursday, 13 June 2013 at 18:51:01 UTC, Brad Roberts wrote:
>> On 6/13/13 10:35 AM, Iain Buclaw wrote:
>>
>> I think it should be illegal, but not because it's a catch block but because of the initialization.
>> If the catch was just "catch (Exception)" then it shouldn't be illegal.
>
> Again for what it's worth, the C++ program with "catch(...)" also fails.

And for "catch(int)", this is interesting, it produces:
main.cpp||In function 'int main()':|
main.cpp|8|error: jump to label 'L2' [-fpermissive]|
main.cpp|3|error:   from here [-fpermissive]|
main.cpp|7|error:   crosses initialization of 'int <anonymous>'|
main.cpp|8|error:   enters catch block|

So yeah, just crossing a declaration, even anonymous, is enough for illegality in C++, hence simply no way to jump to inside a catch block.

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

On Thu, Jun 13, 2013 at 11:50:49AM -0700, Brad Roberts wrote:
> On 6/13/13 10:35 AM, 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);
> >}
> >
> >
> >Thanks
> >Iain.
> 
> I think it should be illegal, but not because it's a catch block but because of the initialization.  If the catch was just "catch (Exception)" then it shouldn't be illegal.

Why shouldn't it be illegal, though? I honestly don't see any use case for such a strange construct. Not to mention, like monarch_dodra said, if the anonymous Exception's reference is on the stack, then at the end of the catch block there'd be code to adjust the stack pointer, which will trash your stack pointer horribly if we goto the middle of the block bypassing the stack allocation of the Exception reference.


T

-- 
What's a "hot crossed bun"? An angry rabbit.

On Thursday, 13 June 2013 at 18:51:01 UTC, Brad Roberts wrote:
> On 6/13/13 10:35 AM, 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);
>> }
>>
>>
>> Thanks
>> Iain.
>
> I think it should be illegal, but not because it's a catch block but because of the initialization.
>  If the catch was just "catch (Exception)" then it shouldn't be illegal.

This could be easily possible to do, and still keep 100% safe.  But I must still ask why why why, Delilah, would you do that?

On 06/13/2013 08:50 PM, Brad Roberts wrote:
> On 6/13/13 10:35 AM, 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);
>> }
>>
>>
>> Thanks
>> Iain.
>
> I think it should be illegal, but not because it's a catch block but
> because of the initialization.  If the catch was just "catch
> (Exception)" then it shouldn't be illegal.

+1.