www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - Not returning a value when you're supposed to

reply Arcane Jill <Arcane_member pathlink.com> writes:
This, on the other hand, does give a run-time Assert Error, but should be a
compile error because the flow of control leaves the function f without
returning anything OR throwing an exception.

#    int f(int x)
#    {
#        if (x < 0) ++x;
#        else return 1;
#    }
#    
#    void main()
#    {
#        printf("%d\n", f(-1));
#    }
Jul 20 2004
next sibling parent reply Derek Parnell <derek psych.ward> writes:
On Tue, 20 Jul 2004 08:58:02 +0000 (UTC), Arcane Jill wrote:

 This, on the other hand, does give a run-time Assert Error, but should be a
 compile error because the flow of control leaves the function f without
 returning anything OR throwing an exception.
 
 #    int f(int x)
 #    {
 #        if (x < 0) ++x;
 #        else return 1;
 #    }
 #    
 #    void main()
 #    {
 #        printf("%d\n", f(-1));
 #    }

I suspect that the reason is that there is a limit to the amount of optimizing that the compiler (not language) is capable of. The current compiler has decided to wait until run-time to catch these types of errors rather than try at compile time. At compile time, it would have to simulate the flow of control whenever it knew the value of the parameters, and record the effect, then at the end of the compile, it would have to go back to see if all calls to each routine were able to be simulated and each call caused a routine to fail to return a value, then it could issue an error message. Probably in the realm of diminishing returns. Consider this type of tracing ... # int f(int x) # { # if (x < 0) ++x; # else return 1; # } # # int x; # void main() # { # x = setX(3); # printf("%d\n", f(x)); # } # # int setX(int y) # { # x = y; # return -2; # } A nasty bit of simulation work there! Now consider multiple modules... -- Derek Melbourne, Australia 20/Jul/04 7:02:48 PM
Jul 20 2004
parent reply Arcane Jill <Arcane_member pathlink.com> writes:
In article <cdinis$1a3o$1 digitaldaemon.com>, Derek Parnell says...

At compile time, it would have to simulate the flow of control whenever it
knew the value of the parameters, and record the effect, then at the end of
the compile, it would have to go back to see if all calls to each routine
were able to be simulated and each call caused a routine to fail to return
a value, then it could issue an error message.

Flow of execution analysis is simpler than that. You just have to assume that EVERY branch of an if or switch will be taken at least once; that a for, foreach or while statement may be executed either zero times or at least once; and that nothing following a throw will ever be executed. Then you're sorted. (Compile-time ifs, like if(false), are an exception to that rule, but since they'll known at compile time, the compiler can figure those out anyway). Failing to make this relatively simple analysis results in the current bug - that it is possible to exit a function which requires a return value by running out of scope, instead of via a return statement. It should be noted further that the actual error message emitted by the compiler is: Error: AssertError Failure auto.d(8) I don't /have/ a file called "auto.d". Every assert failure demonstrates a bug. (Walter himself once stated "Arcane Jill, you rock" when I explained this to someone, and encouraged me to post my explanation to the Wiki, so I'm fairly confident he would agree with me on this one). So, whose bug is it? Have I violated the in contract of a DbC function? Nope. Have I written rubbish code? Yes - but why should that get me an assert error from within a file I don't even have? It's not exactly a useful error message. So whose bug is it? Certainly my code /does/ have a bug. The question is, is it (or should it be) legal D? If so, it's a user bug; if not, it's a DMD bug. Let's ask the expert. Walter, is this legal D or not? # int f(int x) # { # if (x<0) ++x; // This branch doesn't execute return # else return x; # } Arcane Jill
Jul 20 2004
parent reply "Walter" <newshound digitalmars.com> writes:
"Arcane Jill" <Arcane_member pathlink.com> wrote in message
news:cdit1q$1c15$1 digitaldaemon.com...
 In article <cdinis$1a3o$1 digitaldaemon.com>, Derek Parnell says...

At compile time, it would have to simulate the flow of control whenever


knew the value of the parameters, and record the effect, then at the end


the compile, it would have to go back to see if all calls to each routine
were able to be simulated and each call caused a routine to fail to


a value, then it could issue an error message.


 EVERY branch of an if or switch will be taken at least once; that a for,

 or while statement may be executed either zero times or at least once; and

 nothing following a throw will ever be executed. Then you're sorted.

 (Compile-time ifs, like if(false), are an exception to that rule, but

 they'll known at compile time, the compiler can figure those out anyway).

 Failing to make this relatively simple analysis results in the current

 that it is possible to exit a function which requires a return value by

 out of scope, instead of via a return statement.

 It should be noted further that the actual error message emitted by the

 is:

 Error: AssertError Failure auto.d(8)

 I don't /have/ a file called "auto.d".

 Every assert failure demonstrates a bug. (Walter himself once stated

 Jill, you rock" when I explained this to someone, and encouraged me to

 explanation to the Wiki, so I'm fairly confident he would agree with me on

 one). So, whose bug is it? Have I violated the in contract of a DbC

 Nope. Have I written rubbish code? Yes - but why should that get me an

 error from within a file I don't even have? It's not exactly a useful

 message. So whose bug is it? Certainly my code /does/ have a bug. The

 is, is it (or should it be) legal D? If so, it's a user bug; if not, it's

 bug.

 Let's ask the expert. Walter, is this legal D or not?

 #    int f(int x)
 #    {
 #        if (x<0) ++x; // This branch doesn't execute return
 #        else return x;
 #    }

Yes, it's legal. The "auto.d", though, if the file is named something different, then that is a bug. But, this is why I want a complete, reproducible example, because when I do the obvious: ------------------------------------------------------- C:\cbx>type test.d int f(int x) { if (x<0) ++x; // This branch doesn't execute return else return x; } void main() { f(-1); } C:\cbx>dmd test \dm\bin\link test,,,user32+kernel32/noi; C:\cbx>test Error: AssertError Failure test.d(5) C:\cbx> ---------------------------------------------------------------- you can see that it prints the correct file name in the assert, so there's something crucial left out of the bug report.
Jul 20 2004
parent reply "Ivan Senji" <ivan.senji public.srce.hr> writes:
"Walter" <newshound digitalmars.com> wrote in message
news:cdjhp8$1m1l$1 digitaldaemon.com...
 "Arcane Jill" <Arcane_member pathlink.com> wrote in message
 news:cdit1q$1c15$1 digitaldaemon.com...
 In article <cdinis$1a3o$1 digitaldaemon.com>, Derek Parnell says...

At compile time, it would have to simulate the flow of control whenever


knew the value of the parameters, and record the effect, then at the



 of
the compile, it would have to go back to see if all calls to each



were able to be simulated and each call caused a routine to fail to


a value, then it could issue an error message.


 EVERY branch of an if or switch will be taken at least once; that a for,

 or while statement may be executed either zero times or at least once;


 that
 nothing following a throw will ever be executed. Then you're sorted.

 (Compile-time ifs, like if(false), are an exception to that rule, but

 they'll known at compile time, the compiler can figure those out


 Failing to make this relatively simple analysis results in the current

 that it is possible to exit a function which requires a return value by

 out of scope, instead of via a return statement.

 It should be noted further that the actual error message emitted by the

 is:

 Error: AssertError Failure auto.d(8)

 I don't /have/ a file called "auto.d".

 Every assert failure demonstrates a bug. (Walter himself once stated

 Jill, you rock" when I explained this to someone, and encouraged me to

 explanation to the Wiki, so I'm fairly confident he would agree with me


 this
 one). So, whose bug is it? Have I violated the in contract of a DbC

 Nope. Have I written rubbish code? Yes - but why should that get me an

 error from within a file I don't even have? It's not exactly a useful

 message. So whose bug is it? Certainly my code /does/ have a bug. The

 is, is it (or should it be) legal D? If so, it's a user bug; if not,


 a DMD
 bug.

 Let's ask the expert. Walter, is this legal D or not?

 #    int f(int x)
 #    {
 #        if (x<0) ++x; // This branch doesn't execute return
 #        else return x;
 #    }

Yes, it's legal. The "auto.d", though, if the file is named something different, then that is a bug. But, this is why I want a complete, reproducible example, because when I do the obvious:

So you are saying that it is legal for an "int" function not to return an int, but it has to have atleast one return even though it might be skiped as int this example: int func() { goto label; return 0; label: ; }
 -------------------------------------------------------
 C:\cbx>type test.d
 int f(int x)
 {
       if (x<0) ++x; // This branch doesn't execute return
        else return x;
 }

 void main()
 {
     f(-1);
 }

 C:\cbx>dmd test
 \dm\bin\link test,,,user32+kernel32/noi;

 C:\cbx>test
 Error: AssertError Failure test.d(5)

 C:\cbx>
 ----------------------------------------------------------------
 you can see that it prints the correct file name in the assert, so there's
 something crucial left out of the bug report.

Jul 20 2004
parent reply "Walter" <newshound digitalmars.com> writes:
"Ivan Senji" <ivan.senji public.srce.hr> wrote in message
news:cdjimt$1mfr$1 digitaldaemon.com...
 So you are saying that it is legal for an "int" function not to return an
 int, but it has to have atleast one return even though it might be skiped
 as int this example:

 int func()
 {
      goto label;
      return 0;
      label:
      ;
 }

It is illegal to drop off the end without a return and to execute such a path.
Jul 20 2004
parent reply "Ivan Senji" <ivan.senji public.srce.hr> writes:
"Walter" <newshound digitalmars.com> wrote in message
news:cdjknj$1nef$1 digitaldaemon.com...
 "Ivan Senji" <ivan.senji public.srce.hr> wrote in message
 news:cdjimt$1mfr$1 digitaldaemon.com...
 So you are saying that it is legal for an "int" function not to return


 int, but it has to have atleast one return even though it might be


 as int this example:

 int func()
 {
      goto label;
      return 0;
      label:
      ;
 }

It is illegal to drop off the end without a return and to execute such a path.

But this function compiles and causes assert, so it basicaly does the same as if there wasn't a return, but the compiler is quite happy accepting it although it is invalid code. Here the compiler is tricked to accept the code that can't work, and it doesn't sound too good to me to trick the compiler. I would more like to trust your compiler to report these errors than to trust any tricks. BTW isn't it a bit silly that the compiler forses us to have a return and it is happy even though the return is placed in a ridicolous or unreachable place? PS. I'm going to the sea tomorow, and i'm looking forward to seeing 0.96 (or some bigger number) when i get back :)
Jul 20 2004
next sibling parent "Walter" <newshound digitalmars.com> writes:
"Ivan Senji" <ivan.senji public.srce.hr> wrote in message
news:cdk0up$1sun$1 digitaldaemon.com...
 But this function compiles and causes assert, so it basicaly does the same
 as if there wasn't a return, but the compiler is quite happy accepting it
 although it is invalid code. Here the compiler is tricked to accept the

 that can't work, and it doesn't sound too good to me to trick the

 I would more like to trust your compiler to report these errors than to
 trust any tricks. BTW isn't it a bit silly that the compiler forses us to
 have
 a return and it is happy even though the return is placed in a ridicolous
 or unreachable place?

As far as the language spec is concerned, it doesn't matter if illegal code is caught at compile time or run time. I don't see the runtime check as being a trick.
Jul 20 2004
prev sibling parent reply Arcane Jill <Arcane_member pathlink.com> writes:
In article <cdk0up$1sun$1 digitaldaemon.com>, Ivan Senji says...
PS. I'm going to the sea tomorow, and i'm looking forward to seeing
0.96 (or some bigger number) when i get back :)

In Valen's name! You don't watch Babylon 5 do you? To a Minbari, "going to the sea" is a euphamism for dying. Coming back again afterwards would definitely elevate you a most exalted status. Jill PS. Sorry. Off topic post - but I couldn't resist it.
Jul 21 2004
parent reply "Ivan Senji" <ivan.senji public.srce.hr> writes:
"Arcane Jill" <Arcane_member pathlink.com> wrote in message
news:cdl6ha$2e47$1 digitaldaemon.com...
 In article <cdk0up$1sun$1 digitaldaemon.com>, Ivan Senji says...
PS. I'm going to the sea tomorow, and i'm looking forward to seeing
0.96 (or some bigger number) when i get back :)

In Valen's name! You don't watch Babylon 5 do you?

I don't :)
 To a Minbari, "going to the sea" is a  euphamism for dying.

I didn't know this! I'm sorry but i really don't know how to say that in English :)
 Coming back again afterwards would definitely elevate you a
 most exalted status.

 Jill

 PS. Sorry. Off topic post - but I couldn't resist it.

Jul 24 2004
parent J C Calvarese <jcc7 cox.net> writes:
Ivan Senji wrote:
 "Arcane Jill" <Arcane_member pathlink.com> wrote in message
 news:cdl6ha$2e47$1 digitaldaemon.com...
 
In article <cdk0up$1sun$1 digitaldaemon.com>, Ivan Senji says...

PS. I'm going to the sea tomorow, and i'm looking forward to seeing
0.96 (or some bigger number) when i get back :)

In Valen's name! You don't watch Babylon 5 do you?

I don't :)
To a Minbari, "going to the sea" is a  euphamism for dying.

I didn't know this! I'm sorry but i really don't know how to say that in English :)

Perhaps if you specify the particular body of water, the Minbari won't be worried about you. ;) -- Justin (a/k/a jcc7) http://jcc_7.tripod.com/d/
Jul 24 2004
prev sibling parent reply "Walter" <newshound digitalmars.com> writes:
"Arcane Jill" <Arcane_member pathlink.com> wrote in message
news:cdimqq$19mc$1 digitaldaemon.com...
 This, on the other hand, does give a run-time Assert Error, but should be

 compile error because the flow of control leaves the function f without
 returning anything OR throwing an exception.

In the general case, it is impossible for the compiler to determine the actual flow of control. I generally dislike getting messages from other compilers about "no return statement" for a path through the function that will never happen. To fix it, then, I have to insert a dummy return statement: return some_dummy_value; // this statement will never be executed, but we put it here to get the compiler // to shut up } which is annoying. Putting in the assert, though, neatly solves the problem, because if it ever actually does go on that path, the error is flagged.
Jul 20 2004
next sibling parent reply "Ivan Senji" <ivan.senji public.srce.hr> writes:
"Walter" <newshound digitalmars.com> wrote in message
news:cdjgje$1lju$1 digitaldaemon.com...
 "Arcane Jill" <Arcane_member pathlink.com> wrote in message
 news:cdimqq$19mc$1 digitaldaemon.com...
 This, on the other hand, does give a run-time Assert Error, but should


 a
 compile error because the flow of control leaves the function f without
 returning anything OR throwing an exception.

In the general case, it is impossible for the compiler to determine the actual flow of control.

Don't be angry when i say this but: i don't believe you :) You are a compiler writer, but from what i know/remember when learning about compilers i know that a compiler can know about every possible flow (i remember we were drawing some diagrams :) so it could forse every flow to end with return or throw.
 I generally dislike getting messages from other
 compilers about "no return statement" for a path through the function that
 will never happen.

So you dislike dmd? It does just that, complains about "no return" even though it will never happen.
 To fix it, then, I have to insert a dummy return statement:

     return some_dummy_value;    // this statement will never be executed,
 but we put it here to get the compiler
                                                  // to shut up
 }

 which is annoying.

 Putting in the assert, though, neatly solves the problem, because if it

 actually does go on that path, the error is flagged.

->Runtime error finding - could be compile time.
Jul 20 2004
parent "Walter" <newshound digitalmars.com> writes:
"Ivan Senji" <ivan.senji public.srce.hr> wrote in message
news:cdjimu$1mfr$2 digitaldaemon.com...
 "Walter" <newshound digitalmars.com> wrote in message
 news:cdjgje$1lju$1 digitaldaemon.com...
 "Arcane Jill" <Arcane_member pathlink.com> wrote in message
 news:cdimqq$19mc$1 digitaldaemon.com...
 This, on the other hand, does give a run-time Assert Error, but should


 a
 compile error because the flow of control leaves the function f



 returning anything OR throwing an exception.

In the general case, it is impossible for the compiler to determine the actual flow of control.

Don't be angry when i say this but: i don't believe you :) You are a compiler writer, but from what i know/remember when learning about compilers i know that a compiler can know about every possible flow (i remember we were drawing some diagrams :) so it could forse every flow to end with return or throw.

One common case for this is: int func() { while (1) { if (...) return 0; } } which will get you an error from many of today's most modern compilers. It's possible for flow analysis to figure this out, but I don't want there to be some D compilers that accept it and some that issue an error. Compilation errors need to be consistent. There's also: extern int x; int func() { if (x == 1) return 0; else if (x == 3) return 2; } where flow analysis cannot determine that x will never be 28. BTW, in the first example, the optimizer *does* figure out that the fall-off return will never be executed, and so removes the assert as 'dead code'.
Jul 20 2004
prev sibling parent reply Stewart Gordon <smjg_1998 yahoo.com> writes:
Walter wrote:
<snip>
 In the general case, it is impossible for the compiler to determine
 the actual flow of control. I generally dislike getting messages from
 other compilers about "no return statement" for a path through the
 function that will never happen. To fix it, then, I have to insert a
 dummy return statement:

Which is actually more likely: a can't happen situation or a coding error?
     return some_dummy_value;    // this statement will never be executed,
 but we put it here to get the compiler
                                                  // to shut up
 }
 
 which is annoying.

Or putting in an assert(false) at that point, which is better if you want to be able to catch the bug.
 Putting in the assert, though, neatly solves the problem, because if
 it ever actually does go on that path, the error is flagged.

Requiring either a return or an assert(false) is even neater IMO, as it would then be easy to pinpoint the missing return, and would force the user to check whether it really is an error or a can't happen. Stewart. -- My e-mail is valid but not my primary mailbox, aside from its being the unfortunate victim of intensive mail-bombing at the moment. Please keep replies on the 'group where everyone may benefit.
Jul 20 2004
parent reply J Anderson <REMOVEanderson badmama.com.au> writes:
Stewart Gordon wrote:

 Requiring either a return or an assert(false) is even neater IMO, as 
 it would then be easy to pinpoint the missing return, and would force 
 the user to check whether it really is an error or a can't happen.

 Stewart.

Agreed! I don't see why all D compiles shouldn't have flow analysis as mandatory. This really is just a way of telling the compiler exactly what you mean. Early detection will save much time. You don't want someone else coming to you half an hour latter telling you that you've broken code only to find out you could of fixed it in a few seconds. Come on, Walter, you can't say that an assert is as good as a compile time error. The only valid excuse I can see is Walter making is that it is: - Its a difficult check to perform -- -Anderson: http://badmama.com.au/~anderson/
Jul 21 2004
parent reply Stewart Gordon <smjg_1998 yahoo.com> writes:
J Anderson wrote:

<snip>
 Agreed!  I don't see why all D compiles shouldn't have flow analysis as 
 mandatory. 

It would be very difficult, if not impossible, to determine at compile-time whether a particular path will acutally ever be executed. I think we'd need to solve the halting problem before it can be done perfectly. It is, however, simple to determine which parts of a function are structurally reachable, as C/C++ compilers manage. For example, import std.c.stdlib; int qwert(int yuiop) { if (abs(yuiop) + 1 > 0) return yuiop; } Obviously this is always true, and so the end of the function would never be reached. However, the compiler can't be absolutely sure of this; the structure of the function (as opposed to the actual behaviour) leaves the end open. Hence the compiler would catch this as an error. OTOH, if a function were defined by int asdfg(int yuiop) { if (abs(yuiop) + 1 > 0) { return yuiop; } else { return -1; } } then all paths through the structure of the program have returns, and so the end is structurally unreachable. And so no error would be flagged. A literal assert(false) or thrown exception would be treated as a return in this respect. At least before you get to catch blocks within the function being inspected.... Stewart. -- My e-mail is valid but not my primary mailbox, aside from its being the unfortunate victim of intensive mail-bombing at the moment. Please keep replies on the 'group where everyone may benefit.
Jul 21 2004
parent reply J Anderson <REMOVEanderson badmama.com.au> writes:
Stewart Gordon wrote:

 J Anderson wrote:

 <snip>

 Agreed!  I don't see why all D compiles shouldn't have flow analysis 
 as mandatory. 

<snip> It would be very difficult, if not impossible, to determine at compile-time whether a particular path will acutally ever be executed. I think we'd need to solve the halting problem before it can be done perfectly.

I think that all paths should have a mandatory return or assert in them whether the user likes one where or not. The compiler is not looking for paths that can be skipped. It should require them on all paths as in: int func() { if (x) { while (something) { return x; } //To fix the error message you'd need an assert here } else { return 0; } //or here } This type of analysis is reasonably simple. -- -Anderson: http://badmama.com.au/~anderson/
Jul 24 2004
parent Arcane Jill <Arcane_member pathlink.com> writes:
In article <cdtk83$so5$1 digitaldaemon.com>, J Anderson says...

I think that all paths should have a mandatory return or assert in them 

Assuming that where you wrote "assert" you actually meant "throw", I'd agree with you and support this one. "assert", of course, is /itself/ a flow control mechanism, with # assert(x); being roughly equivalent to: # debug # { # if (!(x)) # { # throw new AssertError(""); # } # // don't forget this path # } # // or this one Arcane Jill
Jul 24 2004