Char based on number

[iskiselev]

Here is PR that changes underling type of System.Char to Number.
It reveals one interesting problem. C#, when call virtual method, emits cal to lowest type - for example:
callvirt instance string System.Char::ToString().
At the same time F#, emits call to highest type - so it will be:
callvirt instance string System.Object::ToString()
Here we have problem - our proxies will replace Char::ToString() with special call, but not Object::ToString(). So, right now this PR breaks translation of char.ToString() method if it was compiled with F# (I moved Issue118_1.fs to FailingTest group).

Probably we can solve this problem later. Probably we should solve it before merging this commit. We could solve it for this particular problem, or we could try to solve it in more generic way (for all proxies) with replacing method call with lowest call much earlier.

[kg]

What do you think the proxy fix would look like?

[iskiselev]

First option that I see, is change MethodReference method in ILBlockTranslator.Translate_Callvirt with appropriate method based on node.Arguments[0].ExpectedType or node.Arguments[0].InferredType.

[iskiselev]

It can be done as separate Transformation and it could be configurable.

[kg]

That sounds reasonable - what's the downside that merits making it configurable?

[iskiselev]

InferredType is calculated by ILSpy, and it may be buggy sometimes.

[kg]

Gotcha, sounds good. Would it make more sense to configure it on a per-method basis or just make it global? It seems to me like you'd want a way to override it per-method when ILSpy does the wrong thing.

[iskiselev]

Let's start with global configuration. If anybody will need it - we may discuss what we should do. We are talking about very restricted situations (right now - only F# compiler).

[kg]

I should think about updating Try JSIL to support F# so it's easier for people to play around with stuff like this.

[iskiselev]

Thought a little bit more on it. Looks like we still need some additional work besides fixing MethodReference.
If we have char as generic type argument, it will also not be replaced with our proxy.

So, looks like we need start with another option - add JSIL.ToString(value,type) and use it as object::ToString replacement.

[iskiselev]

Probably we should file separate issue for MethodReference replacement in that case.

[iskiselev]

I've implemented JSIL.ToString(value,type) and use it through JSReplacement.

To make it work, based on #850, I changed JSReplacement logic, so that it doesn't affect on method translation (previously there was no translation of methods marked with JSReplacement).
To suppress translation of method could be used newly-introduced JSSuppressTranslation. To achieve same logic as before, method should be marked with both JSReplacementand JSSuppressTranslation.
Right now I've not marked any method with JSSuppressTranslation.

[iskiselev]

So, now all tests are passed with this PR.

[iskiselev]

@kg, reviewing of proxies feels like too big task, and I probably will not have time for it soon.
This PR introduce some changes in Proxies usage, but is stable enough. Could you review it and give me some ideas if you plan to merge it? If ye, I'll rebase it on top of current master.

In my fork I already migrated char to number base - and it now introduce me lot's of merges, so I'd like either merge this PR or think if I could revert this change in my fork.

[kg]

According to github this PR has conflicts. However, I'll read over it and let you know if I'd feel okay merging it.

Moving char to number will have some awkward JS interop implications but it's fine otherwise, at least in principle.

[iskiselev]

I started it based on your suggestion in #860.
Our translation is a little bit easier with numbers for Char. At the same time, there is no "Char" type in JS - only strings with one symbol. Honestly we use string in .Net also much more often, and char is used only in some special case - so probably numbers works for it.

[kg]

This matches .NET, right? re: surrogate pairs, etc.

[iskiselev]

I'll check it additionally, but really .Net strings (same as JS string) knows nothing about surrogate pairs.
So, it should just break them into two Chars (and Char is unable to represent surrogate pair, as it only Int16, and we need Int32 for surrogate pairs).

[kg]

OK, I read through this PR and it looks fine. I'm OK with merging it if you can resolve the conflicts. One question: Did you update things so that char[] arrays have a backing store type of Uint16Array? That would be nice to do at some point, but you don't need to do it right away.

[iskiselev]

Thank you for review!
Will check array type when will rebase it.