nikhilk / scriptsharp

Script# Project - a C# to JavaScript compiler, to power your HTML5 and Node.js web development.
http://scriptsharp.com
Other
658 stars 182 forks source link

Bug with non-public classes inheriting from public classes #395

Open mattjphillips opened 11 years ago

mattjphillips commented 11 years ago

In the following:

public class A {}
public class B:A {
   public void Foo() {}
}
class C:B {}
(new C()).Foo();

The call to C.Foo() fails at runtime because C's prototype does not contain foo(). I'm still trying to follow the subtleties of ss.module() but roughly speaking it appears to do with (a) the fact that C is not public means it is declared in the "implementation" section of ss.module(), and so is created before B; and (b) the fact that B has a base type means that it gets an anonymous prototype and so its methods are not prototypically inherited by C.

If I make C public, the problem resolves because then C ends up in the exports section and ordered such that B is created first.

I'm happy to take a whack at fixing this but would need some guidance. One possibility is merging the implementations and exports into a single, dependency-sorted list along with a boolean indicating whether each entry is public. I haven't looked at the code generation side to see what the implications are there. Another is to patch up the dependencies on the fly in ss.module() but that seems a lot less elegant, and bug-prone.

Thanks! Matt

mattjphillips commented 11 years ago

Or, come to think of it, if the difference between implementation and exports is purely one of public vs not, then it might be as simple as reversing the order of their type creation (ie, do the exports first, then the implementations). That's based on the observation that one can't create a public class that inherits from a non-public class. Thus any non-public class must be higher in the dependency tree than any public class.

nikhilk commented 11 years ago

Yes, we might be able to leverage that fact, and reverse the order of processing inside the module() function in https://github.com/nikhilk/scriptsharp/blob/cc/src/Core/Scripts/Runtime/TypeSystem.js.

This should be verifiable with just a change in the script, without requiring any compiler changes.

I am still trying to think if there were specific reasons to process internal implementation before public types...

mattjphillips commented 11 years ago

I've verified it fixes the problem. Whether it introduces any new problems, I can't say. Looking at the code in ScriptGenerator.GenerateScript() nothing jumps out at me, but I'm only thinking in terms of classes and class inheritance and not the other sorts of things that might have declaration dependencies.