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

[interop] Argument handling inconsistent #314

Open
fniephaus opened this issue Jul 3, 2020 · 1 comment
Open

[interop] Argument handling inconsistent #314

fniephaus opened this issue Jul 3, 2020 · 1 comment
Assignees
Labels
interop Polyglot Language Interoperability

Comments

@fniephaus
Copy link
Member

fniephaus commented Jul 3, 2020

When a function is called in JavaScript, there can be more arguments than needed. For example, Array.prototype.forEach() can be used like this:

$ polyglot --jvm --shell
GraalVM MultiLanguage Shell 20.1.0
Copyright (c) 2013-2019, Oracle and/or its affiliates
  JavaScript version 20.1.0
js> [1, 2, 3].forEach((currentValue) => console.log(currentValue))
1
2
3
js> [1, 2, 3].forEach((currentValue, index) => console.log(currentValue + "-" + index))
1-0
2-1
3-2
js> [1, 2, 3].forEach((currentValue, index, array) => console.log(currentValue + "-" + index + "-" + array))
1-0-1,2,3
2-1-1,2,3
3-2-1,2,3

However, when doing interop with other languages, the callback provided must consume exactly three arguments:

$ polyglot --jvm --shell
GraalVM MultiLanguage Shell 20.1.0
Copyright (c) 2013-2019, Oracle and/or its affiliates
  JavaScript version 20.1.0
  Python version 3.8.2
js> [1, 2, 3].forEach(Polyglot.eval("python", "lambda currentValue,index,array: print('%s-%s-%s' % (currentValue, index, array))"))
1-0-[1, 2, 3]
2-1-[1, 2, 3]
3-2-[1, 2, 3]
js> [1, 2, 3].forEach(Polyglot.eval("python", "lambda currentValue: print(currentValue)"))
TypeError: <lambda>() takes 1 positional argument but 3 were given

This is not expected from the user's point of view.

I believe the reason for this is that there's no way to determine the arity of an executable interop value prior invocation, so Graal.js always tries to execute the interop callback with three arguments. IIRC all callbacks are always invoked with three arguments. But in case of an anonymous JS function, unused arguments will just be ignored. A naive solution would be to retry invoking an interop executable with fewer arguments until success. If the invocation with no arguments fails, so must the interop call. That, however, may not be the best strategy to provide a more consistent user experience. Maybe the interop protocol needs some sort of arity mechanism, but that may need too support many kinds of arguments (positional, optional, keyword, splat, ...).

/cc @chumer and @LeonBein

@wirthi
Copy link
Member

wirthi commented Jul 3, 2020

Hi,

I think the problem here is in the ECMAScript specification (and we just play by those rules). https://www.ecma-international.org/ecma-262/10.0/index.html#sec-array.prototype.foreach states that

callbackfn should be a function that accepts three arguments.

and we call it like that, with three arguments all the time. If an argument is not available, it will be undefined. And this is fine in JS: Excess arguments will land in the arguments array, or your named argument will be undefined, when you provided too many/too few arguments respectively. So while it should be a three argument function, JS does not care too much in practice and you can call a zero-argument function still (if you can live with the consequences, e.g. reading the arguments out of the arguments array). Python does not seem to like that, <lambda>() takes 1 positional argument but 3 were given seems it did not accept our two extra (most likely undefined) arguments.

I am not sure there is a clean way to deal with that.

We could detect this is a foreign lambda and then - in violation of the ECMAScript spec! - call it with fewer arguments. But with how many? Python might expect another number than Ruby. You brought up that question already above.

-- Christian

@wirthi wirthi added the interop Polyglot Language Interoperability label Jul 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interop Polyglot Language Interoperability
Projects
None yet
Development

No branches or pull requests

3 participants