Skip to content

Commit afec64d

Browse files
cknittclaude
andcommitted
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel <ck@cca.io>
1 parent 32aa190 commit afec64d

2 files changed

Lines changed: 41 additions & 5 deletions

File tree

‎compiler/ml/typecore.ml‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2519,8 +2519,10 @@ type targs = (Asttypes.arg_label * Typedtree.expression option) list
25192519
to an open object row ([obj["m"]] with [obj: {..}]), [None] after any other
25202520
read. Only the [Pexp_apply] case reads it: it resets it before typing the
25212521
callee and checks it right after, where it is only meaningful when the
2522-
callee itself is such a read. *)
2523-
let open_row_field_read : type_expr option ref = ref None
2522+
callee itself is such a read. Request-local, so requests type-checked
2523+
concurrently on other domains cannot overwrite it in between. *)
2524+
let open_row_field_read : type_expr option ref Request_local.key =
2525+
Request_local.new_key (fun () -> ref None)
25242526
25252527
(* Eliminate an object-field scheme for a read. A declared field payload is
25262528
wrapped in [Tpoly]: an empty binder list needs ordinary instantiation, while
@@ -2790,7 +2792,7 @@ and type_expect_ ?deprecated_context ~context ?(recarg = Rejected)
27902792
assert (sargs <> []);
27912793
begin_def ();
27922794
(* one more level for non-returning functions *)
2793-
open_row_field_read := None;
2795+
Request_local.get open_row_field_read := None;
27942796
let funct =
27952797
type_exp ~deprecated_context:FunctionCall ~context:None env sfunct
27962798
in
@@ -2799,7 +2801,7 @@ and type_expect_ ?deprecated_context ~context ?(recarg = Rejected)
27992801
like that of a callee that never returns, but the method receives its
28002802
arguments, so [type_application] must not report them as unused. *)
28012803
let open_row_method =
2802-
match (funct.exp_desc, !open_row_field_read) with
2804+
match (funct.exp_desc, !(Request_local.get open_row_field_read)) with
28032805
| Texp_object_get _, Some ty -> repr ty == repr funct.exp_type
28042806
| _ -> false
28052807
in
@@ -3699,7 +3701,8 @@ and type_expect_ ?deprecated_context ~context ?(recarg = Rejected)
36993701
requiring or adding write capability. *)
37003702
let typ = filter_method env met obj.exp_type in
37013703
let typ = object_field_use_type env typ in
3702-
open_row_field_read := if adds_field_to_row then Some typ else None;
3704+
Request_local.get open_row_field_read
3705+
:= if adds_field_to_row then Some typ else None;
37033706
rue
37043707
{
37053708
exp_desc = Texp_object_get (obj, met_loc);

‎tests/ounit_tests/ounit_compiler_request_tests.ml‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1505,6 +1505,38 @@ let concurrent_jsx_diagnostic_tests _context =
15051505
assert_equal right_expected.stderr (Domain.join right).stderr
15061506
done)
15071507

1508+
(* Calls of open JS object methods mark their callee in request-local typing
1509+
state. Another request typing applications at the same time must not
1510+
overwrite that mark, which would report the method's arguments as unused
1511+
(warning 20, off by default and enabled here). *)
1512+
let concurrent_open_object_method_tests _context =
1513+
with_temp_dir "rewatch-driver-open-method-" (fun root ->
1514+
let source =
1515+
"@get external obj: string => {..} = \"obj\"\n"
1516+
^ String.concat ""
1517+
(List.init 200 (fun i ->
1518+
Printf.sprintf "let v%d = obj(\"e\")[\"m\"](\"a\", \"b\")\n" i))
1519+
in
1520+
let setup name =
1521+
let directory = Filename.concat root name in
1522+
ensure_dir directory;
1523+
write directory "Calls.res" source;
1524+
fun () -> snd (run directory ~extra:["-w"; "+20"] "Calls.res")
1525+
in
1526+
let left_request = setup "left" in
1527+
let right_request = setup "right" in
1528+
let expect_clean result =
1529+
expect_code 0 result;
1530+
assert_equal ~printer:Fun.id "" result.Rescript_compiler_driver.stderr
1531+
in
1532+
expect_clean (left_request ());
1533+
for _ = 1 to 5 do
1534+
let left = Domain.spawn left_request in
1535+
let right = Domain.spawn right_request in
1536+
expect_clean (Domain.join left);
1537+
expect_clean (Domain.join right)
1538+
done)
1539+
15081540
(* Exercise concurrent compiler requests without the process-worker boundary. *)
15091541
let concurrent_request_isolation_tests _context =
15101542
with_temp_dir "rewatch-driver-domains-" (fun root ->
@@ -1710,6 +1742,7 @@ let suites =
17101742
"concurrent_diagnostic_recovery"
17111743
>:: concurrent_diagnostic_recovery_tests;
17121744
"concurrent_jsx_diagnostic" >:: concurrent_jsx_diagnostic_tests;
1745+
"concurrent_open_object_method" >:: concurrent_open_object_method_tests;
17131746
"concurrent_request_isolation" >:: concurrent_request_isolation_tests;
17141747
"concurrent_async_context" >:: concurrent_async_context_tests;
17151748
]

0 commit comments

Comments
 (0)