Fix issue caused by effects of gadt expansion in mode cross check #1263

ccasin · 2023-03-27T15:57:58Z

We previously observed the issue that because the improved immediacy system means the mode crossing check may use gadt equations, those equations may appear to escape their scope in benign situations. I've brought over the fix we used in the unboxed types branch (snapshotting to forget the scope changes), but moved it from Typecore.mode_cross to Ctype.is_always_global (to avoid duplication as the latter function now has two callers).

Two things to consider:

We're sure it's fine to ignore the scope changes resulting from use of equations during the mode cross check?
Is a similar change needed to Typeopt.is_always_gc_ignorable (I think no, because it's too late for the effects to matter).

lpw25

I think it's fine to ignore scope changes from checking immediacy: relying on type a = int to mode cross an a doesn't need to make it ambiguous whether that a should be an a or an int.

I agree that you don't need to worry about this stuff in the Typeopt version.

ocaml/typing/ctype.ml

1686928 flambda-backend: Restore Cmm unboxing behaviour inside regions (ocaml-flambda#1285) cf9be42 flambda-backend: Fix all the no-naked-pointers problems (ocaml-flambda#1282) 8fe089e flambda-backend: Unrevert ocaml-flambda#1131 and fix a Cmm unboxing bug (ocaml-flambda#1284) c4143c3 flambda-backend: Revert "Make Selectgen treat region boundaries more precisely" (ocaml-flambda#1283) 2078dce flambda-backend: Add some -dtimings output for the typechecker (ocaml-flambda#1245) 273a7f9 flambda-backend: Make Selectgen treat region boundaries more precisely (ocaml-flambda#1131) 47610e6 flambda-backend: Bump magic numbers for 4.14.1-6 (ocaml-flambda#1274) fd53d38 flambda-backend: Generate *.cms files for merlin (ocaml-flambda#1232) 853f95f flambda-backend: Add tail_mod_const to builtin_attrs (ocaml-flambda#1265) f9ef051 flambda-backend: Fix issue caused by effects of gadt expansion in mode cross check (ocaml-flambda#1263) e9ffcf8 flambda-backend: Fix dependencies for regenerating Flambda2 parser, tests (ocaml-flambda#1255) 6f1cd1f flambda-backend: Restore a lost location, needed for merlin (ocaml-flambda#1242) 009332b flambda-backend: Fix merge from ocaml-jst git-subtree-dir: ocaml git-subtree-split: 1686928

a7d005a flambda-backend: Lazy deserialization of cmi files (ocaml-flambda#1322) aa83fa3 flambda-backend: Reinstate previous API for Env.lookup_value (ocaml-flambda#1323) e4007a4 flambda-backend: Lazy substitution into value_declaration (ocaml-flambda#1320) 634b607 flambda-backend: Merge Types.* and Subst.Lazy.* types (ocaml-flambda#1312) cf82708 flambda-backend: Bump magic numbers for 4.14.1-7 (ocaml-flambda#1317) 6470400 flambda-backend: zero_alloc attribute payload "assert all" and "ignore" (ocaml-flambda#1296) bba5248 flambda-backend: Teach `ocamldep` about all the language extensions (ocaml-flambda#1303) 33e97b0 flambda-backend: Change Includemod to work on lazy modtypes (ocaml-flambda#1228) 16e5002 flambda-backend: zero_alloc new warning for unchecked functions (ocaml-flambda#1302) 36b4626 flambda-backend: Attribute [@@@zero_alloc check] to turn the check on (ocaml-flambda#1294) 3b524c6 flambda-backend: Cmm.value_kind cleanup (ocaml-flambda#1091) ec99505 flambda-backend: Fix failure of `check_all_arches` on RISCV (ocaml-flambda#1300) 450bc58 flambda-backend: Backend changes for multiple returns (ocaml-flambda#1268) 84a7a26 flambda-backend: Static check for zero_alloc: ignore allocation that lead to exceptional return (ocaml-flambda#1157) 1723728 flambda-backend: Re-enable parallel build of the runtime (ocaml-flambda#1287) 26ea7f3 flambda-backend: Fix closure marshalling when not in NNP mode (ocaml-flambda#1286) 9b91f2e flambda-backend: Reduce number of caml_apply functions taking/returning "I" and "V" (ocaml-flambda#1272) 1686928 flambda-backend: Restore Cmm unboxing behaviour inside regions (ocaml-flambda#1285) cf9be42 flambda-backend: Fix all the no-naked-pointers problems (ocaml-flambda#1282) 8fe089e flambda-backend: Unrevert ocaml-flambda#1131 and fix a Cmm unboxing bug (ocaml-flambda#1284) c4143c3 flambda-backend: Revert "Make Selectgen treat region boundaries more precisely" (ocaml-flambda#1283) 2078dce flambda-backend: Add some -dtimings output for the typechecker (ocaml-flambda#1245) 273a7f9 flambda-backend: Make Selectgen treat region boundaries more precisely (ocaml-flambda#1131) 47610e6 flambda-backend: Bump magic numbers for 4.14.1-6 (ocaml-flambda#1274) fd53d38 flambda-backend: Generate *.cms files for merlin (ocaml-flambda#1232) 853f95f flambda-backend: Add tail_mod_const to builtin_attrs (ocaml-flambda#1265) f9ef051 flambda-backend: Fix issue caused by effects of gadt expansion in mode cross check (ocaml-flambda#1263) e9ffcf8 flambda-backend: Fix dependencies for regenerating Flambda2 parser, tests (ocaml-flambda#1255) 6f1cd1f flambda-backend: Restore a lost location, needed for merlin (ocaml-flambda#1242) 009332b flambda-backend: Fix merge from ocaml-jst git-subtree-dir: ocaml git-subtree-split: a7d005a

Fix issue caused by effects of gadt expansion in mode cross check

2fcbc26

ccasin requested review from lpw25 and antalsz as code owners March 27, 2023 15:57

lpw25 approved these changes Mar 27, 2023

View reviewed changes

ocaml/typing/ctype.ml Outdated Show resolved Hide resolved

Skip backtracking if unnecessary

7b0a09e

ccasin merged commit aabb2bd into ocaml-flambda:main Mar 27, 2023

ccasin deleted the snapshot-modecross branch March 27, 2023 17:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix issue caused by effects of gadt expansion in mode cross check #1263

Fix issue caused by effects of gadt expansion in mode cross check #1263

ccasin commented Mar 27, 2023

lpw25 left a comment

Fix issue caused by effects of gadt expansion in mode cross check #1263

Fix issue caused by effects of gadt expansion in mode cross check #1263

Conversation

ccasin commented Mar 27, 2023

lpw25 left a comment

Choose a reason for hiding this comment