Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JS Null and Undefined objects are not treated correctly in .call and .appl #38

Closed
ScottWeinstein opened this issue May 4, 2011 · 8 comments
Labels

Comments

@ScottWeinstein
Copy link

It might be too early for this to work, but I was playing w/ the idea of using CoffeeScript from IronJS.

So I have the following minimal snippet

open System.Net  
open IronJS
open IronJS.Hosting.FSharp
let cssrcUri = System.Uri "https://github.com/jashkenas/coffee-script/raw/master/extras/coffee-script.js"
let wc = new WebClient()
let ctx = IronJS.Hosting.FSharp.createContext()
let ret1 = ctx |> execute(wc.DownloadString cssrcUri)
let ret2 = ctx |> execute("CoffeeScript.compile('foo -> 1', {bare: true});")

but I get the following error on the last line:

System.Reflection.TargetInvocationException was unhandled
Message: Exception has been thrown by the target of an invocation.
at IronJS.Hosting.FSharp.run(Delegate compiled, T t) in GitHub\IronJS\Src\IronJS\Hosting.fs:line 178
at IronJS.Hosting.FSharp.execute(String source, T t) in GitHub\IronJS\Src\IronJS\Hosting.fs:line 200

@otac0n
Copy link
Collaborator

otac0n commented May 4, 2011

That particular version is the "command line version", which has external dependencies on Node.js

The coffee script core compiler: http://jashkenas.github.com/coffee-script/extras/coffee-script.js
Has all of the require'd stuff built in.

Could you try the core compiler version?

@ScottWeinstein
Copy link
Author

The two urls have the same content, the core compiler.

http://jashkenas.github.com/coffee-script/extras/coffee-script.js
https://github.com/jashkenas/coffee-script/raw/master/extras/coffee-script.js

But I made sure, same result with both versions.

@fholm
Copy link
Owner

fholm commented May 5, 2011

I'll look into this and see if I can re-produce it over the weekend, to busy with work this week to have time right now, sadly.

@otac0n
Copy link
Collaborator

otac0n commented May 6, 2011

This looks like an issue with the Function.prototype.call method. (Native.Function.fs, lines 131 though 133)

We are generating a delegate to call, based on the types of the parameters passed in.
However, we are always using the CLR value of the parameters.
The CLR Value of a JS undefined object is null, and we are then calling TypeUtils.getType on null, which is causing the exception.

For example (min repro):

var foo = function (a, b, c, d, e, f) { print('bleh'); };
var me = "";
foo.call(me, 1, 'a', undefined, null);

results in:
CLR Values: { 1, "a", null, null, null, null }
CLR Types: { Int32, String, error, ... }

Whereas it should probably result in:
CLR Types: { Int32, String, BoxedValue, BoxedValue, BoxedValue, BoxedValue}

@otac0n
Copy link
Collaborator

otac0n commented May 6, 2011

I'm not 100% sure what the fix should be, here.

It seems to me that the TC.ToClrObject should maybe return the BoxedValue for anything that is not IsClr, but I'm not sure if that is the desired semantics of this method. (i.e., I don't know whether a JS Null should return a CLR null here.)

Otherwise, we need to change line 132 to read:

let argsClr = args |> Array.map (fun i -> if not i.IsNull && i.IsClr then TC.ToClrObject i else i :> obj)

This does, in fact, allow the above example to work.

@otac0n otac0n closed this as completed in a6ca007 May 6, 2011
@otac0n
Copy link
Collaborator

otac0n commented May 6, 2011

I opted to use the latter definition, in order to preserve all previous usages of TC.ToClrObject.

@ScottWeinstein
Copy link
Author

The fix lets the code run w/o errors, that said, the results aren't correct yet.

Compiling this CoffeeScript fragment foo -> 1 with IronJS gives:

var ;
 foo(function() {
var ;
return 1;
});

but the correct result looks like

foo(function() {
  return 1;
});

@otac0n
Copy link
Collaborator

otac0n commented May 6, 2011 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants