Local variable and field assignment to an expression with a safe invoke operator in it causes a runtime error if the result of the safely invoked expression is null.
Int n = maybeObj?.GetAnInt() //runtime error on null, works on non-null
It would seem that this should be a compile-time error, not a run-time one. Is that the case or is this the desired behavior?
brianFri 2 Jul 2010
Well what is really happening is this:
Int n := (Int)(maybeObj?.GetAnInt())
So the cast from Int? to Int is implied. I suppose the compiler could warn you that the right hand side could be null, but in general unless we know for sure it is null we make that a runtime error, not a compile-time error.
rfeldmanFri 2 Jul 2010
I would argue in this particular case a compile time warning would make good sense.
Compare these two statements:
Int n = maybeObj.GetAnInt()
Int n = maybeObj?.GetAnInt()
In both cases, if maybeObj is null, you will get a runtime error; adding the safe invoke operator is superfluous here.
The fact that the programmer has included a superfluous operator seems to me the same sort of red flag that an unreachable line of code would be. If they think this safe invoke operator is doing anything, isn't that a sign that they have likely made an error? That maybe they intended Int? n instead of Int n for example?
I would personally like to see a compile time warning of "Unnecessary safe invoke operator" for this case.
brianFri 2 Jul 2010
I agree it makes sense, although I suspect there are lots of cases where a warning might make sense. Not sure those belong in the compiler proper or as some sort of lint plugin?
qualidafialFri 2 Jul 2010
I agree with rfeldman, but I'd approach the compiler warning message from the other side: if I'm assigning from a safe-invoke expression to a non-nullable, then I want a warning stating that either the assignment target should be nullable, or that I need to add an elvis expression to the end to make the expression evaluate to non-null.
rfeldmanSat 3 Jul 2010
Sure, that sounds good to me too.
I would argue this belongs in the compiler proper, because it's not so much a code cleanliness issue as it is a red flag that something seems likely to be wrong in the program logic.
yachrisTue 6 Jul 2010
The lesson from 40 (!) years of having "lint" for C is that no one ever uses "lint" for C.
Although it's easy for me to say, since it involves work on your part :-) but PLEASE make the compiler do as much work as possible. It's a one-time expense that has HUGE payoffs, for thousands of people all over the world.
As an example, I'm on a C++ project now, and got caught by the
x == y;
bug. Java would have flagged this statement as an error, C++ blithely let it go right on by...
tacticsTue 6 Jul 2010
Perhaps a lint tool just needs better integration. Fantom has an official unit test tool. Perhaps a Fantom lint ought to be run by default before running the test suite on a pod.
rfeldmanTue 6 Jul 2010
This enabled-by-default plugin idea hints at an interesting paradigm for compilers.
Suppose the Fantom compiler's role was reimagined to detect only errors. If there's just no way it can build, it will let you know, but everything else is up to the plugins. By default (say, in some compiler config file), the Fantom Warnings plugin would be enabled, but you could remove or replace it as you liked.
This would be an interesting way to combat the problem I've seen where developers ignore all warnings simply because their warning list is mostly filled instances of (language-level) warnings they don't care about. It would be much better for them to be able to selectively disable such warnings by extending the default compiler plugin (or editing some plugin config file), than for them to ignore all warnings altogether.
Of course this would also have the nice side effect of answering the root question here. If the end user has the ability to suppress warnings they consider overzealous, it's relatively harmless to add useful ones to the default plugin as we think of them.
Worth considering, I think...
jodastephenTue 6 Jul 2010
Certainly Eclipse allows you to turn errors/warnings on/off at a fine level. Allowing that based on the standard compiler would avoid some of the need to create a second compiler.
However, in general I think most warnings are an annoyance, and usually occur in Java to warn about backwards compatibility (generics) or because the compiler writer wasn't determined enough to just make things errors.
rfeldmanTue 6 Jul 2010
And wouldn't it be nice if you could turn those kinds of warnings off? :)
brianWed 7 Jul 2010
Renamed from Safe invoke operator bug to Expand scope of illegal nullable to non-nullable errors
brianWed 7 Jul 2010
Promoted to ticket #1127 and assigned to brian
I agree with Stephen, if something shouldn't be done then is should be an error.
Warnings are for things like deprecated methods.
brianWed 7 Jul 2010
Ticket resolved in 1.0.54
I expanded the number of cases which result in compile time errors. These types of expressions are classified as "always nullable" which means they are never safe to implicitly coerse to non-nullable:
null literal (this was always caught at compile time)
safe invoke method calls and field accesses (per the original discussion)
use of the as operator
Attempting to use any of those expressions where a non-nullable type is expected will now result in a compile time error.
stravant Sat 26 Jun 2010
Local variable and field assignment to an expression with a safe invoke operator in it causes a runtime error if the result of the safely invoked expression is null.
It would seem that this should be a compile-time error, not a run-time one. Is that the case or is this the desired behavior?
brian Fri 2 Jul 2010
Well what is really happening is this:
So the cast from
Int?
toInt
is implied. I suppose the compiler could warn you that the right hand side could be null, but in general unless we know for sure it is null we make that a runtime error, not a compile-time error.rfeldman Fri 2 Jul 2010
I would argue in this particular case a compile time warning would make good sense.
Compare these two statements:
In both cases, if maybeObj is null, you will get a runtime error; adding the safe invoke operator is superfluous here.
The fact that the programmer has included a superfluous operator seems to me the same sort of red flag that an unreachable line of code would be. If they think this safe invoke operator is doing anything, isn't that a sign that they have likely made an error? That maybe they intended
Int? n
instead ofInt n
for example?I would personally like to see a compile time warning of "Unnecessary safe invoke operator" for this case.
brian Fri 2 Jul 2010
I agree it makes sense, although I suspect there are lots of cases where a warning might make sense. Not sure those belong in the compiler proper or as some sort of lint plugin?
qualidafial Fri 2 Jul 2010
I agree with rfeldman, but I'd approach the compiler warning message from the other side: if I'm assigning from a safe-invoke expression to a non-nullable, then I want a warning stating that either the assignment target should be nullable, or that I need to add an elvis expression to the end to make the expression evaluate to non-null.
rfeldman Sat 3 Jul 2010
Sure, that sounds good to me too.
I would argue this belongs in the compiler proper, because it's not so much a code cleanliness issue as it is a red flag that something seems likely to be wrong in the program logic.
yachris Tue 6 Jul 2010
The lesson from 40 (!) years of having "lint" for C is that no one ever uses "lint" for C.
Although it's easy for me to say, since it involves work on your part :-) but PLEASE make the compiler do as much work as possible. It's a one-time expense that has HUGE payoffs, for thousands of people all over the world.
As an example, I'm on a C++ project now, and got caught by the
bug. Java would have flagged this statement as an error, C++ blithely let it go right on by...
tactics Tue 6 Jul 2010
Perhaps a lint tool just needs better integration. Fantom has an official unit test tool. Perhaps a Fantom lint ought to be run by default before running the test suite on a pod.
rfeldman Tue 6 Jul 2010
This enabled-by-default plugin idea hints at an interesting paradigm for compilers.
Suppose the Fantom compiler's role was reimagined to detect only errors. If there's just no way it can build, it will let you know, but everything else is up to the plugins. By default (say, in some compiler config file), the Fantom Warnings plugin would be enabled, but you could remove or replace it as you liked.
This would be an interesting way to combat the problem I've seen where developers ignore all warnings simply because their warning list is mostly filled instances of (language-level) warnings they don't care about. It would be much better for them to be able to selectively disable such warnings by extending the default compiler plugin (or editing some plugin config file), than for them to ignore all warnings altogether.
Of course this would also have the nice side effect of answering the root question here. If the end user has the ability to suppress warnings they consider overzealous, it's relatively harmless to add useful ones to the default plugin as we think of them.
Worth considering, I think...
jodastephen Tue 6 Jul 2010
Certainly Eclipse allows you to turn errors/warnings on/off at a fine level. Allowing that based on the standard compiler would avoid some of the need to create a second compiler.
However, in general I think most warnings are an annoyance, and usually occur in Java to warn about backwards compatibility (generics) or because the compiler writer wasn't determined enough to just make things errors.
rfeldman Tue 6 Jul 2010
And wouldn't it be nice if you could turn those kinds of warnings off? :)
brian Wed 7 Jul 2010
Renamed from Safe invoke operator bug to Expand scope of illegal nullable to non-nullable errors
brian Wed 7 Jul 2010
Promoted to ticket #1127 and assigned to brian
I agree with Stephen, if something shouldn't be done then is should be an error.
Warnings are for things like deprecated methods.
brian Wed 7 Jul 2010
Ticket resolved in 1.0.54
I expanded the number of cases which result in compile time errors. These types of expressions are classified as "always nullable" which means they are never safe to implicitly coerse to non-nullable:
as
operatorAttempting to use any of those expressions where a non-nullable type is expected will now result in a compile time error.