rescript-lang / syntax

ReScript's syntax as a standalone repo.
MIT License
255 stars 38 forks source link

[10.1] Redefined async functions don't support named arguments #707

Closed TheSpyder closed 2 years ago

TheSpyder commented 2 years ago

I'm migrating my codebase to 10.1 and I have functions called async (some external, some let) that were defined long before ReScript considered adding async/await.

Is this supposed to be allowed now that async is a keyword? Simple functions work, but functions with named arguments throw a syntax error after the function call such as Did you forget a => here? when I attempt to call them:

let async = (~a, b) => a+b
let _ = async(~a=1, 1)

I'm happy to say redefining async shouldn't be allowed, but then let async and external async should trigger an error rather than at the call site.

cristianoc commented 2 years ago

async is not a keyword, as you're observing. The second line looks like might be recognised by the parser as the beginning of a function definition. Would you move this to the syntax repo? It could be that when => is missing, the case could be handled differently when in an async context. Since it's a syntax error, there's low risk trying to change that case see what happens. Though it could be that some complex backtracking is required in which case one would have to assess whether it's worth it.

So this is clearly an error:

let _ = (~a=1, 1)

but this (after desugaring)

 let _ = @res.async (~a=1, 1)

could be handled differently

TheSpyder commented 2 years ago

I don't have move access 🤔 or did you want me to close and re-log?

cristianoc commented 2 years ago

Let me do it.

TheSpyder commented 2 years ago

Example of redefined async function that works:

let async = (a, b) => a+b
let r = async(1, 1)
cristianoc commented 2 years ago

Here: https://github.com/rescript-lang/syntax/pull/708

TheSpyder commented 2 years ago

Thank you! I'll test it as soon as I can.