Apparent namespace collision for toplevel object and getter method

[jvasileff]

This:

shared void run(){
    Foo foo = getDefaultFoo();
    print(foo);
}

interface Foo { }

object defaultFoo satisfies Foo { }
Foo getDefaultFoo() => defaultFoo;

results in:

/Users/jvasileff/Desktop/js/simple/modules/com/example/simple/1.0.0/com.example.simple-1.0.0.js:43
function getDefaultFoo(){
                      ^
RangeError: Maximum call stack size exceeded

[chochos]

Damn. The only way to fix this one is by breaking backwards compat... the problem here is that top level objects are accessed as properties, so object foo turns into a getFoo() function in JS. To avoid collisions with a getFoo toplevel method we'd need to rename the foo toplevel object function to something else (probably just foo() since no foo toplevel method can be declared, unless it's aliased; so maybe foo$obj() or something like that will be needed).

[gavinking]

In the JVM we use the trailing _ for that.

[chochos]

Yes, adding a prefix won't be a problem. The problem is with backwards compat (not that there is a vast array of ceylon-js modules being used in the wild tho)

[gavinking]

Well you could continue to provide the unsuffixed name (it's a suffix, not a prefix) as an alias, in the case that there is no clash. That should solve the compatibility problem, right?

[chochos]

good point.

[quintesse]

Are you sure? How about code that uses this module that was compiled with an earlier compiler, so it uses the unsuffixed name. Then in the imported module you add a toplevel with the same name and recompile, it then removes the alias and you've broken older code. Right?

[gavinking]

@quintesse Wrong. Modules are versioned.

[quintesse]

Well, the other way around then. You compile this module that has a collision with a newer compiler. So it generates this suffixed version without an alias and then somebody decides to update their old code to use the new module but they're still using the old compiler. That would break.

[chochos]

whatever route we go, some code will break.

[quintesse]

Which means breaking binary compatibility.

[chochos]

yes that's what I said at the beginning, but I don't see any other way of doing it. The aliasing thing will surely minimize breakage but it's likely to cause trouble in some other scenarios.

[gavinking]

You compile this module that has a collision with a newer compiler. So it generates this suffixed version without an alias and then somebody decides to update their old code to use the new module but they're still using the old compiler. That would break.

I don't understand what you're trying to say here. I can't see how anything would break.

[chochos]

The old compiler would call the object with getObject(), but getObject() is some toplevel method in that other module.

[gavinking]

yes that's what I said at the beginning, but I don't see any other way of doing it. The aliasing thing will surely minimize breakage but it's likely to cause trouble in some other scenarios.

I don't understand.

All we're trying to do is ensure that updating the compiler, without changing the source of either module, does not cause new breakage. If the module that is being depended on is changed, and introduces a new collision, of a type which could previously not occur, then surely its clients could break. But surely in that case it's a whole new version of the module.

[gavinking]

The old compiler would call the object with getObject(), but getObject() is some toplevel method in that other module.

This is impossible, unless the module that is being depended on has changed, in which case it is a new version of that module.

[chochos]

Ah right and the old compiler shouldn't accept that module because it can see it's been compiled with a newer version of the compiler ok

[gavinking]

If you recompile the dependent module with an old version of the compiler, when the module it depends on has been compiled with a new version of the compiler, is will still work because of the alias. Unless the module it depends on has been changed, a name collision has been introduced, and it is thus a new version of the module it depends on. But this is something that is just vanishingly unlikely!

[gavinking]

But this is something that is just vanishingly unlikely!

And note that, all we're seeing in this "vanishingly unlikely" case is the same bug that already exists right now, in the "old", i.e. current compiler! i.e. that the "old" compiler doesn't allow you to have foo and getFoo().

No, we can't retroactively fix bugs in the Ceylon 1.1.0 compiler, but I don't think anyone expects us to. If it's a real problem (it's not) we can release Ceylon 1.1.1 to fix just this bug.

So I think the alias completely solves any reasonable expectation for backward compatibility. Backward compatibility does not mean that bugs in old compiler versions get retroactively fixed by pure magic!

[gavinking]

But there's something else I don't understand here: why do we even have the naming convention getFoo() in JS? I thought that now with Object.defineProperty() that this was totally obsolete. Wouldn't we just call the toplevel object foo? We don't even need the _ suffix, do we?

[jvasileff]

I'm trying to understand when this is even an issue.

AFAICT, old code can't call into new code without https://github.com/ceylon/ceylon-spec/issues/1134 (will supporting that create additional binary incompatibilities anyway?) An exception of course is the language module, but I imagine there are other solutions for the 20 or so toplevels there.

New code can be made to call into old code by simply using the old convention during compilation. And if the old module is recompiled with the new compiler, the version should be bumped anyway. Support for 1134 would create a problem here, but is this future scenario worth the permanent bloat? Will 1.1.0 modules even be important when 1134 is ready?

And I think it's reasonable to disallow using 1.2.0 modules with the 1.1.0 compiler. Java has never allowed this sort of thing.

[chochos]

Object.defineProperty only works to define members of objects, not on toplevel stuff. So toplevel values and toplevel objects still use this getFoo syntax (so actually this bug affects also toplevel values and getters).

[quintesse]

Backward compatibility does not mean that bugs in old compiler versions get retroactively fixed by pure magic

It also means a new compiler doesn't introduce new bugs without the old compiler noticing it can't compile using the new code.

It doesn't matter if its vanishing small, we all know what "vanishing small" in IT means, it means that at most 3 days after releasing the compiler someone will have found the problem. We'd be breaking the "public interface" of the code. And once the compiler is out there we have different types of code generation but no way to distinguish between them, so there'd be no way to "release Ceylon 1.1.1 to fix just this bug".

And it's easy. Module A uses B.foo, it's the old compiler so it just refers B.foo, no suffixes. Now imagine the following two cases:

a) B gets recompiled with a new compiler. No problem, B.foo now becomes B.foo_ but we also have an alias B.foo added by the new compiler. The developer of A , still using the old compiler, now decides to upgrade to the new B. Because of the alias everything works, but...

b) B gets recompiled with a new compiler and also gets a new toplevel getter called foo. Now we can't have an alias B.foo anymore because it get's used by the new getter. The developer of A , still using the old compiler, now decides to upgrade to the new B. Everything compiles but suddenly his code accesses the wrong foo!

It's a nasty little bug that even if it won't occur often it would be very very hard to spot by the developer of module A.

[quintesse]

And I think it's reasonable to disallow using 1.2.0 modules with the 1.1.0 compiler

That's the point, if you don't bump the version number the 1.1.0 compiler has no way of knowing that the user is trying to use a module compiled with a 1.2.0 compiler! The moment you change your code generation in a way that's not 100% backward compatible you have to bump your binary version number.

[gavinking]

Object.defineProperty only works to define members of objects, not on toplevel stuff.

I understand that.

So toplevel values and toplevel objects still use this getFoo syntax.

But why should they? That seems simply silly!

"So" is not the appropriate word here. The first sentence does not logically entail the second. ;-)

[gavinking]

It also means a new compiler doesn't introduce new bugs without the old compiler noticing it can't compile using the new code.

The new compiler is not introducing any new bug. The bug is one that pre-exists in the "old" (current) compiler.

[gavinking]

We'd be breaking the "public interface" of the code. And once the compiler is out there we have different types of code generation but no way to distinguish between them, so there'd be no way to "release Ceylon 1.1.1 to fix just this bug".

Nonsense. The public API of any bugfree module compiled with the 1.1.0 compiler continues to be supported when recompiled using the 1.2.0 compiler.

[gavinking]

b) B gets recompiled with a new compiler and also gets a new toplevel getter called foo.

And then the module B needs to release a new whole version because its public API has changed.

Now we can't have an alias B.foo anymore because it get's used by the new getter. The developer of A , still using the old compiler, now decides to upgrade to the new B. Everything compiles but suddenly his code accesses the wrong foo!

Right. Because there is a bug in the old compiler. If the developer would have compiled both A and B with the old compiler they would still have run into this same bug. If they need to avoid this bug they need to use the new compiler. Duh.

[chochos]

It's a bug. When we switched from using getter functions to using Object.defineProperty, toplevel objects and values were left behind, so to speak.

[gavinking]

It's a bug.

OK, fine, so we agree. The all I'm saying is we don't even need the underscore.

[quintesse]

Sorry, I'm still not convinced. So what happens if the (new) compiler compiles code that uses a defaultFoo from another module? How does it know whether it should call getDefaultFoo() or the suffixed getDefaultFoo_()? The imported module might be compiled with an older version of the compiler or with the newer, there's no way to know. The aliasing only works for old code calling new code, which we already determined can't happen because of versioning. But new code calling imported code needs to know if the imported code is compiled with a new compiler or with an older one.

[gavinking]

@quintesse I'm certain that @chochos could implement it to use (defaultFoo ? defaultFoo : getDefaultFoo())

[quintesse]

But that's it, you don't know if it's an old module implementing a getter, an old module implementing a toplevel object or a new module implementing both. So suddenly instead of simply saying "give me the native method name for a Ceylon method called foo in version 1.1 of Ceylon" you need to have access to the entire class definition so we can perform all kinds of heuristics. That's exactly what we have binary version numbers for!

[gavinking]

@quintesse huh?!

There are only two cases here:

1. It was compiled with the "old" compiler. Then defaultFoo is undefined, and getDefaultFoo() will be called.
2. It was compiled with the new compiler. Then defaultFoo is defined, and will be used.

[chochos]

Right. With the old compiler there's no defaultFoo. Now, about the old module, of course I know if it's implementing a getter or a toplevel object; that's what the model loader's for. When referring to a toplevel object from an old module, the compiler can generate getDefaultFoo but if it's from a new module then generate defaultFoo.

[quintesse]

Well, you'd have to do what @gavinking said, and generate both with a test, something like:

if (defaultFoo === undefined) { getDefaultFoo(); } else { defaultFoo(); }

And you can never again simply ask "give me the name of the access method for toplevel object defaultFoo in Ceylon 1.1", life will never again be that simple. I just hope this won't come back to bite us in the future.

[gavinking]

@quintesse the workaround is only needed until next time we break compatibility.

[quintesse]

Right. Let's not forget to remove it then ;)

[chochos]

OK so toplevel values are being correctly generated. Now, should an alias be generated on newly compiled modules so that a toplevel value can be accessed both with foo() and getFoo()? I'm not sure this is very useful.

The other way around is the one we obviously need: retrieving toplevel values from older modules with getFoo(), but I'm not sure about the alias in newly compiled modules.

[quintesse]

Is this still necessary in light of #490 ?

[chochos]

Modules compiled with 1.1 have their toplevel values as getFoo() so yes, it's necessary. And I can even use the 1.1 language module to compile stuff with the 1.1.x compiler; not sure if that should be allowed though, but if it is, then we do need the aliases.

[quintesse]

I don't think that 1.1 code can depend on 1.1.1 code, right? If that's true then the 1.1.1 doesn't need the aliases. To guarantee backward compatibility 1.1.1 code can depend on 1.1, but there are no aliases there either. But again I might just be misunderstanding it all.

[chochos]

This just happened to me: I compiled the 1.1.1 js language module, with binary version 7.0 then tried to use to with the 1.1.1 compiler to compile some other stuff, and after modifying the JS model loader to work with 1.1 and 1.1.1, it worked. So technically I was able to use the 1.1 language module with the 1.1.1 compiler to compile new code.

Then, I tried to run (just run) the ceylon.test tests I had compiled before the binary version was bumped, also using the 7.0 binary JS language module. It blew up because the code there looks for toplevel objects in the language module using getFoo() instead of just foo().

In practice, I don't think people will run into this case (it happened to me because I'm in the middle of recompiling stuff between version changes, not all my repos are up-to-date, etc). But I think it's reasonable to expect new code to be compiled still using older binary versions of libraries (hence #490) which will have toplevel values available as getFoo(). So I need to generate new code expecting that case.

To avoid bloated code, I think the best option is to look at the module's binary version to know whether to call a toplevel value with getFoo() or just foo().

[chochos]

Fixed, with no aliasing of toplevel values in new modules, but compiler generates the proper reference to imported toplevel values depending on the binary version of their modules.

[jvasileff]

@chochos - just wondering, will modules compiled with 1.1.0 be able to access toplevels of the 1.1.1 language module? That was the one case I thought of where aliases may be necessary in newly compiled code.

[gavinking]

Excellent, thanks @chochos!

[chochos]

@jvasileff no, modules compiled with 1.1.0 won't be able to access toplevels of any module compiled with 1.1.1. I decided against adding the getFoo aliases for toplevels based on what @quintesse said about causing problems at runtime that will be very hard to debug.

And, I don't see how a module compiled with 1.1.0 would be able to use the 1.1.1 language module, since it already has a reference to the 1.1.0 version.

[quintesse]

since it already has a reference to the 1.1.0 version.

Except that it doesn't, I just now realised, at least on the JVM side we purposely remove the language module reference from the module definition. I'm not sure anymore, @jvasileff seems to have a point. @FroMage, would you care to comment on this?

[chochos]

JS modules have a reference to the language module which includes the version. If you compiled a language module with 1.1.0 then it has a reference to language module v1.1.0