From 9d4f9531fc050a29bb1d731a9b4187edf530d972 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Mon, 19 Oct 2026 19:49:21 +0000 Subject: [PATCH 1/6] Stop warning 20 on calls of open JS object methods Why: Calling a method read from an open JS object type, such as `obj["getData"]("text/plain")` with `obj: {..}`, reported warning 20 "this argument will not be used by the function". The method's type is a fresh type variable, which the warning took as a sign that the function never returns, but the call passes its arguments to the JS method. Calls whose function is a fresh type variable for other reasons, such as `failwith("x")(1)`, still warn. Diagnostic removed (warning 20 on open object methods): Example: @get external clipboardData: string => {..} = "clipboardData" let text = clipboardData("event")["getData"]("text/plain") Before (diagnostic): Warning number 20 2:46-57 this argument will not be used by the function. After (diagnostic): none Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel --- changes/object-method-unused-argument.fix.md | 1 + compiler/ml/typecore.ml | 13 ++++++++++++- .../unused_argument_object_method.res.expected | 10 ++++++++++ .../fixtures/unused_argument_object_method.res | 7 +++++++ 4 files changed, 30 insertions(+), 1 deletion(-) create mode 100644 changes/object-method-unused-argument.fix.md create mode 100644 tests/build_tests/super_errors/expected/unused_argument_object_method.res.expected create mode 100644 tests/build_tests/super_errors/fixtures/unused_argument_object_method.res diff --git a/changes/object-method-unused-argument.fix.md b/changes/object-method-unused-argument.fix.md new file mode 100644 index 0000000000..8ae5956216 --- /dev/null +++ b/changes/object-method-unused-argument.fix.md @@ -0,0 +1 @@ +- Fix warning 20 ("this argument will not be used by the function") firing on calls of methods of open JS object types, such as `obj["getData"]("text/plain")`. diff --git a/compiler/ml/typecore.ml b/compiler/ml/typecore.ml index a2b5fdbb8f..09c1d17d6a 100644 --- a/compiler/ml/typecore.ml +++ b/compiler/ml/typecore.ml @@ -4481,9 +4481,20 @@ and type_application ~context total_app env funct (sargs : sargs) : List.map (fun (l1, _) -> {Types.lbl = l1; typ = newvar ()}) sargs in let ty_ret = newvar () in + (* A fresh type variable as function type usually means the function + never returns (e.g. [failwith("x")(1)]), so the arguments are + unused. A JS object method read through an open object type + ([obj["m"]] with [obj: {..}]) also gets a fresh variable, but it is + an ordinary JS method that receives its arguments. *) + let is_object_method = + match funct.exp_desc with + | Texp_object_get _ -> true + | _ -> false + in if ty_fun.level >= ty_ret.level - && not (is_identity_coercion funct.exp_desc) + && (not (is_identity_coercion funct.exp_desc)) + && not is_object_method then List.iter (fun (_, sarg1) -> diff --git a/tests/build_tests/super_errors/expected/unused_argument_object_method.res.expected b/tests/build_tests/super_errors/expected/unused_argument_object_method.res.expected new file mode 100644 index 0000000000..5592a9e2b8 --- /dev/null +++ b/tests/build_tests/super_errors/expected/unused_argument_object_method.res.expected @@ -0,0 +1,10 @@ + + Warning number 20 + /.../fixtures/unused_argument_object_method.res:7:45 + + 5 │ + 6 │ // A function that never returns ignores its arguments: warning 20. + 7 │ let ignored = () => failwith("unreachable")(1) + 8 │ + + this argument will not be used by the function. \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/unused_argument_object_method.res b/tests/build_tests/super_errors/fixtures/unused_argument_object_method.res new file mode 100644 index 0000000000..c30d0dc9ad --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/unused_argument_object_method.res @@ -0,0 +1,7 @@ +@get external clipboardData: string => {..} = "clipboardData" + +// A method of an open JS object type receives its arguments: no warning 20. +let text = clipboardData("event")["getData"]("text/plain") + +// A function that never returns ignores its arguments: warning 20. +let ignored = () => failwith("unreachable")(1) From ee777910e25b30646a3626e93f733ddc8d5e192a Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Tue, 20 Oct 2026 13:31:16 +0000 Subject: [PATCH 2/6] Keep warning 20 on object fields that cannot return Why: The exemption for open JS object methods covered every object field read, so calls of fields that cannot return, such as `failwith("boom")["m"](x)` or `{"m": failwith("boom")}["m"](x)`, lost warning 20. The read now records the fresh variable it creates for a field added to an open row, and the application skips the warning only when its callee is that variable. Diagnostic added (warning 20 on fields that cannot return): Example: @get external clipboardData: string => {..} = "clipboardData" let text = clipboardData("event")["getData"]("text/plain") let fromFailwith = () => failwith("boom")["m"](Console.log("b")) Before (diagnostic): none, on either call After (diagnostic): Warning number 20 3:48-63 this argument will not be used by the function. Review: Why a global slot rather than a typed-tree flag? The callee's read is the last read typed before its application, and the slot is set on every read and compared by physical equality with the callee's type, so it only matches the callee's own read. A typed-tree field would change Texp_object_get and the .cmt format for a warning. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel --- compiler/ml/typecore.ml | 29 +++++++++++++++---- ...unused_argument_object_method.res.expected | 26 +++++++++++++++++ .../unused_argument_object_method.res | 4 +++ 3 files changed, 54 insertions(+), 5 deletions(-) diff --git a/compiler/ml/typecore.ml b/compiler/ml/typecore.ml index 09c1d17d6a..b02114d624 100644 --- a/compiler/ml/typecore.ml +++ b/compiler/ml/typecore.ml @@ -2510,6 +2510,13 @@ let should_unify_expected_result_before_typing_lowered_apply funct sargs = type targs = (Asttypes.arg_label * Typedtree.expression option) list +(* The type of the most recent object field read if that read added the field + to an open object row ([obj["m"]] with [obj: {..}]), [None] after any other + read. [type_application] compares it with its callee's type to tell such a + JS method, whose type is a fresh variable, from a callee that never + returns. *) +let open_row_field_read : type_expr option ref = ref None + (* Eliminate an object-field scheme for a read. A declared field payload is wrapped in [Tpoly]: an empty binder list needs ordinary instantiation, while a nonempty list needs polymorphic instantiation. An inferred field whose @@ -3642,11 +3649,22 @@ and type_expect_ ?deprecated_context ~context ?(recarg = Rejected) env sexp } | Pexp_object_get (e, ({txt = met} as met_loc)) -> ( let obj = type_exp ~context:None env e in + (* An object type without the field gets it from its open row (or the + lookup fails). An object whose type is still a variable, such as + [failwith("x")], does not count. *) + let adds_field_to_row = + match (expand_head env obj.exp_type).desc with + | Tobject fields -> + let fields, _ = Ctype.flatten_fields fields in + not (List.exists (fun (f : Ctype.field_info) -> f.f_name = met) fields) + | _ -> false + in try (* Lookup constrains an open object row to contain the field, without requiring or adding write capability. *) let typ = filter_method env met obj.exp_type in let typ = object_field_use_type env typ in + open_row_field_read := if adds_field_to_row then Some typ else None; rue { exp_desc = Texp_object_get (obj, met_loc); @@ -4483,12 +4501,13 @@ and type_application ~context total_app env funct (sargs : sargs) : let ty_ret = newvar () in (* A fresh type variable as function type usually means the function never returns (e.g. [failwith("x")(1)]), so the arguments are - unused. A JS object method read through an open object type - ([obj["m"]] with [obj: {..}]) also gets a fresh variable, but it is - an ordinary JS method that receives its arguments. *) + unused. A JS object method that the read added to an open object + row ([obj["m"]] with [obj: {..}]) also gets a fresh variable, but it + is an ordinary JS method that receives its arguments. The callee's + read is the last one typed, so [open_row_field_read] belongs to it. *) let is_object_method = - match funct.exp_desc with - | Texp_object_get _ -> true + match (funct.exp_desc, !open_row_field_read) with + | Texp_object_get _, Some ty -> repr ty == repr funct.exp_type | _ -> false in if diff --git a/tests/build_tests/super_errors/expected/unused_argument_object_method.res.expected b/tests/build_tests/super_errors/expected/unused_argument_object_method.res.expected index 5592a9e2b8..e35a7844f5 100644 --- a/tests/build_tests/super_errors/expected/unused_argument_object_method.res.expected +++ b/tests/build_tests/super_errors/expected/unused_argument_object_method.res.expected @@ -6,5 +6,31 @@ 6 │ // A function that never returns ignores its arguments: warning 20. 7 │ let ignored = () => failwith("unreachable")(1) 8 │ + 9 │ // A field read that cannot return ignores its arguments: warning 20. + + this argument will not be used by the function. + + + Warning number 20 + /.../fixtures/unused_argument_object_method.res:10:56-71 + + 8 │ + 9 │ // A field read that cannot return ignores its arguments: warning 20. + 10 │ let fromLiteral = () => ({"m": failwith("boom")})["m"](Console.log("a") + │ ) + 11 │ let fromFailwith = () => failwith("boom")["m"](Console.log("b")) + 12 │ + + this argument will not be used by the function. + + + Warning number 20 + /.../fixtures/unused_argument_object_method.res:11:48-63 + + 9 │ // A field read that cannot return ignores its arguments: warning 20. + 10 │ let fromLiteral = () => ({"m": failwith("boom")})["m"](Console.log("a") + │ ) + 11 │ let fromFailwith = () => failwith("boom")["m"](Console.log("b")) + 12 │ this argument will not be used by the function. \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/unused_argument_object_method.res b/tests/build_tests/super_errors/fixtures/unused_argument_object_method.res index c30d0dc9ad..a285ddae00 100644 --- a/tests/build_tests/super_errors/fixtures/unused_argument_object_method.res +++ b/tests/build_tests/super_errors/fixtures/unused_argument_object_method.res @@ -5,3 +5,7 @@ let text = clipboardData("event")["getData"]("text/plain") // A function that never returns ignores its arguments: warning 20. let ignored = () => failwith("unreachable")(1) + +// A field read that cannot return ignores its arguments: warning 20. +let fromLiteral = () => ({"m": failwith("boom")})["m"](Console.log("a")) +let fromFailwith = () => failwith("boom")["m"](Console.log("b")) From 0ba40e760a8e7d0199c76f8a58bfa0eb80c573fc Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Tue, 20 Oct 2026 13:32:48 +0000 Subject: [PATCH 3/6] Test object field reads in arguments of open JS object methods Arguments that read object fields, from closed or open objects, in first or last position or through a nested method call, keep the call of an open JS object method free of warning 20. The exemption is decided before any argument is typed, so their reads cannot replace the callee's. let closedFirst = clipboardData("e")["m"](closed["x"], "a") let nested = clipboardData("e")["m"](clipboardData("f")["n"]("y")) Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel --- ...unused_argument_object_method.res.expected | 34 +++++++++---------- .../unused_argument_object_method.res | 10 ++++++ 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/tests/build_tests/super_errors/expected/unused_argument_object_method.res.expected b/tests/build_tests/super_errors/expected/unused_argument_object_method.res.expected index e35a7844f5..3e23b047e1 100644 --- a/tests/build_tests/super_errors/expected/unused_argument_object_method.res.expected +++ b/tests/build_tests/super_errors/expected/unused_argument_object_method.res.expected @@ -1,36 +1,36 @@ Warning number 20 - /.../fixtures/unused_argument_object_method.res:7:45 + /.../fixtures/unused_argument_object_method.res:17:45 - 5 │ - 6 │ // A function that never returns ignores its arguments: warning 20. - 7 │ let ignored = () => failwith("unreachable")(1) - 8 │ - 9 │ // A field read that cannot return ignores its arguments: warning 20. + 15 │ + 16 │ // A function that never returns ignores its arguments: warning 20. + 17 │ let ignored = () => failwith("unreachable")(1) + 18 │ + 19 │ // A field read that cannot return ignores its arguments: warning 20. this argument will not be used by the function. Warning number 20 - /.../fixtures/unused_argument_object_method.res:10:56-71 + /.../fixtures/unused_argument_object_method.res:20:56-71 - 8 │ - 9 │ // A field read that cannot return ignores its arguments: warning 20. - 10 │ let fromLiteral = () => ({"m": failwith("boom")})["m"](Console.log("a") + 18 │ + 19 │ // A field read that cannot return ignores its arguments: warning 20. + 20 │ let fromLiteral = () => ({"m": failwith("boom")})["m"](Console.log("a") │ ) - 11 │ let fromFailwith = () => failwith("boom")["m"](Console.log("b")) - 12 │ + 21 │ let fromFailwith = () => failwith("boom")["m"](Console.log("b")) + 22 │ this argument will not be used by the function. Warning number 20 - /.../fixtures/unused_argument_object_method.res:11:48-63 + /.../fixtures/unused_argument_object_method.res:21:48-63 - 9 │ // A field read that cannot return ignores its arguments: warning 20. - 10 │ let fromLiteral = () => ({"m": failwith("boom")})["m"](Console.log("a") + 19 │ // A field read that cannot return ignores its arguments: warning 20. + 20 │ let fromLiteral = () => ({"m": failwith("boom")})["m"](Console.log("a") │ ) - 11 │ let fromFailwith = () => failwith("boom")["m"](Console.log("b")) - 12 │ + 21 │ let fromFailwith = () => failwith("boom")["m"](Console.log("b")) + 22 │ this argument will not be used by the function. \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/unused_argument_object_method.res b/tests/build_tests/super_errors/fixtures/unused_argument_object_method.res index a285ddae00..de7dc0db1d 100644 --- a/tests/build_tests/super_errors/fixtures/unused_argument_object_method.res +++ b/tests/build_tests/super_errors/fixtures/unused_argument_object_method.res @@ -3,6 +3,16 @@ // A method of an open JS object type receives its arguments: no warning 20. let text = clipboardData("event")["getData"]("text/plain") +// Arguments that read object fields do not change that. +let closed: {"x": string} = {"x": "x"} +let closedArg = clipboardData("e")["getData"](closed["x"]) +let openArg = clipboardData("e")["getData"](clipboardData("o")["x"]) +let closedLast = clipboardData("e")["m"]("a", closed["x"]) +let closedFirst = clipboardData("e")["m"](closed["x"], "a") +let openLast = clipboardData("e")["m"]("a", clipboardData("o")["x"]) +let openFirst = clipboardData("e")["m"](clipboardData("o")["x"], "a") +let nested = clipboardData("e")["m"](clipboardData("f")["n"]("y")) + // A function that never returns ignores its arguments: warning 20. let ignored = () => failwith("unreachable")(1) From b17a06794d4e1d1dcde4a7d4ccf9c12329ff9722 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Tue, 20 Oct 2026 13:46:51 +0000 Subject: [PATCH 4/6] Use throw in the unused-argument object method fixture The fixture writes its never-returning expressions as `throw(Failure(...))` instead of the deprecated `failwith`, so its expected output holds only warning 20 once `failwith` reports warning 3. It also covers an open object kept in a binding whose method is read before it is called: that call reports no warning 20 either. let retained = clipboardData("e") let first = retained["m"] let second = retained["m"]("used") Review: Does the retained case need a record of every open-row field variable? No: a binding keeps the object type at the binding's level, below the level of the later call, so the call's fresh-variable check does not apply and no warning is reported. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel --- ...unused_argument_object_method.res.expected | 38 +++++++++---------- .../unused_argument_object_method.res | 17 +++++++-- 2 files changed, 33 insertions(+), 22 deletions(-) diff --git a/tests/build_tests/super_errors/expected/unused_argument_object_method.res.expected b/tests/build_tests/super_errors/expected/unused_argument_object_method.res.expected index 3e23b047e1..3b74ee7a83 100644 --- a/tests/build_tests/super_errors/expected/unused_argument_object_method.res.expected +++ b/tests/build_tests/super_errors/expected/unused_argument_object_method.res.expected @@ -1,36 +1,36 @@ Warning number 20 - /.../fixtures/unused_argument_object_method.res:17:45 + /.../fixtures/unused_argument_object_method.res:28:51 - 15 │ - 16 │ // A function that never returns ignores its arguments: warning 20. - 17 │ let ignored = () => failwith("unreachable")(1) - 18 │ - 19 │ // A field read that cannot return ignores its arguments: warning 20. + 26 │ + 27 │ // A function that never returns ignores its arguments: warning 20. + 28 │ let ignored = () => throw(Failure("unreachable"))(1) + 29 │ + 30 │ // A field read that cannot return ignores its arguments: warning 20. this argument will not be used by the function. Warning number 20 - /.../fixtures/unused_argument_object_method.res:20:56-71 + /.../fixtures/unused_argument_object_method.res:31:62-77 - 18 │ - 19 │ // A field read that cannot return ignores its arguments: warning 20. - 20 │ let fromLiteral = () => ({"m": failwith("boom")})["m"](Console.log("a") - │ ) - 21 │ let fromFailwith = () => failwith("boom")["m"](Console.log("b")) - 22 │ + 29 │ + 30 │ // A field read that cannot return ignores its arguments: warning 20. + 31 │ let fromLiteral = () => ({"m": throw(Failure("boom"))})["m"](Console.lo + │ g("a")) + 32 │ let fromThrow = () => throw(Failure("boom"))["m"](Console.log("b")) + 33 │ this argument will not be used by the function. Warning number 20 - /.../fixtures/unused_argument_object_method.res:21:48-63 + /.../fixtures/unused_argument_object_method.res:32:51-66 - 19 │ // A field read that cannot return ignores its arguments: warning 20. - 20 │ let fromLiteral = () => ({"m": failwith("boom")})["m"](Console.log("a") - │ ) - 21 │ let fromFailwith = () => failwith("boom")["m"](Console.log("b")) - 22 │ + 30 │ // A field read that cannot return ignores its arguments: warning 20. + 31 │ let fromLiteral = () => ({"m": throw(Failure("boom"))})["m"](Console.lo + │ g("a")) + 32 │ let fromThrow = () => throw(Failure("boom"))["m"](Console.log("b")) + 33 │ this argument will not be used by the function. \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/unused_argument_object_method.res b/tests/build_tests/super_errors/fixtures/unused_argument_object_method.res index de7dc0db1d..86d6ecc4c8 100644 --- a/tests/build_tests/super_errors/fixtures/unused_argument_object_method.res +++ b/tests/build_tests/super_errors/fixtures/unused_argument_object_method.res @@ -13,9 +13,20 @@ let openLast = clipboardData("e")["m"]("a", clipboardData("o")["x"]) let openFirst = clipboardData("e")["m"](clipboardData("o")["x"], "a") let nested = clipboardData("e")["m"](clipboardData("f")["n"]("y")) +// Reading the method before calling it does not change that either. +let retained = clipboardData("e") +let first = retained["m"] +let second = retained["m"]("used") +let readElsewhere = () => { + let o = clipboardData("e") + let get = () => o["m"] + o["m"]("used")->ignore + get +} + // A function that never returns ignores its arguments: warning 20. -let ignored = () => failwith("unreachable")(1) +let ignored = () => throw(Failure("unreachable"))(1) // A field read that cannot return ignores its arguments: warning 20. -let fromLiteral = () => ({"m": failwith("boom")})["m"](Console.log("a")) -let fromFailwith = () => failwith("boom")["m"](Console.log("b")) +let fromLiteral = () => ({"m": throw(Failure("boom"))})["m"](Console.log("a")) +let fromThrow = () => throw(Failure("boom"))["m"](Console.log("b")) From a38596ed9750dcf0954e8a2252621dc5c55fa0e2 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Wed, 21 Oct 2026 04:32:20 +0000 Subject: [PATCH 5/6] Pass the open-row method check to type_application explicitly The Pexp_apply case resets open_row_field_read before typing the callee, reads it right after and passes the result to type_application as ~open_row_method, so the global is no longer read where other expressions may have been typed in between. Behavior is unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel --- compiler/ml/typecore.ml | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/compiler/ml/typecore.ml b/compiler/ml/typecore.ml index b02114d624..a6674223ef 100644 --- a/compiler/ml/typecore.ml +++ b/compiler/ml/typecore.ml @@ -2512,9 +2512,9 @@ type targs = (Asttypes.arg_label * Typedtree.expression option) list (* The type of the most recent object field read if that read added the field to an open object row ([obj["m"]] with [obj: {..}]), [None] after any other - read. [type_application] compares it with its callee's type to tell such a - JS method, whose type is a fresh variable, from a callee that never - returns. *) + read. Only the [Pexp_apply] case reads it: it resets it before typing the + callee and checks it right after, where it is only meaningful when the + callee itself is such a read. *) let open_row_field_read : type_expr option ref = ref None (* Eliminate an object-field scheme for a read. A declared field payload is @@ -2765,9 +2765,19 @@ and type_expect_ ?deprecated_context ~context ?(recarg = Rejected) env sexp assert (sargs <> []); begin_def (); (* one more level for non-returning functions *) + open_row_field_read := None; let funct = type_exp ~deprecated_context:FunctionCall ~context:None env sfunct in + (* Whether the callee is a JS object method that its read added to an open + object row ([obj["m"]] with [obj: {..}]). Its type is a fresh variable + like that of a callee that never returns, but the method receives its + arguments, so [type_application] must not report them as unused. *) + let open_row_method = + match (funct.exp_desc, !open_row_field_read) with + | Texp_object_get _, Some ty -> repr ty == repr funct.exp_type + | _ -> false + in (if should_unify_expected_result_before_typing_lowered_apply funct sargs then (* The async wrapper is lowered to a regular application, so thread the @@ -2790,7 +2800,8 @@ and type_expect_ ?deprecated_context ~context ?(recarg = Rejected) env sexp let args, ty_res, fully_applied = match translate_unified_ops env funct sargs with | Some (targs, result_type) -> (targs, result_type, true) - | None -> type_application ~context total_app env funct sargs + | None -> + type_application ~context ~open_row_method total_app env funct sargs in end_def (); if transformed_jsx then check_jsx_props ~loc env args; @@ -4469,8 +4480,8 @@ and translate_unified_ops (env : Env.t) (funct : Typedtree.expression) | _ -> None) | _ -> None -and type_application ~context total_app env funct (sargs : sargs) : - targs * Types.type_expr * bool = +and type_application ~context ~open_row_method total_app env funct + (sargs : sargs) : targs * Types.type_expr * bool = match sargs with (* Special case for ignore: avoid discarding warning *) | [(Nolabel, sarg)] when is_ignore ~env funct -> @@ -4503,17 +4514,12 @@ and type_application ~context total_app env funct (sargs : sargs) : never returns (e.g. [failwith("x")(1)]), so the arguments are unused. A JS object method that the read added to an open object row ([obj["m"]] with [obj: {..}]) also gets a fresh variable, but it - is an ordinary JS method that receives its arguments. The callee's - read is the last one typed, so [open_row_field_read] belongs to it. *) - let is_object_method = - match (funct.exp_desc, !open_row_field_read) with - | Texp_object_get _, Some ty -> repr ty == repr funct.exp_type - | _ -> false - in + is an ordinary JS method that receives its arguments + ([open_row_method]). *) if ty_fun.level >= ty_ret.level && (not (is_identity_coercion funct.exp_desc)) - && not is_object_method + && not open_row_method then List.iter (fun (_, sarg1) -> From 5e83724db26f636de4b209877844c02254f718e0 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Wed, 21 Oct 2026 10:56:54 +0000 Subject: [PATCH 6/6] Keep the open-row field read marker request-local Requests type-checked concurrently on separate domains shared the open_row_field_read ref, so one request could overwrite another's marker between typing a callee and reading it, and report warning 20 on an open JS object method call. The marker is now a Request_local key. A compiler request test compiles open-object method calls in two concurrent requests with warning 20 enabled and expects no warning; it failed on every run with the shared ref. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel --- compiler/ml/typecore.ml | 13 +++++--- .../ounit_compiler_request_tests.ml | 33 +++++++++++++++++++ 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/compiler/ml/typecore.ml b/compiler/ml/typecore.ml index a6674223ef..d111c62367 100644 --- a/compiler/ml/typecore.ml +++ b/compiler/ml/typecore.ml @@ -2514,8 +2514,10 @@ type targs = (Asttypes.arg_label * Typedtree.expression option) list to an open object row ([obj["m"]] with [obj: {..}]), [None] after any other read. Only the [Pexp_apply] case reads it: it resets it before typing the callee and checks it right after, where it is only meaningful when the - callee itself is such a read. *) -let open_row_field_read : type_expr option ref = ref None + callee itself is such a read. Request-local, so requests type-checked + concurrently on other domains cannot overwrite it in between. *) +let open_row_field_read : type_expr option ref Request_local.key = + Request_local.new_key (fun () -> ref None) (* Eliminate an object-field scheme for a read. A declared field payload is wrapped in [Tpoly]: an empty binder list needs ordinary instantiation, while @@ -2765,7 +2767,7 @@ and type_expect_ ?deprecated_context ~context ?(recarg = Rejected) env sexp assert (sargs <> []); begin_def (); (* one more level for non-returning functions *) - open_row_field_read := None; + Request_local.get open_row_field_read := None; let funct = type_exp ~deprecated_context:FunctionCall ~context:None env sfunct in @@ -2774,7 +2776,7 @@ and type_expect_ ?deprecated_context ~context ?(recarg = Rejected) env sexp like that of a callee that never returns, but the method receives its arguments, so [type_application] must not report them as unused. *) let open_row_method = - match (funct.exp_desc, !open_row_field_read) with + match (funct.exp_desc, !(Request_local.get open_row_field_read)) with | Texp_object_get _, Some ty -> repr ty == repr funct.exp_type | _ -> false in @@ -3675,7 +3677,8 @@ and type_expect_ ?deprecated_context ~context ?(recarg = Rejected) env sexp requiring or adding write capability. *) let typ = filter_method env met obj.exp_type in let typ = object_field_use_type env typ in - open_row_field_read := if adds_field_to_row then Some typ else None; + Request_local.get open_row_field_read + := if adds_field_to_row then Some typ else None; rue { exp_desc = Texp_object_get (obj, met_loc); diff --git a/tests/ounit_tests/ounit_compiler_request_tests.ml b/tests/ounit_tests/ounit_compiler_request_tests.ml index cac6ab4ff7..dd5b5eeabd 100644 --- a/tests/ounit_tests/ounit_compiler_request_tests.ml +++ b/tests/ounit_tests/ounit_compiler_request_tests.ml @@ -1505,6 +1505,38 @@ let concurrent_jsx_diagnostic_tests _context = assert_equal right_expected.stderr (Domain.join right).stderr done) +(* Calls of open JS object methods mark their callee in request-local typing + state. Another request typing applications at the same time must not + overwrite that mark, which would report the method's arguments as unused + (warning 20, off by default and enabled here). *) +let concurrent_open_object_method_tests _context = + with_temp_dir "rewatch-driver-open-method-" (fun root -> + let source = + "@get external obj: string => {..} = \"obj\"\n" + ^ String.concat "" + (List.init 200 (fun i -> + Printf.sprintf "let v%d = obj(\"e\")[\"m\"](\"a\", \"b\")\n" i)) + in + let setup name = + let directory = Filename.concat root name in + ensure_dir directory; + write directory "Calls.res" source; + fun () -> snd (run directory ~extra:["-w"; "+20"] "Calls.res") + in + let left_request = setup "left" in + let right_request = setup "right" in + let expect_clean result = + expect_code 0 result; + assert_equal ~printer:Fun.id "" result.Rescript_compiler_driver.stderr + in + expect_clean (left_request ()); + for _ = 1 to 5 do + let left = Domain.spawn left_request in + let right = Domain.spawn right_request in + expect_clean (Domain.join left); + expect_clean (Domain.join right) + done) + (* Exercise concurrent compiler requests without the process-worker boundary. *) let concurrent_request_isolation_tests _context = with_temp_dir "rewatch-driver-domains-" (fun root -> @@ -1710,6 +1742,7 @@ let suites = "concurrent_diagnostic_recovery" >:: concurrent_diagnostic_recovery_tests; "concurrent_jsx_diagnostic" >:: concurrent_jsx_diagnostic_tests; + "concurrent_open_object_method" >:: concurrent_open_object_method_tests; "concurrent_request_isolation" >:: concurrent_request_isolation_tests; "concurrent_async_context" >:: concurrent_async_context_tests; ]