Add MultiReturn support

[ParadiseFallen]

It kinda with breaking changes (in filesystem tests) but i tryed my best to bring multret support as softly as i can.

[ParadiseFallen]

Linked issue is https://github.com/ceifa/wasmoon/issues/84

[ParadiseFallen]

code that failed wasn't changed. so it looks like skip this one for me. What do you think, @ceifa?

[ceifa]

code that failed wasn't changed. so it looks like skip this one for me. What do you think, @ceifa?

can you run npm run lint? I think it will fix the build

[ParadiseFallen]

@ceifa done. check now

[ceifa]

Im not confortable about this change. Its a little bit weird that the return type changes if you are returning a single value or a multi value, if we understand that we should support it, we should stick with always returning the array, but this causes other problems, like we cant write a function in lua and just translate it to JS, since one will return a value, and the other an array, example:

function sum(x, y) -- return an array in JS
    return x + y
end

function sum(x, y) { // return a number in lua
    return x + y
}

@timstableford do you have any thoughts?

[ParadiseFallen]

@ceifa there is soft support of mulret. so basically your first code will return one and only one return. and only if you wrap add , x it will return mulret. So anyone can check return type (which is fine) and do what they need to.

[ParadiseFallen]

@ceifa i was thinking about this issue too. but mine impl wont bring any breaking changes to all exsisting projects. So if we make it always return an array it will cause many breaking changes.

For now we can use kinda generics

const [a,b,c] = lua.doString<MulRet>(`code`); // return all returns
const a = lua.doString<any>(`code`); // return first one return

[timstableford]

Im not confortable about this change. Its a little bit weird that the return type changes if you are returning a single value or a multi value, if we understand that we should support it, we should stick with always returning the array, but this causes other problems, like we cant write a function in lua and just translate it to JS, since one will return a value, and the other an array, example:

function sum(x, y) -- return an array in JS
    return x + y
end

function sum(x, y) { // return a number in lua
    return x + y
}

@timstableford do you have any thoughts?

That's a tough one. I think there's two cases though that could be reasonably kept consistent. When calling into a Lua program from JS as an entrypoint like with doString I'd personally have it always return an array. Then for functions that are passed back to JS from Lua have it only use the single return value, like when calling JSON.stringify from Lua and passing a replacer function.

Perhaps I'm just struggling to see a use case for that half of it though? @ParadiseFallen out of curiosity is this a just for the fun of it PR or did you have a specific use in mind?

[ParadiseFallen]

there is no any special use cases but it would be nice to have that feture. if lua uses mulret we can support it softly.

For now there is some built in functions that i want to call and can't bc there is no mulret.

[ParadiseFallen]

@timstableford why we won't use generics? its seems exatly what we want. so by default it will be just single return and if you pass expected return as <MulRet>() it will wrap it in mullret. even if there is only one return

[timstableford]

I'm not against using generics but it'd be necessary to link it to something runtime. As far as I understand TypeScript that could be an argument to the function to say which return mode to use or an alternate function. I'm having a bit of trouble imagining it at the mo but tl;dr I'm sure you can go that route if you work through quirks

[ParadiseFallen]

@timstableford well. if you want use it in js then we can made kinda

function doString(code: string, returnStategy: IReturnStrategy = SingleReturn)
{
  // just as normal code
  return returnStrategy.makeReturn(thread);
}

but need to look for how to hookup it when you want mulret (bc we need start top)

[ParadiseFallen]

or even just use enum for that

export enum ReturnType{
  SingleValue = 0,
  MultipleValues = 1,
}

[ceifa]

That's a tough one. I think there's two cases though that could be reasonably kept consistent. When calling into a Lua program from JS as an entrypoint like with doString I'd personally have it always return an array. Then for functions that are passed back to JS from Lua have it only use the single return value, like when calling JSON.stringify from Lua and passing a replacer function.

You are right, it would break implementations like that, which is very common.

@ceifa there is soft support of mulret. so basically your first code will return one and only one return. and only if you wrap add , x it will return mulret. So anyone can check return type (which is fine) and do what they need to.

I'm not a fan of this approach, returning always the same type would be better for maintainability.

I will let the PR open for now, and think better about it later

[ParadiseFallen]

@ceifa any updates? i suggest move to always returning an array expecting multiparams

[tims-bsquare]

@ParadiseFallen I've come up with something that might make this more palatable (to me, I can't speak for @ceifa who will be merging it).

What you could do is overload the function signatures like this https://stackoverflow.com/a/13212871 For instance with doFile

public doFile(filename: string, returnMode: 'multi'): Promise<MultiReturn>;
public doFile(filename: string, returnMode?: 'single' | 'multi'): Promise<any> {
 // Implementation here
}

That would mean the current function signatures would remain compatible and return a single item but if return mode was set to multi the return type would be MultiReturn instead

[ParadiseFallen]

Автор

@ParadiseFallen I've come up with something that might make this more palatable (to me, I can't speak for @ceifa who will be merging it).

What you could do is overload the function signatures like this https://stackoverflow.com/a/13212871 For instance with doFile

public doFile(filename: string, returnMode: 'multi'): Promise<MultiReturn>;
public doFile(filename: string, returnMode?: 'single' | 'multi'): Promise<any> {
 // Implementation here
}

That would mean the current function signatures would remain compatible and return a single item but if return mode was set to multi the return type would be MultiReturn instead

I already proposed that. we dont even need overload for that.

[tims-bsquare]

You're right that you proposed a return mode, which I generally liked but the explicit return type of multireturn I think will make it easier.

I've just been reading through the comments again after your prompt and the other part that I think was tripping us up was that functions returned from Lua to JS would need to be single return for JS compatibility. I'd be okay with you passing it in as an option to the engine whether to do single returns or multi-returns in that circumstance.

It's been a while since I've put any thought into this, so sorry for rehashing a bit.

[ParadiseFallen]

@tims-bsquare why just single return? its easilly supported with just mulret

const mulret = call();
const [r1, r2, r3] = call();

[tims-bsquare]

For JS interoperability unless I'm misunderstanding you. Consider exposing the JSON.stringify function to Lua as an example. So Lua does

JSON.stringify({}, function (replacer) return something end)

stringify expects a function that return a single argument, by not keeping single as default the above breaks

[ParadiseFallen]

@tims-bsquare there many pain points on example like that. also it looke like

-- lua
-- return generic builder
someJsFunc(function (startIndex) 
-- some wrap for example.
return function ()
  local x = startIndex;
  -- indexer
  return function ()
    x = x + 1;
    return x;
  end
end

end)

this example will breaks. same for js side.

[tims-bsquare]

Sorry @ParadiseFallen I'm not following the problem you're describing. Currently the example I showed works and I'm not sure what your function is showing

[ParadiseFallen]

@tims-bsquare it's showing another problem, that not mentioned there. sometimes wrapper breaks

[ceifa]

@tims-bsquare it's showing another problem, that not mentioned there. sometimes wrapper breaks

I'm a little bit confused too, can you elaborate the problem you are showing?

[ParadiseFallen]

@ceifa will do it later and on another issue

[ParadiseFallen]

@ceifa @tims-bsquare https://github.com/ceifa/wasmoon/issues/98

[gudzpoz]

My two cents as a user:

In Lua, a = (function() return "a", "truncated" end)() does not assign a table to a but simply truncates the stack (i.e. a = "a"). And I think it is only natural to keep this as default.

Alternatively, if multiple return values are wanted, I can always wrap them in a table like return { "a", "b" } and can comfortably achieve anything I want without necessitating this MULTI_RET change.

Some more justifications:

1. The Array when >= 2 (JS) approach:

   In Lua, changing from return 1 to return 1, "extra" is usually not a breaking change due to how the stack truncation works. However, with the current PR (return 1 => (JS) 1, return 1, 2 => (JS) [1, 2]), it will. So either we stick with our previous truncation approach or we always return an array.

2. The Always array approach:

   Always returning an array has its own problem: it poses a limit on the library's capability - Lua functions will no longer be able to simulate any JS function that can return anything.

The following matrix outlines whether the user will be able to do something with code change in only one language:

	Get a single value	Return multiple values
Previous (JS side)	/	Function wrapper
Previous (Lua side)	/	Wrap in tables
Always array (JS)	Function wrapper	/
Always array (Lua)	You can't	/
Array when >= 2 (JS)	Sometimes / wrapper	Sometimes / wrapper
Array when >= 2 (Lua)	Sometimes / wrapper	Sometimes / wrapper

Therefore, speaking as a user, I am against this breaking change. (The overloading approach is non-breaking and feels better.)

[ParadiseFallen]

@ceifa so any updates on this issue?

[ParadiseFallen]

lets take this as example

-- IdGenerator
return function (start,step)
  local current = start
  return function ()
    current = current + step
    return current
  end
end

-- main
local IdGenerator = require 'IdGenerator'

-- this method will called from js
function tryCreateGenerator(start)

  if start < 0 then
    return false
  end

  return true, IdGenerator(start, 1) 
end

return tryCreateGenerator

Then lets call it from js

const factory = new LuaFactory();
const engine = await factory.createEngine();

const tryCreateGenerator = engine.doString(/*code goes here*/);
// we can also get `tryCreateGenerator` just by engine.global.get('name'). 

// and now we facing few issues
// first one is mulret. it won't work
// second one is returning of lua function. it just breakes
const [created, generator] = tryCreateGenerator(2) // won't work

[ParadiseFallen]

also it shows one more issue. if generator() will return 2 values ie

function createGenerator(start, step)
  local current = start
  -- generator itself
  return function()
    current = current + step
    if current > 100 then
      return false, current - step
    end
    return true, current
  end
end

it won't work. bc internal function will called on js side. and it must return mulret. but we can't specify which value we expect. mulret or not. So this option muse be specified a bit higher that just doString or doFile

[ParadiseFallen]

i tryed this again after updating to latest verion. oh yeah. issue with func seems to be fixed now

---

But lets keep talking about mulret. Trimming of stack as done by now isn't what i expect. if it won't trim i could at least get that values by low level calls.

It could be option in createEngine to make this behaviour engine wide. I don't like idea of overload bc of sample above with generator

also it shows one more issue. if generator() will return 2 values

---

We could utilize https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/instanceof so current behaviour will be kept ie

const singleValue = engine.doString(`
return 1;
`); // typeof number
const array = engine.doString(`
return {"a","b","c"};
`); // instance of array
cosnt object = engine.doString(`
return { a = 1, b = 2, c = 3 }
`); // typeof object && ! instanceof MulRet
// new behaviour
const mulret = engine.doString(`
return 1,2,3
`); // typeof object && instance of MulRet

It won;t breaks any existing code unleast there smth strange goes at lua side example

function badFunction()
  return true, "ommited"
end

bc now it will translates to mulret. wich is exatly what on lua side.

[jmptbl]

I tried getting MultiRet working today, and I couldn't figure it out. What I've done for now is to add this shim function to my Lua code:

function multiret (fname, ...)
    local f = _G[fname]
    if f == nil then return nil end
    return table.pack(f(...))
end

Maybe it's not pretty, but I missed how the code in this PR's commit is supposed to work, so this shim seemed reasonable. Either way I don't think there's a clean way of handling MultiRet. Obviously the two languages are different, so the programmer will have to step in to fill this gap. I suggest keeping the default case of returning only one value as is, and supplying a JS-side shim to convert multiret into an array return so that programmers don't have to write their own shim.

[ParadiseFallen]

@jmptbl mulret is

return a, b, c

And not what you wrote there. So on js it will transform to

const result = await engine.doString(`...`);
if(result instanceof  MulRet)
{
  ...
}
else
{
  throw new Error("mulret expected");
}

[jmptbl]

@ParadiseFallen Thank you. For some reason I thought multiret support had already been committed to wasmoon. I was trying to get it working with the wasmoon 1.16.0 release, but ended up writing the lua shim that I shared in my previous comment.

[RodrigoDornelles]

how about a simple solution like:

// multi return in lua
// single return custom object in js
function javascript() {
 return someWasmoonLib.unpack([1, 2])
}

-- multi return in lua
-- single return array in js
function lua()
 return 1, 2
end

I think it shouldn't be something complicated that changes existing interfaces, or something that is difficult to explain beyond an example.

// javascript has destructors, this is already a bit of a multi return, keep it simple.
const [a, b] = lua();

I think it should only be work on the side: javascript -> lua as that's where I felt a deficiency, but the opposite is easily solvable.

what exists, must work as is

function lua()
 return 'hello'
end

const message = lua();

---

@ParadiseFallen @timstableford @tims-bsquare @gudzpoz @jmptbl @ceifa