Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

null is not properly handled in generic static extensions #4434

Closed
kevinresol opened this issue Jul 23, 2015 · 12 comments
Closed

null is not properly handled in generic static extensions #4434

kevinresol opened this issue Jul 23, 2015 · 12 comments

Comments

@kevinresol
Copy link
Contributor

using Test;

class Test {
    static function main() {
        var i = 1.f();
        var n = null.f();
    }

    public static function f<T>(d:T):T return d;
}

generated:

(function (console) { "use strict";
var Test = function() { };
Test.main = function() {
    Test.f(1);
    null.f();
};
Test.f = function(d) {
    return d;
};
Test.main();
})(typeof console != "undefined" ? console : {log:function(){}});

note that null.f() is expected to be Test.f(null)

@Simn
Copy link
Member

Simn commented Jul 23, 2015

This is working as designed because null has an unknown type and thus results in an unknown field access. Static extensions are only resolved if the type is known.

@Simn Simn closed this as completed Jul 23, 2015
@kevinresol
Copy link
Contributor Author

but then why it compiles?

@andyli
Copy link
Member

andyli commented Jul 23, 2015

Because currently, the following compiles:

class Test {
    static function main():Void {
        null.whatever();
    }
}

It is not related to static extension.

Think of null being a Dynamic var.

@kevinresol
Copy link
Contributor Author

IMO that doesn't make sense, null.whatever should fail at compile time.

@andyli
Copy link
Member

andyli commented Jul 23, 2015

I remember that was experimentally forbidden recently. But that caused problems since it is often to pass null to inline functions resulting in compilation error.
@Simn can we detect null.something and error before inlining?

@back2dos
Copy link
Member

I agree with @kevinresol. Field access to null should not intrinsically resolve to anything. It could either error right there, or proceed with normal resolution, which (at least in my naive mind) would make static extension work.

@Simn
Copy link
Member

Simn commented Jul 23, 2015

Are we talking about null in particular or unknown types in general?

@andyli
Copy link
Member

andyli commented Jul 23, 2015

For me, just null. I'm not sure about the effect of forbiding field access of general unknown types.

@Simn
Copy link
Member

Simn commented Jul 23, 2015

It would break all of Nicolas' code so... ;)

I've already tried disallowing explicit null stuff but that lead to a mess when inlining got involved.

@ncannasse
Copy link
Member

field access does not depend on the value but of the type. and null is Unkown. I don't see any reason to make an exception here, given I fail to see how it actually happens in "real world" code ?

@kevinresol
Copy link
Contributor Author

I understand that the type vs value argument... but it does happen in real world code
I (naively as @back2dos said) thought the static extension would work because it compiles. So, is it possible to, at least, show some warning message?

jasononeil added a commit to ufront/ufront-mvc that referenced this issue Jul 24, 2015
@jasononeil
Copy link
Contributor

The real world code was my fault... I did know about this limitation of static extension, but forgot it in the moment and the compiler let it pass. That particular line wasn't caught by the unit tests either (though I've fixed it and made sure it is covered now).

So 👍 to some kind of warning if it is possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants