From 992088e2208e24227504046bc2f1654572890d9d Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 4 Sep 2025 12:21:04 +0200 Subject: [PATCH 1/8] compiler: update typecore; tests: add VariantCoercionConstructUsed; docs: update AGENTS; docstring_tests: update DocTest outputs Signed-off-by: Cristiano Calcagno --- AGENTS.md | 26 ++++++++++++++-- compiler/ml/typecore.ml | 30 +++++++++++++++++++ tests/docstring_tests/DocTest.res | 6 +++- tests/docstring_tests/DocTest.res.js | 4 ++- .../src/VariantCoercionConstructUsed.mjs | 11 +++++++ .../src/VariantCoercionConstructUsed.res | 12 ++++++++ 6 files changed, 84 insertions(+), 5 deletions(-) create mode 100644 tests/tests/src/VariantCoercionConstructUsed.mjs create mode 100644 tests/tests/src/VariantCoercionConstructUsed.res diff --git a/AGENTS.md b/AGENTS.md index 937ed9585d..71c9851c97 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,6 +1,6 @@ -# CLAUDE.md +# AGENTS.md -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. +Shared guidance for code agents (Claude, Gemini, Codex, etc.) working in this repository. The files `CLAUDE.md` and `GEMINI.md` are symlinks to this document. For Codex‑specific workflow details also see `CODEX.md`. ## Project Overview @@ -136,4 +136,24 @@ The compiler is designed for fast feedback loops and scales to large codebases. - Avoid introducing meaningless symbols - Maintain readable JavaScript output - Consider compilation speed impact -- Use appropriate optimization passes in the Lambda and JS IRs \ No newline at end of file +- Use appropriate optimization passes in the Lambda and JS IRs + +## Agent Notes and Gotchas + +- **Node version features:** Runtime docstring tests conditionally enable features by Node major version: + - 20+: array `toReversed`/`toSorted` + - 22+: new `Set` APIs and `Promise.withResolvers` + - 24+: `RegExp.escape` + Tests auto‑skip unsupported features based on `process.version`. +- **CPU count in sandboxes:** Some CI/sandboxed environments report `os.cpus().length === 0`. + - Clamp concurrency/batch size to at least 1 when using `os.cpus()`. + - This pattern is used in `tests/docstring_tests/DocTest.res` and `cli/rescript-legacy/format.js`. +- **Formatting in tests:** `make test` checks formatting (OCaml, ReScript, JS, Rust). If it fails locally, run `make format` and re‑run tests. +- **Executables location:** Build copies platform binaries into `packages/@rescript//bin/` and convenience folders like `darwinarm64/`. +- **Direct dune usage:** You can use `dune build`/`dune build -w`, but prefer `make` targets which also copy executables. + +## References + +- `CODEX.md`: detailed setup, build, testing, and workflows for agented development +- `README.md`: high‑level repo overview and usage +- `Makefile`: authoritative list of build/test targets diff --git a/compiler/ml/typecore.ml b/compiler/ml/typecore.ml index 90522f13d2..045f800b66 100644 --- a/compiler/ml/typecore.ml +++ b/compiler/ml/typecore.ml @@ -3018,6 +3018,36 @@ and type_expect_ ~context ?in_function ?(recarg = Rejected) env sexp ty_expected raise (Error (loc, env, Not_subtype (tr1, tr2, ctx)))); (arg, ty', cty') in + (* After a successful coercion, if we coerced between concrete variant + types, mark the intersection of constructors as positively used for the + target type to avoid spurious "constructor ... is never used" warnings + when values are introduced via coercions. *) + (match + ( (try Some (Ctype.extract_concrete_typedecl env arg.exp_type) + with Not_found -> None), + try Some (Ctype.extract_concrete_typedecl env ty') + with Not_found -> None ) + with + | Some (_p_src, _p_src_conc, src_decl), Some (_p_tgt, p_tgt_conc, tgt_decl) + -> ( + match (src_decl.type_kind, tgt_decl.type_kind) with + | Type_variant src_cons, Type_variant tgt_cons -> + let src_names = + List.map + (fun (c : Types.constructor_declaration) -> Ident.name c.cd_id) + src_cons + in + let has_src name = List.exists (fun n -> n = name) src_names in + let tgt_ty_name = Path.last p_tgt_conc in + List.iter + (fun (c : Types.constructor_declaration) -> + let cname = Ident.name c.cd_id in + if has_src cname then + Env.mark_constructor_used Env.Positive env tgt_ty_name tgt_decl + cname) + tgt_cons + | _ -> ()) + | _ -> ()); rue { exp_desc = arg.exp_desc; diff --git a/tests/docstring_tests/DocTest.res b/tests/docstring_tests/DocTest.res index dff139cf0f..df893a0be2 100644 --- a/tests/docstring_tests/DocTest.res +++ b/tests/docstring_tests/DocTest.res @@ -62,7 +62,11 @@ let extractDocFromFile = async file => { } } -let batchSize = OS.cpus()->Array.length +// Some environments may report 0 CPUs; clamp to at least 1 to avoid zero-sized batches +let batchSize = switch OS.cpus()->Array.length { +| n if n > 0 => n +| _ => 1 +} let runtimePath = Path.join(["packages", "@rescript", "runtime"]) diff --git a/tests/docstring_tests/DocTest.res.js b/tests/docstring_tests/DocTest.res.js index 12b54bf200..0e42d8c0b9 100644 --- a/tests/docstring_tests/DocTest.res.js +++ b/tests/docstring_tests/DocTest.res.js @@ -67,7 +67,9 @@ async function extractDocFromFile(file) { } } -let batchSize = Nodeos.cpus().length; +let n = Nodeos.cpus().length; + +let batchSize = n > 0 ? n : 1; let runtimePath = Nodepath.join("packages", "@rescript", "runtime"); diff --git a/tests/tests/src/VariantCoercionConstructUsed.mjs b/tests/tests/src/VariantCoercionConstructUsed.mjs new file mode 100644 index 0000000000..3cf18ee208 --- /dev/null +++ b/tests/tests/src/VariantCoercionConstructUsed.mjs @@ -0,0 +1,11 @@ +// Generated by ReScript, PLEASE EDIT WITH CARE + + +function upcast(x) { + return x; +} + +export { + upcast, +} +/* No side effect */ diff --git a/tests/tests/src/VariantCoercionConstructUsed.res b/tests/tests/src/VariantCoercionConstructUsed.res new file mode 100644 index 0000000000..55cec8c108 --- /dev/null +++ b/tests/tests/src/VariantCoercionConstructUsed.res @@ -0,0 +1,12 @@ +// Repro for incorrect "constructor ... is never used" warning with coercions +// This should compile cleanly without warnings when coercing from a -> b. + +type a = A | B + +type b = + | ...a + | C + +let upcast = (x: a): b => (x :> b) + +let _ = upcast(A) From 7f1f3c49d667c3ca4126d7c74d4fbadb0cb25f0b Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 4 Sep 2025 13:37:19 +0200 Subject: [PATCH 2/8] next iter --- compiler/ml/typecore.ml | 27 ++++++++--- example.res | 10 ++++ issue.txt | 46 +++++++++++++++++++ .../src/VariantCoercionConstructUsed.res | 15 +++--- 4 files changed, 86 insertions(+), 12 deletions(-) create mode 100644 example.res create mode 100644 issue.txt diff --git a/compiler/ml/typecore.ml b/compiler/ml/typecore.ml index 045f800b66..d43f10848a 100644 --- a/compiler/ml/typecore.ml +++ b/compiler/ml/typecore.ml @@ -3028,8 +3028,8 @@ and type_expect_ ~context ?in_function ?(recarg = Rejected) env sexp ty_expected try Some (Ctype.extract_concrete_typedecl env ty') with Not_found -> None ) with - | Some (_p_src, _p_src_conc, src_decl), Some (_p_tgt, p_tgt_conc, tgt_decl) - -> ( + | Some (_p_src, _p_src_conc, src_decl), + Some (p_tgt, p_tgt_conc, tgt_decl) -> ( match (src_decl.type_kind, tgt_decl.type_kind) with | Type_variant src_cons, Type_variant tgt_cons -> let src_names = @@ -3038,14 +3038,29 @@ and type_expect_ ~context ?in_function ?(recarg = Rejected) env sexp ty_expected src_cons in let has_src name = List.exists (fun n -> n = name) src_names in - let tgt_ty_name = Path.last p_tgt_conc in + let tgt_ty_name_conc = Path.last p_tgt_conc in + (* Mark usage for the concrete target decl (implementation view). *) List.iter (fun (c : Types.constructor_declaration) -> let cname = Ident.name c.cd_id in if has_src cname then - Env.mark_constructor_used Env.Positive env tgt_ty_name tgt_decl - cname) - tgt_cons + Env.mark_constructor_used Env.Positive env tgt_ty_name_conc + tgt_decl cname) + tgt_cons; + (* If the target type path differs from its concrete decl (e.g., + when a signature exposes a private or abstract alias), also mark + usage on the exposed target declaration so scheduled warnings + attached to that declaration are cleared. *) + if not (Path.same p_tgt p_tgt_conc) then ( + let exposed_ty_name = Path.last p_tgt in + let exposed_decl = Env.find_type p_tgt env in + List.iter + (fun (c : Types.constructor_declaration) -> + let cname = Ident.name c.cd_id in + if has_src cname then + Env.mark_constructor_used Env.Positive env exposed_ty_name + exposed_decl cname) + tgt_cons) | _ -> ()) | _ -> ()); rue diff --git a/example.res b/example.res new file mode 100644 index 0000000000..ce214600f4 --- /dev/null +++ b/example.res @@ -0,0 +1,10 @@ +type a = A | B + +// b spreads a and adds one more constructor +type b = + | ...a + | C + +let upcast = (x: a): b => (x :> b) + +let _ = upcast(A) diff --git a/issue.txt b/issue.txt new file mode 100644 index 0000000000..5a064b0c48 --- /dev/null +++ b/issue.txt @@ -0,0 +1,46 @@ +found a compiler bug: https://github.com/rescript-lang/rescript/issues/7838 (due to the coercing between variants) +GitHub +Incorrect compiler warning (constructor is never used to build valu... +https://rescript-lang.org/try?version=v11.1.4&module=esmodule&code=LYewJgrgNgpgBAUQG4wHYBc4F44G8BQcc6AngA7yY4A+iKGAgvoXLJgBYCGqYs2cAChj10ALmIBKbAD48LIgGcA7gEt0AY3ZxhaTASJFayXQxmCJ8uAF8WNm-l... +Incorrect compiler warning (constructor is never used to build valu... +[09:34]Gabriel Nordeborn: What is wrong here...? +[09:35]jaap: The warning? +[09:36]jaap: The variants are constructed in handle +[09:37]jaap: I mean, the coercion constructs the variants +[09:38]jaap: so if we have variantA :> variantB, we can mark that variantB is always constructed +[09:38]Gabriel Nordeborn: Right, so if you adapt the example to not use coercion, it marks them as constructed? +[09:39]Gabriel Nordeborn: It's good if you write this in the PR issue +[09:39]Gabriel Nordeborn: Right now, just looking at the example, I did not understand what you're referring to +[09:41]jaap: Yes +[09:41]jaap: https://rescript-lang.org/try?version=v11.1.4&module=esmodule&code=LYewJgrgNgpgBAUQG4wHYBc4F44G8BQcc6AngA7yY4A+iKGAgvoXLJgBYCGqYs2cAChj10ALmIBKbAD48LIgGcA7gEt0AY3ZxhaTASJFayXQxmCJ8uAF8WNm-lCQ+AZRgAnFG+MYA4iHDi+qwwmGgAjhAwkeLe6AB0VLIQqGr4VvxBpBTE2Ja0cQWxCcxEbHBcPHw4nAokqOqCOhji6FJYskGKqhpaTXp5dCZmAhYG1rYsZeGRkfxCIjEiCW2yFbwwAspqmtoicoaDjGaxTERWEgC00ioA5qggbjBpzI7Q8K4e7rEAQpxggZMQtpUBEojBFroEmZkql0jhMuRKLkxvlCkt0CVghxuOt+DU6g15roWit9gYtj1droyQcTsNRgYbGdAaEQTN4Dgic1DvFWmY1rAuZhRLJWldbvdHs8gA +ReScript Documentation +ReScript Playground +Try ReScript in the browser +ReScript Playground +[09:41]jaap: here with two examples +[09:41]jaap: also updated the ticket +[09:44]cristianoc: The type system was created with no coercion in mind. +[09:45]cristianoc: So fix warning —-> compiler error? +[09:47]Gabriel Nordeborn: @jaap is this not a better example? https://rescript-lang.org/try?version=v11.1.4&module=esmodule&code=LYewJgrgNgpgBAUQG4wHYBc4F44G8BQcc6AngA7yY7JroCC+hcsmAFgIapizZwAUMFBgBcxAJTYAfHiZEAzgHcAlugDGrOINoyiRAD6Ih9KfzGy4AXyZWr+UJB4BlGACcULmhgDiIcKIJEpBTEvAYAdBGe6GHojEQsmqgAjhAwqaJU0hCoKvgWvAHE5JShcBFhUTFxzDBsnNzwOOxyJKiq-Foi4iaF8spqGp2YvXAGUXQmfGa6ltZMCWgpaY0dRhkSWNIcXLACRmIAtJJKAOaoIC4weYz20PDObq5RAELsYP7ztYlL6Ya0MSZsrl8jhCkESjhwpEjFVPnUditmq12ntaOseuZFCp1JojDpdGMjBNNqZzFYiOSaphFqlUrxUV1KugNlt6rsGZhhNJmYdjmcLlcrEA +ReScript Documentation +ReScript Playground +Try ReScript in the browser +ReScript Playground +[09:47]Gabriel Nordeborn: Using the switch fixes the warning of course because you're now constructing it explicitly +[09:47]Gabriel Nordeborn: If you were to add the switch instead of coercion that'd work as well +[09:48]Gabriel Nordeborn: But from what I understand the problem is that there's a value of a type that gets coerced to another type, and that other type is not marked as used +[09:49]Gabriel Nordeborn: The value that's not coerced is marked, but the new type from the coercion is not marked +[10:35]jaap: @cristianoc there is no way to fix this warning - you need to convert it to a switch expression, but that is a lot of redudant code potentially +[10:36]cristianoc: That’s actually better. So the message is at least not inconsistent. +[10:36]jaap: but the tricky part is that when coercing, the members that are in variantA need to marked as used, but not the new ones +[10:36]cristianoc: I was worried: you remove the case, then get a compile error +[10:37]jaap: Yes you do get an error - so it's not swallowing that +[10:37]jaap: if you include it (warning) if you remove it (error) +[10:38]cristianoc: There’s no such thing in the compiler. As ocaml does not have that. So it will be a genuine extension to handle this. +And I don’t even know if possible. +Is this global property? If so the. Compiler can’t do it. +[10:38]jaap: but yeah seems to be a gap in the type system after adding the nice spreading of variants and coercing +[10:39]jaap: No it's local - in the example you see the module signature added, so it only happens if you make the type private, and then coerce it +[10:40]jaap: I guess there is some mechanism to mark members as constructed - and coercion is not doing that +[10:40]cristianoc: Ok this is enough discussion. +Paste it in a prompt and see if it gets close. diff --git a/tests/tests/src/VariantCoercionConstructUsed.res b/tests/tests/src/VariantCoercionConstructUsed.res index 55cec8c108..7aeada489a 100644 --- a/tests/tests/src/VariantCoercionConstructUsed.res +++ b/tests/tests/src/VariantCoercionConstructUsed.res @@ -2,11 +2,14 @@ // This should compile cleanly without warnings when coercing from a -> b. type a = A | B +module DoNotWarn: { + let log: a => unit +} = { + type b = + | ...a + | C -type b = - | ...a - | C + let log = (x: a) => Js.log((x :> b)) +} -let upcast = (x: a): b => (x :> b) - -let _ = upcast(A) +let _ = DoNotWarn.log(A) From 1844a6cb3dd5ea5d17e7563d28f4e2066e831683 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 4 Sep 2025 13:42:07 +0200 Subject: [PATCH 3/8] typecore: mark constructors used on variant coercion; add super-errors test for C warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix spurious “constructor … is never used” warnings when values are introduced via coercions between concrete variant types. - After a successful coercion, mark the intersection of constructors as positively used for both: - the concrete target declaration, and - the exposed target alias (e.g. private/abstract signature path) when it differs. - Preserve warnings for constructors only present on the target side (e.g. C) — they remain unused and should warn. Tests: - Add super errors fixture ensuring C still warns: - fixtures: tests/build_tests/super_errors/fixtures/VariantCoercionWarnOnC.res - expected: tests/build_tests/super_errors/expected/VariantCoercionWarnOnC.res.expected Build: - Compiler builds cleanly; super-errors snapshot matches. --- compiler/ml/typecore.ml | 8 ++++---- .../expected/VariantCoercionWarnOnC.res.expected | 13 +++++++++++++ .../fixtures/VariantCoercionWarnOnC.res | 16 ++++++++++++++++ tests/tests/src/VariantCoercionConstructUsed.mjs | 14 ++++++++++---- 4 files changed, 43 insertions(+), 8 deletions(-) create mode 100644 tests/build_tests/super_errors/expected/VariantCoercionWarnOnC.res.expected create mode 100644 tests/build_tests/super_errors/fixtures/VariantCoercionWarnOnC.res diff --git a/compiler/ml/typecore.ml b/compiler/ml/typecore.ml index d43f10848a..490ebfd425 100644 --- a/compiler/ml/typecore.ml +++ b/compiler/ml/typecore.ml @@ -3028,8 +3028,8 @@ and type_expect_ ~context ?in_function ?(recarg = Rejected) env sexp ty_expected try Some (Ctype.extract_concrete_typedecl env ty') with Not_found -> None ) with - | Some (_p_src, _p_src_conc, src_decl), - Some (p_tgt, p_tgt_conc, tgt_decl) -> ( + | Some (_p_src, _p_src_conc, src_decl), Some (p_tgt, p_tgt_conc, tgt_decl) + -> ( match (src_decl.type_kind, tgt_decl.type_kind) with | Type_variant src_cons, Type_variant tgt_cons -> let src_names = @@ -3051,7 +3051,7 @@ and type_expect_ ~context ?in_function ?(recarg = Rejected) env sexp ty_expected when a signature exposes a private or abstract alias), also mark usage on the exposed target declaration so scheduled warnings attached to that declaration are cleared. *) - if not (Path.same p_tgt p_tgt_conc) then ( + if not (Path.same p_tgt p_tgt_conc) then let exposed_ty_name = Path.last p_tgt in let exposed_decl = Env.find_type p_tgt env in List.iter @@ -3060,7 +3060,7 @@ and type_expect_ ~context ?in_function ?(recarg = Rejected) env sexp ty_expected if has_src cname then Env.mark_constructor_used Env.Positive env exposed_ty_name exposed_decl cname) - tgt_cons) + tgt_cons | _ -> ()) | _ -> ()); rue diff --git a/tests/build_tests/super_errors/expected/VariantCoercionWarnOnC.res.expected b/tests/build_tests/super_errors/expected/VariantCoercionWarnOnC.res.expected new file mode 100644 index 0000000000..433d638209 --- /dev/null +++ b/tests/build_tests/super_errors/expected/VariantCoercionWarnOnC.res.expected @@ -0,0 +1,13 @@ + + Warning number 37 + /.../fixtures/VariantCoercionWarnOnC.res:9:3-11:7 + + 7 │ let log: a => unit + 8 │ } = { + 9 │ type b = + 10 │  | ...a + 11 │  | C + 12 │ + 13 │ let log = (x: a) => Js.log((x :> b)) + + unused constructor C. diff --git a/tests/build_tests/super_errors/fixtures/VariantCoercionWarnOnC.res b/tests/build_tests/super_errors/fixtures/VariantCoercionWarnOnC.res new file mode 100644 index 0000000000..78eafc9a98 --- /dev/null +++ b/tests/build_tests/super_errors/fixtures/VariantCoercionWarnOnC.res @@ -0,0 +1,16 @@ +// Ensure unused-constructor warning still fires for new members introduced +// only on the target side of a coercion. + +type a = A | B + +module M: { + let log: a => unit +} = { + type b = + | ...a + | C + + let log = (x: a) => Js.log((x :> b)) +} + +let _ = M.log(A) diff --git a/tests/tests/src/VariantCoercionConstructUsed.mjs b/tests/tests/src/VariantCoercionConstructUsed.mjs index 3cf18ee208..b2a95aab6e 100644 --- a/tests/tests/src/VariantCoercionConstructUsed.mjs +++ b/tests/tests/src/VariantCoercionConstructUsed.mjs @@ -1,11 +1,17 @@ // Generated by ReScript, PLEASE EDIT WITH CARE -function upcast(x) { - return x; +function log(x) { + console.log(x); } +let DoNotWarn = { + log: log +}; + +console.log("A"); + export { - upcast, + DoNotWarn, } -/* No side effect */ +/* Not a pure module */ From 6012c8680459c4d4de6932d840a0aac72d8c800d Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 4 Sep 2025 13:44:22 +0200 Subject: [PATCH 4/8] changelog: link variant coercion unused-constructor fix to #7839 --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66de9f1385..52fdaed3f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ - Fix generation of interfaces for module types containing multiple type constraints. https://github.com/rescript-lang/rescript/pull/7825 - JSX preserve mode: fix "make is not a valid component name". https://github.com/rescript-lang/rescript/pull/7831 - Rewatch: include parser arguments of experimental features. https://github.com/rescript-lang/rescript/pull/7836 +- Suppress spurious “constructor … is never used” warnings when constructors are introduced via variant coercions; still warn for constructors only present on the target side. https://github.com/rescript-lang/rescript/pull/7839 #### :memo: Documentation From 5d5e111efdd9ebc36cc0ac1a301100c73228f1b5 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Fri, 5 Sep 2025 12:00:26 +0200 Subject: [PATCH 5/8] typecore: avoid option tuple match; wrap both extractions in single try/with Mark constructor usage only when both concrete typedecls are found. Preserves behavior and simplifies the control flow vs tuple-of-options match. Signed-off-by: Cristiano Calcagno --- compiler/ml/typecore.ml | 81 ++++++++++++++++++++--------------------- 1 file changed, 40 insertions(+), 41 deletions(-) diff --git a/compiler/ml/typecore.ml b/compiler/ml/typecore.ml index 490ebfd425..046b99f8c5 100644 --- a/compiler/ml/typecore.ml +++ b/compiler/ml/typecore.ml @@ -3022,47 +3022,46 @@ and type_expect_ ~context ?in_function ?(recarg = Rejected) env sexp ty_expected types, mark the intersection of constructors as positively used for the target type to avoid spurious "constructor ... is never used" warnings when values are introduced via coercions. *) - (match - ( (try Some (Ctype.extract_concrete_typedecl env arg.exp_type) - with Not_found -> None), - try Some (Ctype.extract_concrete_typedecl env ty') - with Not_found -> None ) - with - | Some (_p_src, _p_src_conc, src_decl), Some (p_tgt, p_tgt_conc, tgt_decl) - -> ( - match (src_decl.type_kind, tgt_decl.type_kind) with - | Type_variant src_cons, Type_variant tgt_cons -> - let src_names = - List.map - (fun (c : Types.constructor_declaration) -> Ident.name c.cd_id) - src_cons - in - let has_src name = List.exists (fun n -> n = name) src_names in - let tgt_ty_name_conc = Path.last p_tgt_conc in - (* Mark usage for the concrete target decl (implementation view). *) - List.iter - (fun (c : Types.constructor_declaration) -> - let cname = Ident.name c.cd_id in - if has_src cname then - Env.mark_constructor_used Env.Positive env tgt_ty_name_conc - tgt_decl cname) - tgt_cons; - (* If the target type path differs from its concrete decl (e.g., - when a signature exposes a private or abstract alias), also mark - usage on the exposed target declaration so scheduled warnings - attached to that declaration are cleared. *) - if not (Path.same p_tgt p_tgt_conc) then - let exposed_ty_name = Path.last p_tgt in - let exposed_decl = Env.find_type p_tgt env in - List.iter - (fun (c : Types.constructor_declaration) -> - let cname = Ident.name c.cd_id in - if has_src cname then - Env.mark_constructor_used Env.Positive env exposed_ty_name - exposed_decl cname) - tgt_cons - | _ -> ()) - | _ -> ()); + (try + let _p_src, _p_src_conc, src_decl = + Ctype.extract_concrete_typedecl env arg.exp_type + in + let p_tgt, p_tgt_conc, tgt_decl = + Ctype.extract_concrete_typedecl env ty' + in + match (src_decl.type_kind, tgt_decl.type_kind) with + | Type_variant src_cons, Type_variant tgt_cons -> + let src_names = + List.map + (fun (c : Types.constructor_declaration) -> Ident.name c.cd_id) + src_cons + in + let has_src name = List.exists (fun n -> n = name) src_names in + let tgt_ty_name_conc = Path.last p_tgt_conc in + (* Mark usage for the concrete target decl (implementation view). *) + List.iter + (fun (c : Types.constructor_declaration) -> + let cname = Ident.name c.cd_id in + if has_src cname then + Env.mark_constructor_used Env.Positive env tgt_ty_name_conc + tgt_decl cname) + tgt_cons; + (* If the target type path differs from its concrete decl (e.g., + when a signature exposes a private or abstract alias), also mark + usage on the exposed target declaration so scheduled warnings + attached to that declaration are cleared. *) + if not (Path.same p_tgt p_tgt_conc) then + let exposed_ty_name = Path.last p_tgt in + let exposed_decl = Env.find_type p_tgt env in + List.iter + (fun (c : Types.constructor_declaration) -> + let cname = Ident.name c.cd_id in + if has_src cname then + Env.mark_constructor_used Env.Positive env exposed_ty_name + exposed_decl cname) + tgt_cons + | _ -> () + with Not_found -> ()); rue { exp_desc = arg.exp_desc; From bfcef5558e32c62a43e991f32e0b3005c081566e Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Fri, 5 Sep 2025 12:03:19 +0200 Subject: [PATCH 6/8] typecore: speed up constructor membership with StringSet Build a set of source constructor names and use O(log n) membership checks instead of O(n) List.exists for each target constructor. Signed-off-by: Cristiano Calcagno --- compiler/ml/typecore.ml | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/compiler/ml/typecore.ml b/compiler/ml/typecore.ml index 046b99f8c5..10e87a2e3a 100644 --- a/compiler/ml/typecore.ml +++ b/compiler/ml/typecore.ml @@ -3031,12 +3031,14 @@ and type_expect_ ~context ?in_function ?(recarg = Rejected) env sexp ty_expected in match (src_decl.type_kind, tgt_decl.type_kind) with | Type_variant src_cons, Type_variant tgt_cons -> - let src_names = - List.map - (fun (c : Types.constructor_declaration) -> Ident.name c.cd_id) - src_cons + let module StringSet = Set.Make (String) in + let src_set = + List.fold_left + (fun acc (c : Types.constructor_declaration) -> + StringSet.add (Ident.name c.cd_id) acc) + StringSet.empty src_cons in - let has_src name = List.exists (fun n -> n = name) src_names in + let has_src name = StringSet.mem name src_set in let tgt_ty_name_conc = Path.last p_tgt_conc in (* Mark usage for the concrete target decl (implementation view). *) List.iter From 2c6a0e93f7971535811794b6489d323c1d22d942 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Sat, 6 Sep 2025 05:11:29 +0200 Subject: [PATCH 7/8] chore: remove example.res and issue.txt Signed-off-by: Cristiano Calcagno --- example.res | 10 ---------- issue.txt | 46 ---------------------------------------------- 2 files changed, 56 deletions(-) delete mode 100644 example.res delete mode 100644 issue.txt diff --git a/example.res b/example.res deleted file mode 100644 index ce214600f4..0000000000 --- a/example.res +++ /dev/null @@ -1,10 +0,0 @@ -type a = A | B - -// b spreads a and adds one more constructor -type b = - | ...a - | C - -let upcast = (x: a): b => (x :> b) - -let _ = upcast(A) diff --git a/issue.txt b/issue.txt deleted file mode 100644 index 5a064b0c48..0000000000 --- a/issue.txt +++ /dev/null @@ -1,46 +0,0 @@ -found a compiler bug: https://github.com/rescript-lang/rescript/issues/7838 (due to the coercing between variants) -GitHub -Incorrect compiler warning (constructor is never used to build valu... -https://rescript-lang.org/try?version=v11.1.4&module=esmodule&code=LYewJgrgNgpgBAUQG4wHYBc4F44G8BQcc6AngA7yY4A+iKGAgvoXLJgBYCGqYs2cAChj10ALmIBKbAD48LIgGcA7gEt0AY3ZxhaTASJFayXQxmCJ8uAF8WNm-l... -Incorrect compiler warning (constructor is never used to build valu... -[09:34]Gabriel Nordeborn: What is wrong here...? -[09:35]jaap: The warning? -[09:36]jaap: The variants are constructed in handle -[09:37]jaap: I mean, the coercion constructs the variants -[09:38]jaap: so if we have variantA :> variantB, we can mark that variantB is always constructed -[09:38]Gabriel Nordeborn: Right, so if you adapt the example to not use coercion, it marks them as constructed? -[09:39]Gabriel Nordeborn: It's good if you write this in the PR issue -[09:39]Gabriel Nordeborn: Right now, just looking at the example, I did not understand what you're referring to -[09:41]jaap: Yes -[09:41]jaap: https://rescript-lang.org/try?version=v11.1.4&module=esmodule&code=LYewJgrgNgpgBAUQG4wHYBc4F44G8BQcc6AngA7yY4A+iKGAgvoXLJgBYCGqYs2cAChj10ALmIBKbAD48LIgGcA7gEt0AY3ZxhaTASJFayXQxmCJ8uAF8WNm-lCQ+AZRgAnFG+MYA4iHDi+qwwmGgAjhAwkeLe6AB0VLIQqGr4VvxBpBTE2Ja0cQWxCcxEbHBcPHw4nAokqOqCOhji6FJYskGKqhpaTXp5dCZmAhYG1rYsZeGRkfxCIjEiCW2yFbwwAspqmtoicoaDjGaxTERWEgC00ioA5qggbjBpzI7Q8K4e7rEAQpxggZMQtpUBEojBFroEmZkql0jhMuRKLkxvlCkt0CVghxuOt+DU6g15roWit9gYtj1droyQcTsNRgYbGdAaEQTN4Dgic1DvFWmY1rAuZhRLJWldbvdHs8gA -ReScript Documentation -ReScript Playground -Try ReScript in the browser -ReScript Playground -[09:41]jaap: here with two examples -[09:41]jaap: also updated the ticket -[09:44]cristianoc: The type system was created with no coercion in mind. -[09:45]cristianoc: So fix warning —-> compiler error? -[09:47]Gabriel Nordeborn: @jaap is this not a better example? https://rescript-lang.org/try?version=v11.1.4&module=esmodule&code=LYewJgrgNgpgBAUQG4wHYBc4F44G8BQcc6AngA7yY7JroCC+hcsmAFgIapizZwAUMFBgBcxAJTYAfHiZEAzgHcAlugDGrOINoyiRAD6Ih9KfzGy4AXyZWr+UJB4BlGACcULmhgDiIcKIJEpBTEvAYAdBGe6GHojEQsmqgAjhAwqaJU0hCoKvgWvAHE5JShcBFhUTFxzDBsnNzwOOxyJKiq-Foi4iaF8spqGp2YvXAGUXQmfGa6ltZMCWgpaY0dRhkSWNIcXLACRmIAtJJKAOaoIC4weYz20PDObq5RAELsYP7ztYlL6Ya0MSZsrl8jhCkESjhwpEjFVPnUditmq12ntaOseuZFCp1JojDpdGMjBNNqZzFYiOSaphFqlUrxUV1KugNlt6rsGZhhNJmYdjmcLlcrEA -ReScript Documentation -ReScript Playground -Try ReScript in the browser -ReScript Playground -[09:47]Gabriel Nordeborn: Using the switch fixes the warning of course because you're now constructing it explicitly -[09:47]Gabriel Nordeborn: If you were to add the switch instead of coercion that'd work as well -[09:48]Gabriel Nordeborn: But from what I understand the problem is that there's a value of a type that gets coerced to another type, and that other type is not marked as used -[09:49]Gabriel Nordeborn: The value that's not coerced is marked, but the new type from the coercion is not marked -[10:35]jaap: @cristianoc there is no way to fix this warning - you need to convert it to a switch expression, but that is a lot of redudant code potentially -[10:36]cristianoc: That’s actually better. So the message is at least not inconsistent. -[10:36]jaap: but the tricky part is that when coercing, the members that are in variantA need to marked as used, but not the new ones -[10:36]cristianoc: I was worried: you remove the case, then get a compile error -[10:37]jaap: Yes you do get an error - so it's not swallowing that -[10:37]jaap: if you include it (warning) if you remove it (error) -[10:38]cristianoc: There’s no such thing in the compiler. As ocaml does not have that. So it will be a genuine extension to handle this. -And I don’t even know if possible. -Is this global property? If so the. Compiler can’t do it. -[10:38]jaap: but yeah seems to be a gap in the type system after adding the nice spreading of variants and coercing -[10:39]jaap: No it's local - in the example you see the module signature added, so it only happens if you make the type private, and then coerce it -[10:40]jaap: I guess there is some mechanism to mark members as constructed - and coercion is not doing that -[10:40]cristianoc: Ok this is enough discussion. -Paste it in a prompt and see if it gets close. From 7da7c056cc92ecfa2b11e189283a7720d56a6ef7 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Sat, 6 Sep 2025 05:16:10 +0200 Subject: [PATCH 8/8] =?UTF-8?q?typecore:=20avoid=20false=20'constructor=20?= =?UTF-8?q?=E2=80=A6=20is=20never=20used'=20after=20variant=20coercion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mark shared constructors as used on the target variant after successful coercion; keep warnings for target-only constructors. Part of #7839. --- compiler/ml/typecore.ml | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/compiler/ml/typecore.ml b/compiler/ml/typecore.ml index 10e87a2e3a..87d28a77b4 100644 --- a/compiler/ml/typecore.ml +++ b/compiler/ml/typecore.ml @@ -3026,9 +3026,7 @@ and type_expect_ ~context ?in_function ?(recarg = Rejected) env sexp ty_expected let _p_src, _p_src_conc, src_decl = Ctype.extract_concrete_typedecl env arg.exp_type in - let p_tgt, p_tgt_conc, tgt_decl = - Ctype.extract_concrete_typedecl env ty' - in + let _, p_tgt_conc, tgt_decl = Ctype.extract_concrete_typedecl env ty' in match (src_decl.type_kind, tgt_decl.type_kind) with | Type_variant src_cons, Type_variant tgt_cons -> let module StringSet = Set.Make (String) in @@ -3047,21 +3045,7 @@ and type_expect_ ~context ?in_function ?(recarg = Rejected) env sexp ty_expected if has_src cname then Env.mark_constructor_used Env.Positive env tgt_ty_name_conc tgt_decl cname) - tgt_cons; - (* If the target type path differs from its concrete decl (e.g., - when a signature exposes a private or abstract alias), also mark - usage on the exposed target declaration so scheduled warnings - attached to that declaration are cleared. *) - if not (Path.same p_tgt p_tgt_conc) then - let exposed_ty_name = Path.last p_tgt in - let exposed_decl = Env.find_type p_tgt env in - List.iter - (fun (c : Types.constructor_declaration) -> - let cname = Ident.name c.cd_id in - if has_src cname then - Env.mark_constructor_used Env.Positive env exposed_ty_name - exposed_decl cname) - tgt_cons + tgt_cons | _ -> () with Not_found -> ()); rue