sq / JSIL

CIL to Javascript Compiler
http://jsil.org/
Other
1.73k stars 242 forks source link

Method from Proxies versus Fields from proxies #769

Open iskiselev opened 9 years ago

iskiselev commented 9 years ago

JSIL adds to type all fields that was defined in Proxies, but skip such methods (so, only methods that was defined in original class will go to output). Why does it work so?

kg commented 9 years ago

Because it's broken, probably. It used to work but I guess I didn't have sufficient test coverage.

I hit some other related bugs in proxies when working on the FNA stuff, I need to fix them.

iskiselev commented 9 years ago

Thanks, so I'll try to write proper DCE analyze as if we could add new methods through proxy (pretty sure it still won't work initially, but at least it will be simpler fix it).

I also have a question - what is "recommended" way to implement proxy if we need to cast it back to original type (to pass it to some other function)? Should we declare proxy as derived type to proxied type? As I understand we also could convert it through object (ProxiedType)(object)this. Do we have any other options?

kg commented 9 years ago

The way proxies should work:

Proxies formally work as an overlay onto the type they proxy. Any reference to the proxy should be translated into a reference to the type it's overlaid onto. As such, you should be able to overlay additional fields, properties, methods and events onto the target type, and they will be compiled as if they were a part of the original type.

In that sense, the proxy shouldn't inherit from anything and it should never be visible once compilation is finished. There are a few bugs I know of right now that break this rule.

iskiselev commented 9 years ago

Ok, lets see an example. Here is source types:

public class SomeClass : Attribute
{
    public void TestMethod()
    {
    }
}

public static class SomeClassHelper
{
    public static void Helper(this SomeClass c)
    {
        Console.WriteLine("SomeClass passed to this method");
    }
}

Let's implement proxy for SomeClass.TestMethod that will run SomeClassHelper.Helper with current instance. I see two options right now:

  1. ProxyClass inherit SomeClass
[JSProxy(typeof(SomeClass))]
public class ProxyClass : SomeClass
{
    public void TestMethod()
    {
        this.Helper();
    }
}
  1. Cast ProxyClass through an object
[JSProxy(typeof(SomeClass))]
public class ProxyClass
{
    public void TestMethod()
    {
        ((SomeClass)(object)this).Helper();
    }
}

Looks like both options are working normal currently. So, my question - what is recommended way to do it? If I missed some other options?

kg commented 9 years ago

If memory serves, the way this SHOULD work is that you define a proxy for the extension method that accepts your proxy type, and then call it. JSILc should swap out the proxy type in the arglist of the extension method proxy, and then the extension method itself will be replaced with a reference to the real one.

(This requires you to make the extension method proxy marked as NeverReplace so it doesn't replace the real extension method.)

I don't know if it works for extension methods, though. I've only done it with normal methods.

iskiselev commented 9 years ago

Cool, I like it! Unfortunately it is broken currently, but let's put it here for reference/future test adding:

using System;
using JSIL.Proxy;

public static class Program
{
    public static void Main()
    {
        new SomeClass().TestMethod();
    }
}

public class SomeClass : Attribute
{
    public void TestMethod()
    {
    }
}

public static class SomeClassHelper
{
    public static void Helper(this SomeClass c)
    {
        Console.WriteLine("SomeClass passed to this method");
    }
}

[JSProxy(typeof(SomeClassHelper))]
public static class SomeClassHelperProxy
{
    [JSNeverReplace]
    public static void Helper(this ProxyClass c)
    {
    }
}

[JSProxy(typeof(SomeClass))]
public class ProxyClass
{
    public void TestMethod()
    {
        this.Helper();
    }
}
iskiselev commented 9 years ago

When I thought about last example I see that it will have problem with identifying source class of Helper method:

using System;

using JSIL.Meta;
using JSIL.Proxy;

public static class Program
{
    public static void Main()
    {
    }
}

public class SomeClass : Attribute
{
    [JSDeadCodeEleminationEntryPoint]
    public void TestMethod()
    {
    }
}

public static class SomeClassHelper
{
    public static void Helper(this SomeClass c)
    {
        Console.WriteLine("SomeClass passed to this method");
    }
}

public static class AnotherClassHelper
{
    public static void Helper(this SomeClass c)
    {
        Console.WriteLine("SomeClass passed to this another method");
    }
}

[JSProxy(new [] {typeof(SomeClassHelper), typeof(AnotherClassHelper)})]
public static class SomeClassHelperProxy
{
    [JSNeverReplace]
    public static void Helper(this ProxyClass c)
    {
    }
}

[JSProxy(typeof(SomeClass))]
public class ProxyClass
{
    public void TestMethod()
    {
        this.Helper();
    }
}

What method from ProxyClass.TestMethod have we called: SomeClassHelper.Helper or AnotherClassHelper.Helper?

Really, I have nearly same problem in DCE when I tried to implement full proxy support there - how can we check what original member reference we have, when we meet proxy member reference?

kg commented 9 years ago

I'm not really sure. Maybe proxies should be restricted to a single type, and to proxy multiple types you need multiple proxies?

iskiselev commented 9 years ago

Probably yes, at least some types of proxies - on other side JSIL currently use multi-type proxies at least for IntegerProxy/NumberProxy. But right now, I'm not sure what is definition of this border line.

kg commented 9 years ago

Maybe if proxies can inherit from other proxies, that solves the issue while keeping the 'specific proxy per specific type' upside

iskiselev commented 9 years ago

@kg, I don't understand your last suggestion :(

kg commented 9 years ago

You'd have Int32Proxy and Int64Proxy, but they both inherit all their actual methods from NumberProxy. Then the method on Int32 is from Int32Proxy so you can map perfectly in either direction without being uncertain where it came from

iskiselev commented 9 years ago

Still don't understand how it solves our problem. It looks that type can be proxy for more than one type while it doesn't reference other proxy types in any form (as field, generic argument, method argument or return type). Probably we should also restrict using any non-single proxy from any other proxy.

All other cases looks fine. We also need introduce resolving original MemberInfo/MemberDefenition by proxied MemberInfo.

iskiselev commented 9 years ago

Looks like I understand you. You suggest something like:

public class NumberProxy
{
  public void ProxyForSomeMethod()
  {
  }
}

[JSProxy(typeof(int))]
public class Int32Proxy : NumberProxy{
}

[JSProxy(typeof(int))]
public class Int64Proxy : NumberProxy {
}

Right now, as I already mention, it is possible to derive proxy type from original type - in this case it will be a little bit easier access protected member of original type - you don't need to declare it in proxy :)

Today I've also think about defining all mappings right on proxy. In my example it could be something like:

[JSProxy(new [] {typeof(SomeClassHelper), typeof(AnotherClassHelper)})]
public static class SomeClassHelperProxy
{
    [JSNeverReplace]
    public static void Helper(this ProxyClass c)
    {
    }
}

[JSProxy(typeof(SomeClass))]
[JSProxyMapping(typeof(SomeClassHelper),typeof(SomeClassHelperProxy))]
public class ProxyClass
{
    public void TestMethod()
    {
        this.Helper();
    }
}

Really I'm still not sure last idea is really possible and I don't like it very much, restriction on references only single-type proxy looks better for me now. Probably I will try to do something on it tomorrow as I have a problem with creating some new proxies (I'm trying to get rid of most BCL classes implementation in Libraries*.js and use all of them translated). So right now I have:

I need to create proxy for Expression.Constant that will still do call to Make, but remove some checks. Right now, it is possible to achieve only with [JSExternal] or using Verbatium inside proxy.