sq / JSIL

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

Incorrect type substitution for typeof(this) #961

Open iskiselev opened 8 years ago

iskiselev commented 8 years ago

Test case:

using System;

public static class Program
{
    public static void Main(string[] args)
    {
        var str = GetValue().ToString("P2");
        Console.WriteLine(str);
    }

    private static decimal GetValue()
    {
        return 10.35m;
    }
}

Translated result:

var str = (JSIL.Reference.Of($asm01.System.Decimal).__Type__.__PublicInterface__.$ToString($thisType.GetValue(), "P2", null));

JSIL uses next proxy for replace ToString for numeric types:

        [JSReplacement("$typeof(this).__PublicInterface__.$$ToString($this, $format, null)")]
        public string ToString (string format) {
            throw new InvalidOperationException();
        }

So, $typeof(this) calculated type was JSIL.Reference<decimal> instead of decimal. At the same time, next: var str = (10.35m).ToString("P2"); work as expected. Problem also go away if we'll use double instead of decimal.

Some investigations: ILBlockTranslator.HandleJSReplacement receives JSResultReferenceExpression as thisExpression. Problem could be hidden if we'll change $typeof(this) with $etypeof(this). Unfortunately, I'm not sure how it fix correct.

kg commented 8 years ago

ToString on T& should always be equivalent to T, so I'm not sure anything is wrong with the inference here. The way the typeof works in the replacement is probably not helpful under any circumstances.

On March 29, 2016 2:03:28 PM PDT, Igor Kiselev notifications@github.com wrote:

Test case:

using System;

public static class Program
{
   public static void Main(string[] args)
   {
       var str = GetValue().ToString("P2");
       Console.WriteLine(str);
   }

   private static decimal GetValue()
   {
       return 10.35m;
   }
}

Translated result:

var str =
(JSIL.Reference.Of($asm01.System.Decimal).__Type__.__PublicInterface__.$ToString($thisType.GetValue(),
"P2", null));

JSIL uses next proxy for replace ToString for numeric types:

[JSReplacement("$typeof(this).__PublicInterface__.$$ToString($this,
$format, null)")]
       public string ToString (string format) {
           throw new InvalidOperationException();
       }

So, $typeof(this) calculated type was JSIL.Reference<decimal> instead of decimal. At the same time, next: var str = (10.35m).ToString("P2"); work as expected. Problem also go away if we'll use double instead of decimal.

Some investigations: ILBlockTranslator.HandleJSReplacement receives JSResultReferenceExpression as thisExpression. Problem could be hidden if we'll change $typeof(this) with $etypeof(this). Unfortunately, I'm not sure how it fix correct.


You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/sq/JSIL/issues/961

  • kg (mobile)
iskiselev commented 8 years ago

Same problem will happen for argument type:

using JSIL.Meta;
using JSIL.Proxy;
using System;

public static class Program
{
    public static void Main(string[] args)
    {
        if (Helper.RunMe(new MyStruct()) == Helper.RunMe(GetValue()))
        {
            Console.WriteLine("true");
        }
    }

    private static MyStruct GetValue()
    {
        return new MyStruct();
    }
}

public struct MyStruct
{
}

public static class Helper
{
    [JSReplacement("\"/*$typeof(a)*/\"")]
    public static string RunMe(MyStruct a)
    {
        return string.Empty;
    }
}

Translation:

if ("/*$T00().__Type__*/" == "/*JSIL.Reference.Of($asm00.MyStruct).__Type__*/") {
      $T04().WriteLine("true");
    }

So, in generic $typeof(arg) return JSIL.Reference<real_type> if real_type is structure and arg is function call result.

kg commented 8 years ago

Yeah, I think the appropriate fix is for typeof(T&) to always produce the equivalent of typeof(T)

iskiselev commented 8 years ago

It would be incorrect for arguments passed by ref. So for string RunMe(ref MyStruct a):