Fix fatal error for external with @as #7978
Conversation
| in | ||
| let fn_type = Ast_helper.Typ.arrows ~loc args result_type in | ||
| ( build_uncurried_type ~arity:(List.length args) fn_type, | ||
| ( fn_type, |
There was a problem hiding this comment.
build_uncurried_type was trying to set a function type even if no args were left.
But the function type with the correct arity is already set by Ast_helper.Typ.arrows anyway when required.
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
Or do we want/need to add an "artificial" |
There was a problem hiding this comment.
The fact that the function without any args gets called automatically seems pretty weird to me.
I would have expected this would not be allowed.
But if that's what one wants, OK.
Notice the risk is this change affects other cases unrelated to this. But I assume some though went into removing build_uncurried_type as mentioned in the comment.
Not sure if that's what one wants. 🙂 I can look into introducing an "artificial" unit arg.
|
|
@cristianoc now adding a synthetic unit argument, please have another look. |
cknitt
force-pushed
the
fix-external-as
branch
from
a385b15 to
1badbec
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ( Ast_helper.Typ.arrows ~loc args result_type, | ||
| External_ffi_types.ffi_bs arg_type_specs return_wrapper ffi, | ||
| unused_attrs, |
There was a problem hiding this comment.
Previously this code rebuilt the return type with build_uncurried_type ~arity:(List.length args) so externals declared with an uncurried arrow retained their arity metadata. After the change it returns Ast_helper.Typ.arrows directly and the build_uncurried_type wrapper is never applied in the non-object path, so externals such as external add: (int, int) => int are now typed as int -> int -> int. Calls written with uncurried syntax (add(1, 2)) will start raising Apply_non_function/arity mismatches during type checking because the arity flag is gone. The new code should still apply build_uncurried_type to the reconstructed arrow to keep uncurried externals working.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
@codex But Ast_helper.Typ.arrows does already set the arity for the outer function to List.length args.
There was a problem hiding this comment.
To use Codex here, create an environment for this repo.
There was a problem hiding this comment.
Definitely not true, external add: (int, int) => int still works correctly.
There was a problem hiding this comment.
How comes codex runs now? That changed?
It used to get stuck trying to install ocaml.
There was a problem hiding this comment.
I think it didn't actually run anything, it just reviews the code.
2 participants
Fixes #7972
When the external function has a single arg that gets a fixed value via@as(json, all that's left is a actually constant, see test.