teal-language / tl

The compiler for Teal, a typed dialect of Lua
MIT License
2.1k stars 108 forks source link

Syntax error in function with function positional parameter with multiple return values followed by any other parameter #569

Open Aire-One opened 1 year ago

Aire-One commented 1 year ago

Hello,

I have this Lua code that I want to translate to teal:

local function do_or_fail(func, ...)
   local res, err = func(...)

   if not res then
      print("do_or_fail failed!", err)
      error(err)
   end

   return res
end

My first attempt was to write this:

https://teal-playground.netlify.app/?c=bG9jYWwgZnVuY3Rpb24gZG9fb3JfZmFpbDxUPihmdW5jOiBmdW5jdGlvbjxUPiguLi46IGFueSk6IFQgfCBuaWwsIHN0cmluZywgLi4uOiBhbnkpOiBUCiAgIGxvY2FsIHJlcywgZXJyID0gZnVuYyguLi4pCgogICBpZiBub3QgcmVzIHRoZW4KICAgICAgcHJpbnQoImRvX29yX2ZhaWwgZmFpbGVkISIsIGVycikKICAgICAgZXJyb3IoZXJyKQogICBlbmQKCiAgIHJldHVybiByZXMKZW5k

local function do_or_fail<T>(func: function<T>(...: any): T | nil, string, ...: any): T
-- ...

But teal can't interpret this function signature, and gives syntax errors:

3 syntax errors:
utils.tl:73:76: expected a type
utils.tl:73:79: syntax error, expected one of: ')', ','
utils.tl:73:81: syntax error, expected one of: ')', ','

(line 73 is the line local function with the character 76 being the whitespace in string, ...)

Here is a working version I found:

https://teal-playground.netlify.app/?c=bG9jYWwgdHlwZSBGdW5jID0gZnVuY3Rpb248VD4oLi4uOiBhbnkpOiBUIHwgbmlsLCBzdHJpbmcKbG9jYWwgZnVuY3Rpb24gZG9fb3JfZmFpbDxUPihmdW5jOiBGdW5jPFQ%2BLCAuLi46IGFueSk6IFQKICAgbG9jYWwgcmVzLCBlcnIgPSBmdW5jKC4uLikKCiAgIGlmIG5vdCByZXMgdGhlbgogICAgICBwcmludCgiZG9fb3JfZmFpbCBmYWlsZWQhIiwgZXJyKQogICAgICBlcnJvcihlcnIpCiAgIGVuZAoKICAgcmV0dXJuIHJlcwplbmQ%3D

local type Func = function<T>(...: any): T | nil, string
local function do_or_fail<T>(func: Func<T>, ...: any): T
   local res, err = func(...)

   if not res then
      print("do_or_fail failed!", err)
      error(err)
   end

   return res
end

As you can see, I had to declare a separate type for the function parameter. This is an acceptable fix, but not ideal. I'd like the first version to work.

lenscas commented 1 year ago

https://teal-playground.netlify.app/?c=bG9jYWwgZnVuY3Rpb24gZG9fb3JfZmFpbDxUPihmdW5jOiBmdW5jdGlvbjxUPiguLi46IGFueSk6IChUIHwgbmlsLCBzdHJpbmcpLCAuLi46IGFueSk6IFQKICAgbG9jYWwgcmVzLCBlcnIgPSBmdW5jKC4uLikKCiAgIGlmIG5vdCByZXMgdGhlbgogICAgICBwcmludCgiZG9fb3JfZmFpbCBmYWlsZWQhIiwgZXJyKQogICAgICBlcnJvcihlcnIpCiAgIGVuZAoKICAgcmV0dXJuIHJlcwplbmQ%3D

Seems to also fix it.

The problem is caused because teal doesn't know how many return parameters you want and thus assumes that every , is followed by a return parameter. ... : any is not valid syntax to write a return parameter and this results in the syntax error.

The easiest way to run into this syntax error is

local function b(func: function():string, c:string) end

with the fix being also being to put ()'s around the return type.

Perhaps the entire return type should be marked as an error, clearly stating that a return type for lambda's should have ()'s around it. Especially as code like

global record Test
   t: function(function():string, string)
end

is currently valid, which is actually quite ambiguous in what it means

Aire-One commented 1 year ago

I like this extra parenthesis to wrap lambda return types syntax.

Perhaps the entire return type should be marked as an error, clearly stating that a return type for lambda's should have ()'s around it. Especially as code like

global record Test
  t: function(function():string, string)
end

Yup, your example is actually perfect to show how ambiguous it can currently be.

I agree with you that the extra parenthesis should be enforced.

hishamhm commented 1 year ago

The canonical solution for the original problem pointed here is indeed to add parentheses to the return tuple.

I'm not entirely sold on the idea that parentheses should always be enforced in function type definitions, though. For the original problem it should be possible to accept it with a a bit of parser lookahead, as it's not ambiguous. For the latter global record Test example, that's indeed an unfortunate ambiguity. For that, I'm wondering if we could detect and report.