Skip to content
This repository has been archived by the owner on Aug 24, 2022. It is now read-only.

DCE creates .JS files that won #727

Closed
dbh1997 opened this issue Apr 13, 2015 · 7 comments · Fixed by #733
Closed

DCE creates .JS files that won #727

dbh1997 opened this issue Apr 13, 2015 · 7 comments · Fixed by #733
Labels

Comments

@dbh1997
Copy link

dbh1997 commented Apr 13, 2015

No description provided.

@dbh1997
Copy link
Author

dbh1997 commented Apr 13, 2015

Just started trying this feature. Out of hundreds of enums, 5 are getting created with a BaseType that isn't recognized. The top one is an example of one that is created fine, the second one is created with $.Int32. Could be that either should work and DCE did something to $.Int32 (although that seems unlikely as it appears in numerous other places).

I've not yet figured out what these 5 enums have in common that the ones which are working don't have. Though I would go ahead and open a defect in case somebody already knows what this is. I will continue to research some more as well.

JSIL.MakeEnum(
{
FullName: "Sketch.Interface.DeckFramingEnums+EndBoardFraming",
BaseType: $asm01.TypeRef("System.Int32"),
IsPublic: false,
IsFlags: false,
},
{
None: 0,
Ledger: 1,
Rim: 2,
Beam: 3,
}
);

/* enum Sketch.Interface.StaircaseFramingEnums+ConstructionType */

JSIL.MakeEnum(
{
FullName: "Sketch.Interface.StaircaseFramingEnums+ConstructionType",
BaseType: $.Int32,
IsPublic: false,
IsFlags: false,
},
{
None: 0,
Framed: 1,
}
);

@iskiselev
Copy link
Member

Can you provide part of source code that enough to reproducing? In any case, I don't see any reasons, why $.Int32 should not work.

@dbh1997
Copy link
Author

dbh1997 commented Apr 13, 2015

Still looking at. For whatever it’s worth, with DCE turned back off, it then has BaseType: $asm01.TypeRef("System.Int32"), and manually replacing it with $.Int32 results in the same JS exception. So, it’s not an allowable Basetype – not in that form at least – don’t know enough about Javascript to comment specifically but it does appear to have been intentional. So my guess is there’s a path through the logic that picks the short form that can be used most places, but not here.

Other things that might be relevant…

These are enums inside of static classes as well (but so are others that do work.)
Attempting to inherit from :uint results in $.UInt32 (which also doesn’t work)
Manually replacing them in the .JS code does work (and it moves onto the next issue – which I’m attempting to narrow as well).

I will attempt to extract the enums into a stand alone test case and see if I can get something relatively straight forward to reproduce.

From: Igor Kiselev [mailto:notifications@github.com]
Sent: Monday, April 13, 2015 1:15 PM
To: sq/JSIL
Cc: Harris, Dave
Subject: Re: [JSIL] DCE creates .JS files that won (#727)

Can you provide part of source code that enough to reproducing? In any case, I don't see any reasons, why $.Int32 should not work.


Reply to this email directly or view it on GitHubhttps://github.com//issues/727#issuecomment-92466860.


Xactware's opt-in mailing list allows you to receive Xactware News that is of interest to you. Visit my.xactware.com today to join or to update your email preferences!


This email is intended solely for the recipient. It may contain privileged, proprietary or confidential information or material. If you are not the intended recipient, please delete this email and any attachments and notify the sender of the error.

@dbh1997
Copy link
Author

dbh1997 commented Apr 13, 2015

Looks like having a an enum in a class is good enough to reproduce it. A.B fails, C works. As a side note Both fail if not passed through a function (with a different error - Memoized value is undefined – presumption being that using it this way keeps it out of the dead code elimination better than just using it straight up).

If I allocate A somewhere in the code (A a = new A()), then the issue also goes away. Presumably it decides that A is used, which runs the elimination code differently – which probably explains why the static classes are more problematic…

namespace SketchJS
{
public class A
{
public enum B
{
None = 0,
Framed = 1,
}
}

public enum C
{
    None = 0,
    Framed = 1,
}


public static class Program
{
    public static void Main(string[] args)
    {
        var start = DateTime.Now;
        Run();
        var end = DateTime.Now;
        Console.WriteLine(end - start);
    }

    public static void Run()
    {

#if true
A.B b1 = A.B.Framed;
A.B b2 = A.B.None;
Func1(b1, b2);
#else
C c1 = C.Framed;
C c2 = C.None;
Func1(c1, c2);

#endif

        Console.WriteLine("Ending...");
    }

#if true
private static void Func1(A.B c1, A.B c2)
{
if (c1 == c2)
Console.WriteLine("Equals");
else
Console.WriteLine("Not Equals");
}
#else
private static void Func1(C c1, C c2)
{
if (c1 == c2)
Console.WriteLine("Equals");
else
Console.WriteLine("Not Equals");
}
#endif
}

public class Runner
{
    Runner()
    {
    }

    public void Run()
    {
        Program.Run();
    }

}

}

From: Igor Kiselev [mailto:notifications@github.com]
Sent: Monday, April 13, 2015 1:15 PM
To: sq/JSIL
Cc: Harris, Dave
Subject: Re: [JSIL] DCE creates .JS files that won (#727)

Can you provide part of source code that enough to reproducing? In any case, I don't see any reasons, why $.Int32 should not work.


Reply to this email directly or view it on GitHubhttps://github.com//issues/727#issuecomment-92466860.


Xactware's opt-in mailing list allows you to receive Xactware News that is of interest to you. Visit my.xactware.com today to join or to update your email preferences!


This email is intended solely for the recipient. It may contain privileged, proprietary or confidential information or material. If you are not the intended recipient, please delete this email and any attachments and notify the sender of the error.

@iskiselev
Copy link
Member

Will look on it in next few days - have a lot of work right now.

On Mon, Apr 13, 2015 at 3:11 PM, dbh1997 notifications@github.com wrote:

Looks like having a an enum in a class is good enough to reproduce it. A.B
fails, C works. As a side note Both fail if not passed through a function
(with a different error - Memoized value is undefined – presumption being
that using it this way keeps it out of the dead code elimination better
than just using it straight up).

If I allocate A somewhere in the code (A a = new A()), then the issue also
goes away. Presumably it decides that A is used, which runs the elimination
code differently – which probably explains why the static classes are more
problematic…

namespace SketchJS
{
public class A
{
public enum B
{
None = 0,
Framed = 1,
}
}

public enum C
{
None = 0,
Framed = 1,
}

public static class Program
{
public static void Main(string[] args)
{
var start = DateTime.Now;
Run();
var end = DateTime.Now;
Console.WriteLine(end - start);
}

public static void Run()
{
#if true
A.B b1 = A.B.Framed;
A.B b2 = A.B.None;
Func1(b1, b2);
#else
C c1 = C.Framed;
C c2 = C.None;
Func1(c1, c2);

#endif

Console.WriteLine("Ending...");
}

#if true
private static void Func1(A.B c1, A.B c2)
{
if (c1 == c2)
Console.WriteLine("Equals");
else
Console.WriteLine("Not Equals");
}
#else
private static void Func1(C c1, C c2)
{
if (c1 == c2)
Console.WriteLine("Equals");
else
Console.WriteLine("Not Equals");
}
#endif
}

public class Runner
{
Runner()
{
}

public void Run()
{
Program.Run();
}

}

}

From: Igor Kiselev [mailto:notifications@github.com]
Sent: Monday, April 13, 2015 1:15 PM
To: sq/JSIL
Cc: Harris, Dave
Subject: Re: [JSIL] DCE creates .JS files that won (#727)

Can you provide part of source code that enough to reproducing? In any
case, I don't see any reasons, why $.Int32 should not work.


Reply to this email directly or view it on GitHub<
https://github.com/sq/JSIL/issues/727#issuecomment-92466860>.


Xactware's opt-in mailing list allows you to receive Xactware News that is
of interest to you. Visit my.xactware.com today to join or to update your
email preferences!


This email is intended solely for the recipient. It may contain
privileged, proprietary or confidential information or material. If you are
not the intended recipient, please delete this email and any attachments
and notify the sender of the error.


Reply to this email directly or view it on GitHub
#727 (comment).

@dbh1997
Copy link
Author

dbh1997 commented Apr 13, 2015

Thanks. I should be able to work around it for now by forcing an allocation of each (or possibly calling a static function) – or possibly whitelisting these.

From: Igor Kiselev [mailto:notifications@github.com]
Sent: Monday, April 13, 2015 4:13 PM
To: sq/JSIL
Cc: Harris, Dave
Subject: Re: [JSIL] DCE creates .JS files that won (#727)

Will look on it in next few days - have a lot of work right now.

On Mon, Apr 13, 2015 at 3:11 PM, dbh1997 <notifications@github.commailto:notifications@github.com> wrote:

Looks like having a an enum in a class is good enough to reproduce it. A.B
fails, C works. As a side note Both fail if not passed through a function
(with a different error - Memoized value is undefined – presumption being
that using it this way keeps it out of the dead code elimination better
than just using it straight up).

If I allocate A somewhere in the code (A a = new A()), then the issue also
goes away. Presumably it decides that A is used, which runs the elimination
code differently – which probably explains why the static classes are more
problematic…

namespace SketchJS
{
public class A
{
public enum B
{
None = 0,
Framed = 1,
}
}

public enum C
{
None = 0,
Framed = 1,
}

public static class Program
{
public static void Main(string[] args)
{
var start = DateTime.Now;
Run();
var end = DateTime.Now;
Console.WriteLine(end - start);
}

public static void Run()
{
#if true
A.B b1 = A.B.Framed;
A.B b2 = A.B.None;
Func1(b1, b2);
#else
C c1 = C.Framed;
C c2 = C.None;
Func1(c1, c2);

#endif

Console.WriteLine("Ending...");
}

#if true
private static void Func1(A.B c1, A.B c2)
{
if (c1 == c2)
Console.WriteLine("Equals");
else
Console.WriteLine("Not Equals");
}
#else
private static void Func1(C c1, C c2)
{
if (c1 == c2)
Console.WriteLine("Equals");
else
Console.WriteLine("Not Equals");
}
#endif
}

public class Runner
{
Runner()
{
}

public void Run()
{
Program.Run();
}

}

}

From: Igor Kiselev [mailto:notifications@github.com]
Sent: Monday, April 13, 2015 1:15 PM
To: sq/JSIL
Cc: Harris, Dave
Subject: Re: [JSIL] DCE creates .JS files that won (#727)

Can you provide part of source code that enough to reproducing? In any
case, I don't see any reasons, why $.Int32 should not work.


Reply to this email directly or view it on GitHub<
https://github.com/sq/JSIL/issues/727#issuecomment-92466860>.


Xactware's opt-in mailing list allows you to receive Xactware News that is
of interest to you. Visit my.xactware.com today to join or to update your
email preferences!


This email is intended solely for the recipient. It may contain
privileged, proprietary or confidential information or material. If you are
not the intended recipient, please delete this email and any attachments
and notify the sender of the error.


Reply to this email directly or view it on GitHub
#727 (comment).


Reply to this email directly or view it on GitHubhttps://github.com//issues/727#issuecomment-92514966.


Xactware's opt-in mailing list allows you to receive Xactware News that is of interest to you. Visit my.xactware.com today to join or to update your email preferences!


This email is intended solely for the recipient. It may contain privileged, proprietary or confidential information or material. If you are not the intended recipient, please delete this email and any attachments and notify the sender of the error.

@kg
Copy link
Member

kg commented Apr 19, 2015

Able to reproduce this here too. Might find time to fix it.

iskiselev added a commit to iskiselev/JSIL that referenced this issue Apr 21, 2015
iskiselev added a commit to iskiselev/JSIL that referenced this issue Jul 16, 2015
@kg kg added the Bug label Jul 17, 2015
@kg kg closed this as completed in #733 Jul 25, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants