Semantic Diff

#9283 Fix optional external arguments that throw on None or run out of order

2 commits

e941d27298f5f7dcbc9a42cce0d5d7b349d27f7d..pr-9283

c90750038 · mununki · Nov 09, 2026, 15:21 UTC

Pass undefined for an optional external argument or JSX key that is None

Why

An optional argument of an external passed as ~x=?e, where e is a call, a dict or array lookup, a conditional or anything else that cannot be read twice to test it first, is compiled to Primitive_option.valFromOption(e). valFromOption reads a property of its argument to find a nested None, so it threw a TypeError when e was None, though its doc comment says that it keeps undefined. #8380 replaced Primitive_option.toUndefined, which tested for undefined before calling valFromOption, on that premise. The same call is emitted for a JSX key, also with -bs-jsx-preserve, for the argument of an inlined function that forwards ~x? to an external, and for an optional @int argument, or @string argument with @as, which switches on it. valFromOption now returns undefined for undefined, as it does for null, which fixes all of them.

modifiedJS Primitive_option.valFromOption

js
Before
After
function valFromOption(x) {function valFromOption(x) { if (x === null || x.BS_PRIVATE_NESTED_SOME_NONE === undefined) { if ((x == null) || x.BS_PRIVATE_NESTED_SOME_NONE === undefined) { return x; return x; } } … …}}

modifiedOutput optional argument that is None

%%raw(`function greet(name) { return name === undefined ? "Hello" : "Hello " + name }`)@val external greet: (~name: string=?, unit) => string = "greet" let greetUser = (names: dict<string>, id) => greet(~name=?names->Dict.get(id), ())Console.log(greetUser(dict{"1": "Ann"}, "2"))
js
Before and After are equal
function greetUser(names, id) {  return greet(Primitive_option.valFromOption(names[id]));}
text
Before
After
TypeError: Cannot read properties of undefined (reading 'BS_PRIVATE_NESTED_SOME_NONE')Hello

Review

  • Why change the runtime and not the compiler? Every path that emits the unchecked call goes through valFromOption: Arg_optional in ocaml_to_js_eff, the JSX key in Lam_compile_primitive, inlined functions, and @int and @as arguments, which switch on its result. get_default_undefined_from_optional returns an expression, so a test in the compiler needs the argument bound to a temporary: a statement for each such argument, through hold_arg for calls and separately for JSX keys, plus temporaries for the arguments to its left (see sequence_arguments). Bringing back toUndefined would undo #8380 for one comparison. This way the generated code stays as it is, and code that 13.0.0-alpha.5 or later already compiled works once the runtime is updated.
  • Does the extra test cost the other callers anything? They pass a value known to be Some: a match on a nested option (Pval_from_option), and the branch after x !== undefined of an @obj assignment or of an optional argument that can be read twice. For them x == null replaces x === null, still one comparison; in a micro-benchmark of 2×10^8 calls the difference is within noise for objects or numbers alone and under 0.3 ns a call for mixed inputs. The runtime grows by one character, and no other compiled module of the runtime, the stdlib or the tests changes.
  • Could an argument that can be read twice now be compiled to valFromOption(x) alone, without x !== undefined ? … : undefined? It could, but that would change much output and call the runtime for None. The next commit does it only for a JS property read, which may run a getter.
308b45686 · mununki · Nov 09, 2026, 15:21 UTC

Evaluate optional @obj arguments in source order and read properties once

Why

An @obj external builds its object as a literal and assigns an optional argument whose presence is not known after it, under if (tmp !== undefined). So that argument was evaluated after the arguments that follow it: make(~size=?size(), ~mode=mode(), ()) called mode() first, and a later argument with side effects could change what the optional argument read, such as a @get getter, a @val global, Dict.get, an array element, a mutable field or a ref. hold_obj_args now binds such an argument to a temporary after its statements, unless evaluating it after the literal gives the same values and effects.

As with the temporaries of #9217, sequence_arguments then evaluates the arguments to the left of the temporary before it, with evaluate_now. That bound an optional argument known to be present as a whole, so that it lost its presence and was assigned after the literal too, and it bound variables and fields of records although defining a temporary as such a value cannot change them. evaluate_now now evaluates the value in an Optional_block and keeps the block, and sequence_arguments leaves an argument that runs no code, a variable, a constant or a field of a record, in place when the statements after it only define variables as such values. A JS property read may run a getter, so it does not count.

An optional argument that is a JS property read, as from a @get external or a @val name with a dot, was read twice, to test it for None and to unwrap it, by a call, a JSX key and an @obj external alike. A getter ran twice, and one that returned a value and then undefined made the call throw a TypeError. Such an argument is now read once: a call and a JSX key give it to Primitive_option.valFromOption, which returns undefined for None since the previous commit, and an @obj external binds it. Js_analyzer.is_okay_to_duplicate ~property_reads:false tells these arguments apart.

modifiedJS optional argument with side effects

@obj external make: (~size: int=?, ~mode: string, unit) => {..} = "" let o = (size, mode) => make(~size=?size(), ~mode=mode(), ())
js
Before
After
function o(size, mode) {function o(size, mode) { let tmp = { let tmp = size(); let tmp$1 = { mode: mode() mode: mode() }; }; let tmp$1 = size(); if (tmp$1 !== undefined) { tmp.size = tmp$1; if (tmp !== undefined) { tmp$1.size = tmp; } } return tmp; return tmp$1;}}

modifiedJS optional argument that a later argument changes

type source@get external name: source => option<string> = "name"@obj external makeName: (~name: string=?, ~mode: string, unit) => {..} = "" let n = (s, rename) => makeName(~name=?name(s), ~mode=rename(s), ())
js
Before
After
function n(s, rename) {function n(s, rename) { let tmp = { let tmp = s.name; let tmp$1 = { mode: rename(s) mode: rename(s) }; }; if (s.name !== undefined) { tmp.name = s.name; if (tmp !== undefined) { tmp$1.name = tmp; } } return tmp; return tmp$1;}}

modifiedJS arguments before a bound optional argument

type props = {color: string, width?: string}@objexternal style: (~color: string=?, ~icon: string=?, ~width: string=?, ~height: string, unit) => {..} = "" let a = (props, height) => style(~color=props.color, ~width=?props.width, ~height=height(), ())let b = (icon, width, height) => style(~icon=icon(), ~width=?width(), ~height=height(), ())
js
Before
After
function a(props, height) {function a(props, height) { let tmp = { let tmp = props.width; let tmp$1 = { color: props.color, color: props.color, height: height() height: height() }; }; if (props.width !== undefined) { tmp.width = props.width; if (tmp !== undefined) { tmp$1.width = tmp; } } return tmp; return tmp$1;}} function b(icon, width, height) {function b(icon, width, height) { let tmp = { icon: icon(), let tmp = icon(); let tmp$1 = width(); let tmp$2 = { icon: tmp, height: height() height: height() }; }; let tmp$1 = width(); if (tmp$1 !== undefined) { if (tmp$1 !== undefined) { tmp.width = tmp$1; tmp$2.width = tmp$1; } } return tmp; return tmp$2;}}

modifiedJS optional argument known to be present before a temporary

type item = {id: int}@obj external mk: (~item: item=?, ~level: @int [#low | @as(10) #high], unit) => {..} = "" let k = async (f, p) => mk(~item=f(), ~level=await p, ())
js
Before
After
async function k(f, p) {async function k(f, p) { let tmp = f(); let tmp = f(); let tmp$1 = await p; let tmp$1 = await p; let tmp$2 = { return { item: tmp, level: (() => { level: (() => { switch (tmp$1) { switch (tmp$1) { … … } } })() })() }; }; if (tmp !== undefined) { tmp$2.item = Primitive_option.valFromOption(tmp); } return tmp$2; }}

modifiedJS optional property read

type source@get external name: source => option<string> = "name"@val external greet: (~name: string=?, unit) => string = "greet"@val external userName: option<string> = "window.userName"@obj external makeName: (~name: string=?, ~mode: string, unit) => {..} = "" let g = s => greet(~name=?name(s), ())let u = () => greet(~name=?userName, ())let m = (o: {"name": option<string>}) => makeName(~name=?o["name"], ~mode="dark", ())
js
Before
After
function g(s) {function g(s) { return greet(s.name !== undefined ? Primitive_option.valFromOption(s.name) : undefined); return greet(Primitive_option.valFromOption(s.name));}} function u() {function u() { return greet(window.userName !== undefined ? Primitive_option.valFromOption(window.userName) : undefined); return greet(Primitive_option.valFromOption(window.userName));}} function m(o) {function m(o) { let tmp = { let tmp = { mode: "dark" mode: "dark" }; }; if (o.name !== undefined) { tmp.name = o.name; let tmp$1 = o.name; if (tmp$1 !== undefined) { tmp.name = tmp$1; } } return tmp; return tmp;}}

modifiedJS temporaries for optional arguments of an @obj external

representation
Before Lam_compile_external_obj.hold_obj_args
After
~x=?f(), ~m=g() ↦ after the literal~x=?p.x, ~m=g() ↦ after the literal~x=?f(), ~m=p.m ↦ after the literal~x=?f(), g() ↦ after the literal~x=?f(), ~m=g() ↦ temporary~x=?p.x, ~m=g() ↦ temporary~x=?f(), ~m=p.m ↦ temporary~x=?f(), g() ↦ temporary~x=?p.x, ~m=p.m ↦ after the literal~x=?p.x, ~m=p.m ↦ after the literal~x=?f(), ~m="dark" ↦ after the literal~x=?f(), ~m="dark" ↦ after the literal~x=?v, ~m=g() ↦ after the literal~x=?v, ~m=g() ↦ after the literal~x=?f(), ~y=?g() ↦ after the literal~x=?f(), ~y=?g() ↦ after the literal
notep.x stands for a field, a global or a lookup, and v for an immutable variable
notep.x stands for a field, a global or a lookup, and v for an immutable variable

modifiedJS arguments before a statement

representation
Before Lam_compile.sequence_arguments
After
r.c, then let tmp = r.x ↦ temporaryr.c, then let tmp = r.x ↦ in placer.c, then let tmp = s.x ↦ temporaryr.c, then let tmp = s.x ↦ temporarys.c, then let tmp = r.x ↦ temporarys.c, then let tmp = r.x ↦ temporaryf(), then let tmp = r.x ↦ temporaryf(), then let tmp = r.x ↦ temporaryr.c, then let tmp = g() ↦ temporaryr.c, then let tmp = g() ↦ temporary~c=f(), then a statement ↦ temporary for the option~c=f(), then a statement ↦ temporary for f(), option in place
noter.c is a variable or a field of a record, s.x a JS property read, and ~c=f() an optional argument known to be present
noter.c is a variable or a field of a record, s.x a JS property read, and ~c=f() an optional argument known to be present

modifiedJS optional arguments read twice

representation
Before Js_analyzer.is_okay_to_duplicate
After Js_analyzer.is_okay_to_duplicate ~property_reads:false
x, M.x, globalName ↦ test, then unwrapx, M.x, globalName ↦ test, then unwrapr.field ↦ test, then unwrapr.field ↦ test, then unwraps.prop, o["prop"], window.x ↦ test, then unwraps.prop, o["prop"], window.x ↦ unwrap oncef(), a[i], c ? x : y ↦ unwrap oncef(), a[i], c ? x : y ↦ unwrap once
noter.field is a field of a record, s.prop a JS property read, such as a @get getter, and window.x a @val name with a dot

Review

  • Which calls keep their output? An @obj call in which nothing that the literal evaluates after the optional argument can change it or be changed by it: constants, immutable variables and functions, reads without side effects when the optional argument has none either, as in make(~x=?p.x, ~m=p.m, ()), and other optional arguments whose presence is not known, which are assigned after it, in order. Variables and fields of records are still tested and unwrapped in place. No checked-in output changes besides the new tests.
  • Which arguments get a temporary they do not need? A later argument is taken to change an optional one unless it is delay_safe or neither has side effects, and the JS IR does not say whether a field is mutable or a call pure. So make(~a=?props.a, ~b=Int.toString(n), ()) binds props.a, as a above binds props.width. A field of a JS object type, o["x"], cannot be told from a getter, so an @obj external binds it even when no later argument can change it, as m above, while a call no longer tests it inline.
  • Why in hold_obj_args and not in hold_arg? Whether the argument has to be bound depends on the arguments after it, which hold_arg does not see, and only an object-creating external evaluates an argument after the ones that follow it. The binding does not depend on the type of the argument, so an optional polymorphic variant argument keeps its place whether or not hold_arg binds a later argument.
  • Why is delay_safe passed to hold_obj_args? It is Lam_compile.delay_safe, which needs the variables the function assigns, and Lam_compile depends on this module.
  • Why change evaluate_now and sequence_arguments here? Before this commit only the temporaries of #9217 for polymorphic variant arguments made sequence_arguments bind the arguments to their left, as in k above, where item lost its place in Object.keys. Binding optional arguments of any type would make that common: without the two changes, b above would assign icon after the literal as well, and a would bind props.color. Both functions are in lam_compile.ml, which #9231 does not change.
  • Why a flag on is_okay_to_duplicate and not another predicate? The two differ only in property reads, and its other callers, min and max of booleans and the object of a Pjs_object_get, keep the default. Js_ast_util.named_expression takes the same flag, which assemble_obj_args passes.
  • Is a @val global still read twice? Yes, a plain one is, like a value of a JS module: only a property read is taken to run a getter, as a @get external is there to do. A @val name with a dot and a global with @scope compile to a property read and are read once.
  • What still runs out of order? The side effects of a unit or @ignore argument are put before the literal or the call, so mk(~a=f(), g()) calls g() first, as before. A getter is taken to have no side effects, so make(~name=?name(s), ~count=count(s), ()) with two @get externals still reads count first.