Page 1 of 2

nil checking

PostPosted: Tue Jan 08, 2013 8:51 pm
by nerdzero
I haven't done much digging into the root cause, but this program...

class NilTesting

def main
try
.case1
catch
print "case1 crashed"

try
.case2
catch
print "case2 crashed"

def case1
nilableList as List<of String>? = nil
print nilableList.count

def case2
if false
nonNilList = List<of String>()
print nonNilList.count


compiles, runs, and outputs...

Code: Select all
case1 crashed
case2 crashed


Does nil checking work with generics? Case 2 is kind of a corner case but I would expect Case 1 to get caught by the compiler.

Re: nil checking

PostPosted: Wed Jan 09, 2013 12:09 am
by hopscc
What did you expect to be caught?
you're declaring a local variable as nilable and assigning nil to it
(? sufffix on Type indicates nilable).
SFAIKnow theres no issue with nilability on Generics not working..


the variables are both nil where you dereference to get count:
case1 cos you explicitly initialised it nil
case2 cos it was implicitly set to nil when declared and never initialised ( the false test stepping over the ctor call)

capture and print the Exception and you can see...
class NilTesting

def main
try
.case1
catch e as Exception
print "case1 crashed", e

try
.case2
catch e1 as Exception
print "case2 crashed", e1

def case1
nilableList as List<of String>? = nil
print nilableList.count

def case2
if false
nonNilList = List<of String>()
print nonNilList.count

Code: Select all
case1 crashed System.NullReferenceException: Object reference not set to an instance of an object.
   at NilTesting.Case1() in c:\Users\hops\src\cobra\nilck.cobra:line 16
   at NilTesting.Main() in c:\Users\hops\src\cobra\nilck.cobra:line 5
case2 crashed System.NullReferenceException: Object reference not set to an instance of an object.
   at NilTesting.Case2() in c:\Users\hops\src\cobra\nilck.cobra:line 21
   at NilTesting.Main() in c:\Users\hops\src\cobra\nilck.cobra:line 10


In case 2 even though in cobra you are using and initialising the variable in the if block
code gen hoists the declaration of the variable out to the enclosing box block - its just the initialisation code thats done in the scope of the if-then ( so it has its default value for a reference (nil))

compile with -kif and look at the generated C# code for the uses of your variables to see whats being done (on your behalf :) )

Re: nil checking

PostPosted: Wed Jan 09, 2013 9:06 am
by nerdzero
Well, I guess I don't see why it doesn't force me to check that those instances are not nil before accessing their members. Doesn't the compiler have enough information to consider these to be errors?

Re: nil checking

PostPosted: Wed Jan 09, 2013 5:34 pm
by Charles
Generics aren't relevant here. Instead of "List<of String>?" and .count, you could have used "Object?" and .getHashCode, and the result would have been the same.

The question still remains why Cobra doesn't flag these obvious errors. It's a legit question.

I was deterred by two things:

1) For instance variables that have nilable types, should an unguarded "_foo.bar" generate an error (or even a warning)? It seems that would get annoying really fast.

2) For local vars, I think it would be more helpful than hurtful, but not until Cobra has thorough and flawless code flow analysis. Which it does not.

I expect we'll cover case 2) in the long run, but case 1) is still an open question.

What Cobra does right now is check if you are passing a nilable type where a non-nilable is expected. This has been helpful and not too annoying.

Re: nil checking

PostPosted: Wed Jan 09, 2013 8:42 pm
by nerdzero
non-nilable and nilable types are super helpful and I've come to rely on them. I guess I sometimes forget there's still some work to do be done on a few things. And I agree regarding class variables. I think a warning would usually be noise in that case. But for local variables, it would be nice to get a hard error.

I guess the only question I still have then is what is the actual type of the nonNilList variable in the example code I originally posted? Is it a nilable type and if not, should it be?

Re: nil checking

PostPosted: Thu Jan 10, 2013 8:08 am
by hopscc
Interesting collision between type inference and code flow and user expectation.

I'd expect nonNilList to be nonnilable since it
  • was not explicitly declared nilable
  • type was inferred from a (non nilable) assignment (even though the assignment doesnt actually happen at runtime)

But thats also possibly because I know the codegen implementation hoists up declarations and it has to Type them to something..

If I was expecting cobra to have execution flow dynamic declaration of variables ( but also with declaration hoisting which would be reasonable from observation of scoping elsewhere ) I'd expect nonNilList to be nil ( since at runtime it wasnt explicitly initialised) but its still not declared/inferred nilable so it should throw an error ( at compile time) (where - first deref??)
but to check that it needs good flow analysis to be done...

OTOH - you get a runtime error at execution indicating much the same thing.

Oh well.
Takeaways:
1) Dont do that. - explicitly type or explicitly initialise at method top level
2) Remember that there will be an implicit declaration and it will be done either from a provided
explicit type declaration before/at first use or from inferring type at first assignment (regardless of flow of control).
3) Dont do that. :)

I have to confess that the first few times I looked at/used cobra code it was a little surprising to have variables whose first use(assignment) was in a subsidiary block be correctly typed to that assignment AND persist outside (and even prior to) the block...
(unexpected hangover from expectations from other block scoped languages).
Once I realised the decl hoisting was how the implementation worked its surprising how that behaviour became natural and expected.
- all local variables exist at method top level scope no matter where you may assign them into existence.

Re: nil checking

PostPosted: Thu Jan 10, 2013 10:37 am
by nerdzero
hopscc wrote:Interesting collision between type inference and code flow and user expectation.

Yep, this nails it. I knew about variable declaration "hoisting" but the first time I encountered it I was also surprised that it worked that way. This was a pleasant surprise and I liked it.

I would also expect the inferred type to be non-nilable but this doesn't seem to make sense since it will be nil when the method starts. Type inference isn't always obvious but should always be correct. So, I think it should be a nilable type.

I don't think this is a "don't do that" scenario because it's pretty convenient. Seems like a good candidate for a koan. Something like this perhaps...

class LessonVariableScopeAndType

def acceptsNonNilable(nonnilable as Object)
print "Enlignment achieved"

def main

if true
castMe = Object()

# change the next line
.acceptsNonNilable(castMe)


If 'castMe' where inferred as an Object? then that code wouldn't compile unless you passed in 'castMe to !' Admittely, that is totally non-obvious from the provided example code. Also, right now, castMe is inferred as an Object so the code compiles.

Re: nil checking

PostPosted: Fri Jan 11, 2013 10:44 pm
by hopscc
I think the direction the compile is erring here is still correct ( nonnilable).
(local) Variables are nonNilable unless explicitly declared or initialised to be nilable.

Yes (by implementation ) there is an area between implicit decl and overt use where the variable (reference) may have a nil value but it doesnt matter because the variable isnt being set till first use and cant be used in that window
since access before use will generate a diagnostic.

I would still say that this construct is a 'dont do this'.
For a construction like this it should be written with some form of (explicit) initialisation unconditionally so that there are no code paths that result in an unset (nil) variable before use.
class LessonVariableScopeAndType

def acceptsNonNilable(nonnilable as Object)
print "Enlignment achieved"

def main
# castMe implicitly declared here

castMe = NullObject() # 1) castMe unconditionally initialised to null object here.
if false
castMe = Object()

#without 1) castMe can be nil/unset here which breaks nilability invariant going on
.acceptsNonNilable(castMe)


Leaving aside nilability questions its the same issue as if the variable was primitive/value and could not ever be 0/unintialised.
def main
castMe as int # typed but not initialised
if false
castMe = 999

#invariant chk below
assert castMe <> 0 # not unintialised - can fail
.acceptsNonNilable(castMe)


wrt cobra, For correction two possibilities (sans full flow analysis)
1 Initialise all local nonnilable refs to an innocuous (compiler provided) static (non nil) Null Object (compiler gen unInitObject = Object())
cons - most of time a wasted assignment as first cobra code ref will set it to something sensible, performance impact.
2) Perhaps better
In one of the binding phases, walk the AST between (implicit) point of dcl and first use outside subordinate blocks and emit a diagnostic if variable used without being set (noting that nonilable needs to be explicitly assigned before use for all codepaths)
This would detect the above caseand force correction (with explicit typing or an explicit correcting unconditional initial assignment)


Re charles note 1) checking unguarded nilable refs.
Some languages ( even with no notion of nil tracking) have a notion of a null checked deref (rather than doing it regardless and
throwing a NullRef at runtime)

It adds checks/boilerplate so that a nil reference returns nil for the expression rather than just barfing at runtime
( I vaguely recall this as named an 'elvis operator' tho thats perhaps more with data derefs, nil coalescing)

say ?. is the checked deref operator

foo.bar = nil # somehow
...
x = foo.bar?.baz # derefed
# executes fine but
assert x is nil # afterward


I dont see how this as particularly helpful though, as it seems its only deferring the point where a nil reference bites yo onna butt.
It does indicate tho that the writer was aware of the possibility of a nil in the deref chain

Re: nil checking

PostPosted: Sun Jan 13, 2013 12:15 pm
by nerdzero
Yes, but when I ran into this in my own code it was not intentional. I had indented only some code under an if statement which included the initialization of a local var to a nonnilable list.

Why is it correct to infer a nonnilable type if the variable is implicitly initialized to nil when the method starts? Aren't you more likely to catch this potential bug if it were inferred as nilable since when you tried to pass it into something that accepts only non-nilable you'd be like "wtf? oh...."

Re: nil checking

PostPosted: Sun Jan 13, 2013 2:40 pm
by Charles
1)
Re: "1 Initialise all local nonnilable refs to an innocuous (compiler provided) static (non nil) Null Object (compiler gen unInitObject = Object())" ... this won't work due to things like typing and initializers that require arguments. For example,
if condition
button = GUI.Button('click me', ref .onClickMe)

And there could be unintended side effects if you tried to automatically generate an instantiation since initializers may take any kind of actions (scan the hard drive, create a window on screen, write a file, create other objects that do these things, etc.).

2)
Re: the unary, postfix operator '?', it allows you to tighten up code:
if obj.foo
if obj.foo.bar
print obj.foo.bar.baz
# vs.:
print obj.foo?.bar?.baz

I'm worried about the possible syntactic ambiguity with "a ? .b". Other languages deal with this by using ?? for nil-coalesce (instead of the single ?). Another potential solution would be to require spaces around the nil-coalesce operator, though we don't have any rules like that so far. Another thought is to use ! if we think that makes sense. Although ! is already reserved, no one uses it afaik.


3)
Regarding:
if true
foo = Foo()
.takesNonNilFoo(foo)

The correct behavior should be an inferred type of Foo (non-nilable) and an error that it was used uninitialized.

It should be an error if you try to use a non-nilable reference variable that might not be initialized.

Maybe it should also be an error for value variables as well. That's debatable.

I don't want to change the inference to nilable because I would just end up changing it back when the code flow analysis and error checking improved.