sq / JSIL

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

Work of JSReplacement attribute #424

Open iskiselev opened 10 years ago

iskiselev commented 10 years ago

I once again have a problem with object.Equals (#241) and object.GetHashCode (#240). My initial solution for this problems was create proxy:

    [JSProxy(
        typeof(Object),
        memberPolicy: JSProxyMemberPolicy.ReplaceNone,
        attributePolicy: JSProxyAttributePolicy.ReplaceDeclared
    )]
public abstract class ObjectProxy
    {
        [JSReplacement("JSIL.ObjectEquals($this, $obj)")]
        public new abstract bool Equals(object obj);

        [JSReplacement("JSIL.ObjectHashCode($this)")]
        [JSRuntimeDispatch]
        public new abstract int GetHashCode();
    }

But as I found today, JSIL does not translate methods, that was marked with JSReplacement attribute. So, if class later tries to override any of this method - this override will not work. So, her I have a question - why do we skip translation of methods marked with JSReplacement attribute? Maybe we should disable it or add some optional flag? This attribute is very convenient for transforming instance call with JS static call, that help calling methods on JS simple type, but I still want to translate original method (so that I can use it through reflection or inside JS static helper).

Or maybe we should introduce some additional attribute, that will force JSIL on translation always get method from class and run it with "call"? Or should we just force JSIL to always generate such call for this two methods?

iskiselev commented 10 years ago

And really I have on more question - why such proxy prevents method translation in inherited classes?

iskiselev commented 10 years ago

I found answer question from previous comment - I should use JSProxy.inheritable parameter.

kg commented 10 years ago

JSReplacement is designed to work the way it does. If you want to replace the body with JS, you use Verbatim.

The reason for this is that some method calls themselves have undesirable properties, like the method is overloaded or invoking it creates garbage. A JSReplacement can circumvent a bunch of that. JSReplacement also lets you do slightly gross syntax stuff.

iskiselev commented 10 years ago

Problem, that here I try to rewrite not body, but how this method is called. I try to transform it from instance to static from (for workaround problem with native data types). Now I'm experimenting with new attribute, that will force JSIL generate same method call, as if method has overloads. Maybe it will help me solve virual object methods problems on native JS types.

kg commented 10 years ago

I see. That is an interesting problem. Maybe I could expand JSReplacement so there is a way for it to not suppress translation, and only alter invocation style. Maybe include a meta-variable named $thisFunction that, if used, enables translation and lets you reference the translated function. Not sure how that'd work out...

iskiselev commented 10 years ago

Really, now I'm not sure that JSReplacement is what I want to use for fixing problem. With it I have another problem:

    public class MyClass
    {
        public override bool Equals(object obj)
        {
            return base.Equals(obj);
        }

        public override int GetHashCode()
        {
            return base.GetHashCode();
        }
    }

Here base.Equals will be also replaced with static call, so that I'll have StackOverflow as my recursion never ends.

kg commented 10 years ago

Hm, I bet the current approach breaks that too. Interesting.

iskiselev commented 10 years ago

No, with current JSIL proxy all works good:

        [JSIsPure]
        [JSChangeName("Object.Equals")]
        [JSNeverReplace]
        [JSRuntimeDispatch]
        new public abstract bool Equals (object obj);

        [JSIsPure]
        [JSReplacement("JSIL.ObjectEquals($objA, $objB)")]
        public static bool Equals (object objA, object objB) {
            throw new InvalidOperationException();
        }

It doesn't try to replace instance Equals call, so we have no recursion cycles. It replace only static version of Equals.

kg commented 10 years ago

Oh, ha, I got that right. I bet I messed it up originally.

kg commented 10 years ago

Hm, I'm guessing you want more like

[JSReplacement("MyEquals(typeof(this), this, typeof(T), rhs)")]
new public ??? bool Equals<T> (T rhs)

If such a thing were possible, in order to preserve type information?

iskiselev commented 10 years ago

Looks like I found how I can break recursive loop and really I don't need option to write method with JSReplacement. So now I add this proxies:

    [JSProxy(
    typeof(Object),
    memberPolicy: JSProxyMemberPolicy.ReplaceNone,
    attributePolicy: JSProxyAttributePolicy.ReplaceDeclared,
    inheritable: false)]
    public abstract class Object2Proxy
    {
        [JSReplacement("JSIL.ObjectEquals($this, $obj, true)")]
        public new abstract bool Equals(object obj);

        [JSReplacement("JSIL.ObjectHashCode($this, true)")]
        public new abstract int GetHashCode();
    }

    [JSProxy(
    new[] { 
            typeof(SByte), typeof(Int16), typeof(Int32),
            typeof(Byte), typeof(UInt16), typeof(UInt32),
            typeof(String)
        },
    memberPolicy: JSProxyMemberPolicy.ReplaceNone,
    attributePolicy: JSProxyAttributePolicy.ReplaceDeclared)]
    public abstract class JSNumbersProxy
    {
        [JSReplacement("JSIL.ObjectHashCode($this, true)")]
        public new abstract int GetHashCode();
    }

And here is modification of JSIL.ObjectHashCode:

JSIL.ObjectHashCode = function (obj, instance) {
  var type = typeof obj;

  if (type === "object") {
      if ((instance !== true || obj.__IsInsideHashCode__ === undefined) && obj.GetHashCode) {
          obj.__IsInsideHashCode__ = true;
          var result = (obj.GetHashCode() | 0);
          delete obj.__IsInsideHashCode__;
          return result;
      }

      return JSIL.HashCodeInternal(obj);
  } else {
    // FIXME: Not an integer. Gross.
    return String(obj);
  }
};

JSIL.ObjectEquals = function (lhs, rhs, instance) {
  if ((lhs === null) || (rhs === null))
    return lhs === rhs;
  if (lhs === rhs)
    return true;

  switch (typeof (lhs)) {
    case "string":
    case "number":
      return lhs == rhs;
      break;

    case "object":
        if (instance !== true || lhs.__IsInsideEquals__ === undefined) {

            var key = JSIL.GetEqualsSignature().GetKey("Object_Equals");
            var fn = lhs[key];

            if (fn) {
                lhs.__IsInsideEquals__ = true;
                var result = fn.call(lhs, rhs);
                delete lhs.__IsInsideEquals__;
                return result;
            }
        }

        break;
  }

  return false;
};

So, now I add lhs.IsInsideEquals on first pass to break cycle. It is very dirty for now, but maybe we can find better way later.

kg commented 10 years ago

mutating obj (and using delete) is unacceptable from various perspectives, unfortunately. It might be better to maintain a list or dictionary of objects at global scope (and add/remove obj from it during execution).

iskiselev commented 10 years ago

Seems, that we really discuss in this thread #241 and #240, :( Maybe it is better to return to some of them. Looks like solution now works in most cases. Is it mature enough (if we add global scope dictionary) to be accepted as patch to main branch? (I will write test, that show all solved problems and clean up code little bit, but the main idea will be same). Or should we wait a little bit more and think how we can do it better? Maybe if you implement boxing as you suggested in #349 we will not need this fix at all.

kg commented 10 years ago

I don't think I like the global scope dictionary much either, but I'd probably be willing to accept it after testing it a little. I need to re-familiarize myself with the problems this solves.

I'd rather just implement boxing properly so this goes away.

iskiselev commented 10 years ago

Really even this my solution will work incorrect on some edge cases, so I will not create pull request for it and will wait for proper fixing using boxing.

kg commented 10 years ago

Thank you for doing the research anyway. I appreciate it.

iskiselev commented 10 years ago

I only tried to run some parts of our product, that use Irony (https://irony.codeplex.com/). Now I was able to run it using JSIL. So now we can create script language parser/interpreter, that work on .Net on top of JavaScript :)

kg commented 10 years ago

That's marvelous! One of the things I want to experiment with eventually is providing a simple JS JIT equivalent to Reflection.Emit/LCG; kinda hard to do though, might require moving chunks of codegen to runtime.

iskiselev commented 10 years ago

One more thing to discuss. Today I tried to replace number types long form of TryParse with its' short form, for example:

public static bool TryParse(string s, System.Globalization.NumberStyles style, System.IFormatProvider provider, out double result)

replace with

public static bool TryParse(string s, out double result)

I can easily create proxy, that will create catch it in all number classes using AnyType, but I can't mention this type in JSReplacement. So, here is example, what I have now:

        [JSReplacement("$$jsilcore.System.Double.TryParse($s, $result)")]
        public double TryParse(string s, System.Globalization.NumberStyles style, System.IFormatProvider provider, out double result)
        {
            throw new InvalidOperationException();
        }

and here is what I want:

        [JSReplacement("$$jsilcore.????????.TryParse($s, $result)")]
        public AnyType TryParse(string s, System.Globalization.NumberStyles style, System.IFormatProvider provider, out AnyType result)
        {
            throw new InvalidOperationException();
        }
kg commented 10 years ago

Oh, so you want some sort of meta-expression for typeof(result) that uses the actual type instead of AnyType? That's tricky. I'm not sure I will support that. Better to just provide overloads for each type, I'm afraid.

I'll think about whether it can be done, though.

iskiselev commented 10 years ago

Really now I want to have some meta-expression, that will catch type on which proxy is applied in current time - so not results, but some like $proxiedType. But if you can add some expression for catching AnyType - it else can be value, Something like $typeof(result), $typeof(argument, n).

kg commented 10 years ago

$typeof(argname) is already supported, i think. You might find it in one of the proxies.

But I bet it won't do the right thing for AnyType (maybe it does, though! Give it a try.)

kg commented 10 years ago
                    foreach (var kvp in methodInfo.Parameters.Zip(arguments, (p, v) => new { p.Name, Value = v })) {
                        argsDict.Add(kvp.Name, kvp.Value);
                        argsDict["typeof(" + kvp.Name + ")"] = Translate_TypeOf(kvp.Value.GetActualType(TypeSystem));
                        argsDict["etypeof(" + kvp.Name + ")"] = Translate_TypeOf(kvp.Value.GetActualType(TypeSystem).GetElementType());

This looks like it's probably using the types of the arguments (so it will grab AnyType), but it could be modified to pull the type of the value expression. Maybe 'vtypeof(x)'.

iskiselev commented 10 years ago

Thank you, I already found it. Looks like it works great with AnyType. There was no documentation for it, so I didn't know about it. I found that it should support next syntax: $assemblyof(executing) $etypeof(argname) - here can be any argument name $typeof(argname) - here can be any argument name $typeof(this) $etypeof(this)

I don't understand yet difference of etypeof and typeof. typeof(this) returns null for static method.

kg commented 10 years ago

etypeof is 'element type of', so for typeof(x) === T[], etypeof(x) === T that lets you do some basic handling of generics that manipulate arrays. I forget what it's used for.

iskiselev commented 10 years ago

This methods generates .Net type e.g. $T01(). Type . Maybe we may introduce something, that will output just type public interface. One more stupid question. How can I get type public interface when I have .Net type? So what I should do with $T01(). Type to get just $T01()?

kg commented 10 years ago

The inverses are

x.__Type__.__PublicInterface__ === x
x.__PublicInterface__.__Type__ === x

Hope that's clear.

iskiselev commented 10 years ago

Thank you!

iskiselev commented 10 years ago

Here is what I want at the beginning:

        [JSReplacement("$typeof(result).__PublicInterface__.TryParse($s, $result)")]
        public static AnyType TryParse(string s, System.Globalization.NumberStyles style, System.IFormatProvider provider, out AnyType result)
        {
            throw new InvalidOperationException();
        }
kg commented 10 years ago

I take it that what you've written doesn't work, but only because of AnyType? Is that right? Or is it already working?

This is a good reminder to write a wiki page documenting all the Meta attributes.

iskiselev commented 10 years ago

Just notice - in proxy instance method replace static method. So, both constructions works

        [JSReplacement("$typeof(result).__PublicInterface__.TryParse($s, $result)")]
        public static AnyType TryParse(string s, System.Globalization.NumberStyles style, System.IFormatProvider provider, out AnyType result)
        {
            throw new InvalidOperationException();
        }

        [JSReplacement("$typeof(result).__PublicInterface__.TryParse($s, $result)")]
        public AnyType TryParse(string s, System.Globalization.NumberStyles style, System.IFormatProvider provider, out AnyType result)
        {
            throw new InvalidOperationException();
        }

I'm not sure, if it is good or bad.

It already works good, even with AnyType. JSIL generates proper type instead of AnyType. Next note, typeof(this) returns Void type for static method. I'm not sure if it good. Maybe it is normal, as static method have no this. But in this realization I was unable to use it to catch peroxided type.

kg commented 10 years ago

I'm guessing you'd prefer that typeof(this) be the containing static class? That's probably a better option, I could implement that.

On Thu, Apr 24, 2014 at 2:07 PM, Igor Kiselev notifications@github.comwrote:

Just notice - in proxy instance method replace static method. So, both constructions works

    [JSReplacement("$typeof(result).__PublicInterface__.TryParse($s, $result)")]
    public __static__ AnyType TryParse(string s, System.Globalization.NumberStyles style, System.IFormatProvider provider, out AnyType result)
    {
        throw new InvalidOperationException();
    }

    [JSReplacement("$typeof(result).__PublicInterface__.TryParse($s, $result)")]
    public AnyType TryParse(string s, System.Globalization.NumberStyles style, System.IFormatProvider provider, out AnyType result)
    {
        throw new InvalidOperationException();
    }

I'm not sure, if it is good or bad.

It already works good, even with AnyType. JSIL generates proper type instead of AnyType. Next note, typeof(this) returns Void type for static method. I'm not sure if it good. Maybe it is normal, as static method have no this. But in this realization I was unable to use it to catch peroxided type.

— Reply to this email directly or view it on GitHubhttps://github.com/sq/JSIL/issues/424#issuecomment-41332265 .

iskiselev commented 10 years ago

And one more question - how do you think, which way of changing method signature is best? Is such proxy, as we discussed good enough? Or here is one more example:

    [JSProxy(
        typeof(Convert),
        memberPolicy:JSProxyMemberPolicy.ReplaceNone,
        attributePolicy:JSProxyAttributePolicy.ReplaceDeclared)]
    public abstract class ConvertProxy
    {
        [JSReplacement("$$jsilcore.System.ConvertProxy.ChangeType($value, $conversionType)")]
        public static object ChangeType(object value, Type conversionType, System.IFormatProvider provider)
        {
            throw new InvalidOperationException();
        }
    }

So, what I want - remove formatProvider from signature and use another overload as overload with formatProvider is not yet implemented.

kg commented 10 years ago

In that case if you just call Convert.ChangeType directly, i.e.

[JSReplacement("$$jsilcore.System.Convert.ChangeType($value, $conversionType)")]

it should work.

If typeof(this) is extended to provide the static type, that would let you use it instead of '$$jsilcore.System.Convert', which is much nicer.

kg commented 10 years ago

Here's a swing at some attribute documentation. https://github.com/sq/JSIL/wiki/Meta-Attributes