Fix @directive on function level with async and multiple parameters #7977
Conversation
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
| (* The default mapper would descend into nested [Pexp_fun] nodes (used for | ||
| additional parameters) before visiting the function body. Those | ||
| nested calls see [async = false] and would reset [async_context] to | ||
| false, so by the time we translate the body we incorrectly think we are |
There was a problem hiding this comment.
Do we need all this? why not just save !in_function_def and restore it later? Does it already fix the issue.
If not, go with this.
There was a problem hiding this comment.
Unfortunately it seems it is needed.
• I tried the “just save and restore in_function_def” tweak you suggested in compiler/frontend/bs_builtin_ppx.ml:102. Even with that in place, the
nested Pexp_fun that represents the second parameter still executes this branch with async = false, so we compute async_context := (true && !
true) || false, i.e. it flips back to false before we ever reach the real body. The result is the same error as before—node cli/bsc.js -dparsetree
tests/tests/src/function_directives_async.res still reports “Await on expression not in an async context”.The manual rebuild keeps the outer async flag alive while we thread through the extra parameter wrappers: we map the attributes and parameters
ourselves, recurse into the body, then reassemble the function with the original ~async and pass that to Ast_async.make_function_async. With that
code back in place, the same CLI run succeeds and the generated JS contains the 'use cache'; directive. So we do already restore in_function_def,
but the heavier change is what stops the nested parameter visit from clearing async_context, which is the root of the regression.
2 participants
Fixes #7974.
@cristianoc Fix by Codex - please have a look if it makes sense, or if there is a better way to handle this.