www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Consensus on goto's into catch blocks

reply "Iain Buclaw" <ibuclaw ubuntu.com> writes:
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.
Jun 13 2013
next sibling parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
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
Jun 13 2013
prev sibling next sibling parent "monarch_dodra" <monarchdodra gmail.com> writes:
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.
Jun 13 2013
prev sibling next sibling parent reply Brad Roberts <braddr puremagic.com> writes:
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.
Jun 13 2013
next sibling parent Timon Gehr <timon.gehr gmx.ch> writes:
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.
Jun 13 2013
prev sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 6/13/2013 3:53 PM, Brad Roberts wrote:
 Oh, just minimizing what's illegal.  _I_ wouldn't do that.

[Looks down, rubs toe in dirt.]
Jun 14 2013
prev sibling next sibling parent "monarch_dodra" <monarchdodra gmail.com> writes:
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!
Jun 13 2013
prev sibling next sibling parent "monarch_dodra" <monarchdodra gmail.com> writes:
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.
Jun 13 2013
prev sibling next sibling parent "Iain Buclaw" <ibuclaw ubuntu.com> writes:
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
Jun 13 2013
prev sibling next sibling parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
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.
Jun 13 2013
prev sibling next sibling parent "Iain Buclaw" <ibuclaw ubuntu.com> writes:
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?
Jun 13 2013
prev sibling next sibling parent reply Iain Buclaw <ibuclaw ubuntu.com> writes:
On 13 June 2013 20:18, H. S. Teoh <hsteoh quickfur.ath.cx> wrote:
 On Thu, Jun 13, 2013 at 11:50:49AM -0700, Brad Roberts 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.

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.

Thank goodness D's exceptions are all heap allocated! -- Iain Buclaw *(p < e ? p++ : p) = (c & 0x0f) + '0';
Jun 13 2013
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 6/13/2013 2:08 PM, monarch_dodra wrote:
 Well, I guess as long as the compiler (optionally) supports skipping over
 initialization it means it can handle the magic required to maintain the stack
 pointer, so that shouldn't really be a problem.

Sometimes it is handy to skip initializations, and I get annoyed with g++ barfs on me doing that. That said, such should only be allowed in system code. On a related note, doing a goto into a finally block is pretty problematic to support because it's actually a mini-function.
Jun 14 2013
parent Timon Gehr <timon.gehr gmx.ch> writes:
On 06/14/2013 08:44 PM, Walter Bright wrote:
 On 6/13/2013 2:08 PM, monarch_dodra wrote:
 Well, I guess as long as the compiler (optionally) supports skipping over
 initialization it means it can handle the magic required to maintain
 the stack
 pointer, so that shouldn't really be a problem.

Sometimes it is handy to skip initializations, and I get annoyed with g++ barfs on me doing that. That said, such should only be allowed in system code. On a related note, doing a goto into a finally block is pretty problematic to support because it's actually a mini-function.

In the worst case, duplicate some code.
Jun 14 2013
prev sibling next sibling parent Iain Buclaw <ibuclaw ubuntu.com> writes:
On 13 June 2013 20:08, monarch_dodra <monarchdodra gmail.com> 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. 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!

Well, a good error message will do well in that case. Currently, gdc has the following stock errors: "cannot goto into catch block" Which is indeed confusing if anonymous catches are allowed to bypass this check. What I would instead do (once I have the scan for skipped initialisations complete) is have a more meaningful, and generic: "goto to label 'L2' skips initialisation of 'Exception e'" Ditto for any other forms of jumps over any other local variable declarations. Regards -- Iain Buclaw *(p < e ? p++ : p) = (c & 0x0f) + '0';
Jun 13 2013
prev sibling next sibling parent "monarch_dodra" <monarchdodra gmail.com> writes:
On Thursday, 13 June 2013 at 20:40:27 UTC, Iain Buclaw wrote:
 On 13 June 2013 20:18, H. S. Teoh <hsteoh quickfur.ath.cx> 
 wrote:
 On Thu, Jun 13, 2013 at 11:50:49AM -0700, Brad Roberts 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.

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.

Thank goodness D's exceptions are all heap allocated! -- Iain Buclaw *(p < e ? p++ : p) = (c & 0x0f) + '0';

Irrelevant no? You're still throwing around your pointer, which takes its stack space. Well, I guess as long as the compiler (optionally) supports skipping over initialization it means it can handle the magic required to maintain the stack pointer, so that shouldn't really be a problem.
Jun 13 2013
prev sibling next sibling parent Brad Roberts <braddr puremagic.com> writes:
On 6/13/13 12:22 PM, Iain Buclaw wrote:
 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?

Oh, just minimizing what's illegal. _I_ wouldn't do that.
Jun 13 2013
prev sibling next sibling parent Iain Buclaw <ibuclaw ubuntu.com> writes:
On 13 June 2013 23:53, Brad Roberts <braddr puremagic.com> wrote:
 On 6/13/13 12:22 PM, Iain Buclaw wrote:
 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?

Oh, just minimizing what's illegal. _I_ wouldn't do that.

OK, and just out of curiosity, what's your view on jumps into try statement blocks? goto L2; try { /* ... */ L2: /* ... */ } Obviously the same rule must apply if the jump to the label skips over any initialisations at the beginning of the try block above it... -- Iain Buclaw *(p < e ? p++ : p) = (c & 0x0f) + '0';
Jun 13 2013
prev sibling parent Iain Buclaw <ibuclaw ubuntu.com> writes:
On 14 June 2013 19:44, Walter Bright <newshound2 digitalmars.com> wrote:
 On 6/13/2013 2:08 PM, monarch_dodra wrote:
 Well, I guess as long as the compiler (optionally) supports skipping over
 initialization it means it can handle the magic required to maintain the
 stack
 pointer, so that shouldn't really be a problem.

Sometimes it is handy to skip initializations, and I get annoyed with g++ barfs on me doing that. That said, such should only be allowed in system code.

IMO, the only annoying ones in C++ are where switch statements jumping to case labels crosses initialisation of variables, but D solves that by having cases in their own scope. :o) Any other type of skipped initialisation is an error that should definitely have all alarm bells ringing. -- Iain Buclaw *(p < e ? p++ : p) = (c & 0x0f) + '0';
Jun 14 2013