Skip to content

Remove Primitive_option.toUndefined; use valFromOption for optional ffi args#8380

Open
cknitt wants to merge 1 commit intomasterfrom
remove-undefined
Open

Remove Primitive_option.toUndefined; use valFromOption for optional ffi args#8380
cknitt wants to merge 1 commit intomasterfrom
remove-undefined

Conversation

@cknitt
Copy link
Copy Markdown
Member

@cknitt cknitt commented Apr 21, 2026

Summary

This PR removes Primitive_option.toUndefined and switches optional FFI argument lowering to use Primitive_option.valFromOption instead.

valFromOption already has the right runtime behavior here:

  • None stays undefined
  • Some(v) becomes v
  • nested options still unwrap by exactly one outer layer

As a result, the extra toUndefined helper is redundant for this code path and can be deleted.

Changes

  • change compiler/core/js_of_lam_option.ml so the non-duplicable optional-argument path emits Primitive_option.valFromOption(...) instead of Primitive_option.toUndefined(...)
  • remove Primitive_option.toUndefined from the runtime source and interface
  • regenerate the JS runtime artifacts
  • update the optional FFI generated test output accordingly

Example

Before:

return hey(Primitive_option.toUndefined(f(x)), 3);

After:

return hey(Primitive_option.valFromOption(f(x)), 3);

@cknitt cknitt requested a review from cristianoc April 21, 2026 16:43
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 21, 2026

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript@8380

@rescript/darwin-arm64

npm i https://pkg.pr.new/@rescript/darwin-arm64@8380

@rescript/darwin-x64

npm i https://pkg.pr.new/@rescript/darwin-x64@8380

@rescript/linux-arm64

npm i https://pkg.pr.new/@rescript/linux-arm64@8380

@rescript/linux-x64

npm i https://pkg.pr.new/@rescript/linux-x64@8380

@rescript/runtime

npm i https://pkg.pr.new/@rescript/runtime@8380

@rescript/win32-x64

npm i https://pkg.pr.new/@rescript/win32-x64@8380

commit: a13afcf

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant