eclipse-archived/ceylon

behavior of 'exists' with native refs in dynamic blocks on JS

Closed this issue · 16 comments

I can't believe this:

shared void run() {
    dynamic {
        something = null;
        print(something exists);
    }
}

This program blows up with:

ceylon.language::Exception "Undefined or null reference: something"

That's just completely wrong.

It compiles to:

function run(){
  something=null;
  m$beo.print(m$beo.nn$((typeof something==='undefined'||something===null?m$beo.throwexc(m$beo.Exception("Undefined or null reference: something"),'5:14-5:22','main.ceylon'):something)));
}

m$beo is the language module, and I assume nn$ is the nonnull function. It looks like the compiler wraps the occurrence of something in a generic existence check regardless of the expression it appears in.

Apparently this is only the case for toplevel dynamic refs. Dynamic member refs work correctly.

I guess what's really broken is that the validation should not include ||something===null, not the exists operator.

In my opinion, undefinedThing exists should evaluate to false, just like it does with member access.

Thanks, trying it out now.

Hrrrm, @chochos, I tried this:

dynamic {
    print(yyyyy exists);
}

And I got:

/Users/gavin/jstest/modules/jstest/1.0.0/jstest-1.0.0.js:26
m$1.print(m$1.nn$(yyyyy));
                  ^

ReferenceError: yyyyy is not defined
    at run (/Users/gavin/jstest/modules/jstest/1.0.0/jstest-1.0.0.js:26:19)

There's something I'm not getting here.

Even this program blows up:

    dynamic {
        yyyyy = null;
        print(yyyyy);
    }

The print() statement results in this code:

m$1.pndo$((typeof yyyyy==='undefined'||yyyyy===null?m$1.throwexc(m$1.Exception("Undefined or null reference: yyyyy"),'20:14-20:18','run.ceylon'):yyyyy));

It seems to me that we don't need that null check when passing assigning a dynamic value to a parameter of type Anything. In general, null is assignable to any optional Ceylon type.

print(yyyy exists) blows up at runtime if yyyy is undefined, nothing I can do about that, that's native JS error.

Initially you asked me to catch that error and throw a proper exception, which is what all the other code is for. But now I don't generate that runtime check for the exists, so the error you get is a native one.

I had mentioned earlier something about removing the null check in that validation, but it didn't solve the original issue.

OK, let's step back a bit so I can understand.

  1. I thought it was possible to pass an undefined around in JS? Is it not? You get an exception every time you try to do anything with it other than a typeof?

  2. If that's the case, then one solution would be to do this whenever referencing a toplevel dynamic value:

     (typeof yyyyy==='undefined' ? null : yyyyy)
    

    this would transform an undefined into a null.

  3. But there is actually a worse bug above. In my program yyyyy isn't undefined! It's merely null, and print() accepts null. So blowing up is incorrect there regardless of what we do about undefined values.

I thought it was possible to pass an undefined around in JS? Is it not? You get an exception every time you try to do anything with it other than a typeof?

No, you can pass around undefined values, but referencing an undeclared variable outside of typeof is a ReferenceError. You can try it out in your browser console (Ctrl+Shift+K):

» yyyy
✘ ReferenceError: yyyy is not defined [Learn More]
» yyyy = undefined
← undefined
» yyyy
← undefined

(Here, I’ve set the global variable yyyy, but you could also assign to var yyyy or let yyyy for the same results.)

You are conflating two different issues here.

First of all, no, you can't just pass an undefined top-level declaration around in JS (members are ok).

About the program that assigns yyy=null and then does print(yyy), that one blows up because the check for null/undefined is still in place. Changing that validation to what you mention would get rid of that issue, but this changes behavior and will break tests (but we were discussing on gitter that now's the perfect time for that, right?)

this changes behavior and will break tests (but we were discussing on gitter that now's the perfect time for that, right?)

Right, now is the time.

Alright, I had a little time to reflect. Let's consider this program:

dynamic {
    if (exists n = navigator) {
        print(n.language);
    }
}

I think it would be very nice if that worked correctly on node.js, where navigator is null (printing nothing, but not blowing up).

On the other hand, I guess I could live with the above program blowing up with a native error about undefined navigator, as long as I could write something like this instead:

dynamic {
    try {
        print(navigator.language);
    }
    catch (Throwable err) {}
}

I don't love this second way, but it would at least let me write useful code.

Alright, now let's consider something different:

void foo(Anything any) {}
void bar(Object thing) {}

dynamic {
    whatever = null;
    foo(null);  //ok
    bar(null);  //error
}
  1. I think it's clear that the line marked "ok" should run without problem (that's not the case right now).
  2. On the other hand, I think it would be much better if the line marked "error" blew up with a meaningful null pointer exception. The body of bar() shouldn't ever observe a null thing.

But the two cases are very different, @chochos, and I think I've failed to communicate what I wanted here.

This is now almost done. There is one tiny thing left, to make if (exists navigator) behave the same as if (navigator exists), by hacking ConditionGenerator.specialConditionRHS() to call BmeGenerator.generateBme().

OK, fixed now.