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.
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.
 












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 ...




















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?







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




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?








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




 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?








 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




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.


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


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?


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


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. 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




be

 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


ever

 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




 be

 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.


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. 


<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.

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.


<snip>

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



being roughly equivalent to:











Arcane Jill
Jul 24 2004