Skip to content

Commit 7e5a626

Browse files
authored
Improve the API of language extensions to better support upstream compatibility (and also tooling) (ocaml-flambda#13)
This doesn't change much inside the compiler, but is important for writing e.g. refactoring tools that aren't part of the compiler.
1 parent c4e17b0 commit 7e5a626

File tree

11 files changed

+501
-51
lines changed

11 files changed

+501
-51
lines changed

driver/compenv.ml

+2-2
Original file line numberDiff line numberDiff line change
@@ -489,9 +489,9 @@ let read_one_param ppf position name v =
489489
| "dump-into-file" -> Clflags.dump_into_file := true
490490
| "dump-dir" -> Clflags.dump_dir := Some v
491491

492-
| "extension" -> Clflags.Extension.enable v
492+
| "extension" -> Clflags.Extension.(enable (of_string_exn v))
493493
| "disable-all-extensions" ->
494-
if check_bool ppf name v then Clflags.Extension.disable_all ()
494+
if check_bool ppf name v then Clflags.Extension.disallow_extensions ()
495495

496496
| _ ->
497497
if !warnings_for_discarded_params &&

driver/main_args.ml

+43-7
Original file line numberDiff line numberDiff line change
@@ -731,15 +731,37 @@ let mk_extension f =
731731
Clflags.Extension.(List.map to_string all)
732732
in
733733
"-extension", Arg.Symbol (available_extensions, f),
734-
"<extension> Enable the extension (may be specified more than once)"
734+
" Enable the specified extension (may be specified more than once)"
735+
;;
736+
737+
let mk_no_extension f =
738+
let available_extensions =
739+
Clflags.Extension.(List.map to_string all)
740+
in
741+
"-no-extension", Arg.Symbol (available_extensions, f),
742+
" Disable the specified extension (may be specified more than once)"
735743
;;
736744

737745
let mk_disable_all_extensions f =
738746
"-disable-all-extensions", Arg.Unit f,
739-
" Disable all extensions, wherever they are specified; this flag\n\
740-
\ overrides the -extension flag (whether specified before or after this\n\
741-
\ flag), disables any extensions that are enabled by default, and\n\
742-
\ ignores any extensions requested in OCAMLPARAM."
747+
" Disable all extensions, wherever they have been specified; this\n\
748+
\ flag overrides prior uses of the -extension flag, disables any\n\
749+
\ extensions that are enabled by default, and causes future uses of\n\
750+
\ the -extension flag to raise an error."
751+
;;
752+
753+
let mk_only_erasable_extensions f =
754+
let erasable_extensions =
755+
let open Clflags.Extension in
756+
all |> List.filter is_erasable |> List.map to_string |> String.concat ", "
757+
in
758+
"-only-erasable-extensions", Arg.Unit f,
759+
" Disable all extensions that cannot be \"erased\" to attributes,\n\
760+
\ wherever they have been specified; this flag overrides prior\n\
761+
\ contradictory uses of the -extension flag, raises an error on\n\
762+
\ future such uses, and disables any such extensions that are\n\
763+
\ enabled by default.\n\
764+
\ (Erasable extensions: " ^ erasable_extensions ^ ")"
743765
;;
744766

745767
let mk_dump_dir f =
@@ -958,7 +980,9 @@ module type Common_options = sig
958980
val _app_funct : unit -> unit
959981
val _no_app_funct : unit -> unit
960982
val _disable_all_extensions : unit -> unit
983+
val _only_erasable_extensions : unit -> unit
961984
val _extension : string -> unit
985+
val _no_extension : string -> unit
962986
val _noassert : unit -> unit
963987
val _nolabels : unit -> unit
964988
val _nostdlib : unit -> unit
@@ -1220,10 +1244,12 @@ struct
12201244
mk_config_var F._config_var;
12211245
mk_custom F._custom;
12221246
mk_disable_all_extensions F._disable_all_extensions;
1247+
mk_only_erasable_extensions F._only_erasable_extensions;
12231248
mk_dllib F._dllib;
12241249
mk_dllpath F._dllpath;
12251250
mk_dtypes F._annot;
12261251
mk_extension F._extension;
1252+
mk_no_extension F._no_extension;
12271253
mk_for_pack_byt F._for_pack;
12281254
mk_g_byt F._g;
12291255
mk_stop_after ~native:false F._stop_after;
@@ -1332,7 +1358,9 @@ struct
13321358
mk_app_funct F._app_funct;
13331359
mk_no_app_funct F._no_app_funct;
13341360
mk_disable_all_extensions F._disable_all_extensions;
1361+
mk_only_erasable_extensions F._only_erasable_extensions;
13351362
mk_extension F._extension;
1363+
mk_no_extension F._no_extension;
13361364
mk_noassert F._noassert;
13371365
mk_noinit F._noinit;
13381366
mk_nolabels F._nolabels;
@@ -1410,7 +1438,9 @@ struct
14101438
mk_config_var F._config_var;
14111439
mk_dtypes F._annot;
14121440
mk_disable_all_extensions F._disable_all_extensions;
1441+
mk_only_erasable_extensions F._only_erasable_extensions;
14131442
mk_extension F._extension;
1443+
mk_no_extension F._no_extension;
14141444
mk_for_pack_opt F._for_pack;
14151445
mk_g_opt F._g;
14161446
mk_function_sections F._function_sections;
@@ -1579,7 +1609,9 @@ module Make_opttop_options (F : Opttop_options) = struct
15791609
mk_app_funct F._app_funct;
15801610
mk_no_app_funct F._no_app_funct;
15811611
mk_disable_all_extensions F._disable_all_extensions;
1612+
mk_only_erasable_extensions F._only_erasable_extensions;
15821613
mk_extension F._extension;
1614+
mk_no_extension F._no_extension;
15831615
mk_no_float_const_prop F._no_float_const_prop;
15841616
mk_noassert F._noassert;
15851617
mk_noinit F._noinit;
@@ -1673,7 +1705,9 @@ struct
16731705
mk_app_funct F._app_funct;
16741706
mk_no_app_funct F._no_app_funct;
16751707
mk_disable_all_extensions F._disable_all_extensions;
1708+
mk_only_erasable_extensions F._only_erasable_extensions;
16761709
mk_extension F._extension;
1710+
mk_no_extension F._no_extension;
16771711
mk_noassert F._noassert;
16781712
mk_nolabels F._nolabels;
16791713
mk_nostdlib F._nostdlib;
@@ -1768,8 +1802,10 @@ module Default = struct
17681802
let _no_strict_formats = clear strict_formats
17691803
let _no_strict_sequence = clear strict_sequence
17701804
let _no_unboxed_types = clear unboxed_types
1771-
let _disable_all_extensions = Extension.disable_all
1772-
let _extension s = Extension.enable s
1805+
let _disable_all_extensions = Extension.disallow_extensions
1806+
let _only_erasable_extensions = Extension.restrict_to_erasable_extensions
1807+
let _extension s = Extension.(enable (of_string_exn s))
1808+
let _no_extension s = Extension.(disable (of_string_exn s))
17731809
let _noassert = set noassert
17741810
let _nolabels = set classic
17751811
let _nostdlib = set no_std_include

driver/main_args.mli

+2
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ module type Common_options = sig
2626
val _app_funct : unit -> unit
2727
val _no_app_funct : unit -> unit
2828
val _disable_all_extensions : unit -> unit
29+
val _only_erasable_extensions : unit -> unit
2930
val _extension : string -> unit
31+
val _no_extension : string -> unit
3032
val _noassert : unit -> unit
3133
val _nolabels : unit -> unit
3234
val _nostdlib : unit -> unit

manual/src/cmds/unified-options.etex

+21
Original file line numberDiff line numberDiff line change
@@ -851,5 +851,26 @@ hyphen (-).}
851851
\item["-help" or "--help"]
852852
Display a short usage summary and exit.
853853

854+
\item[(JST) "-extension" \var{language-extension}]
855+
Enable the specified \var{language-extension}. Can be specified more than once,
856+
either with the same or a different language extension; is idempotent.
857+
858+
\item[(JST) "-no-extension" \var{language-extension}]
859+
Disable the specified \var{language-extension}. Can be specified more than once,
860+
either with the same or a different language extension; is idempotent.
861+
862+
\item[(JST) "-only-erasable-extensions"]
863+
Restricts the "-extension" option to work only with so-called ``erasable''
864+
extensions: ones that can be rewritten into attributes while still preserving
865+
the program's runtime input/output behavior. Turns off currently-enabled
866+
non-erasable extensions when specified. After this flag, specifying a
867+
non-erasable extension (even to disable it) will fail with an error. This flag
868+
cannot be reversed, but it can be strengthened (by "-disable-all-extensions").
869+
870+
\item[(JST) "-disable-all-extensions"]
871+
Disallow all language extensions moving forward, and turn off currently-enabled
872+
ones. This makes "-extension" raise errors moving forwards. This flag cannot
873+
be reversed.
874+
854875
\end{options}
855876
%

testsuite/tests/ast-invariants/test.ml

+1-1
Original file line numberDiff line numberDiff line change
@@ -85,5 +85,5 @@ let rec walk dir =
8585
(Sys.readdir dir)
8686

8787
let () =
88-
List.iter Clflags.Extension.enable_t Clflags.Extension.all;
88+
List.iter Clflags.Extension.enable Clflags.Extension.all;
8989
walk root

testsuite/tests/comprehensions/syntax.ml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
(* TEST
22
include ocamlcommon *)
33

4-
let () = Clflags.Extension.enable "comprehensions_experimental";;
4+
let () = Clflags.Extension.enable Comprehensions;;
55

66
let printf = Printf.printf;;
77

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
(* TEST
2+
include ocamlcommon
3+
flags = "-I ${ocamlsrcdir}/parsing"
4+
reference = "${test_source_directory}/reference.txt"
5+
*)
6+
7+
(* Change these two variables to change which extension is being tested *)
8+
let extension = Clflags.Extension.Comprehensions
9+
let extension_expression = "[x for x = 1 to 10]"
10+
11+
let extension_name = Clflags.Extension.to_string extension
12+
let extension_parsed_expression =
13+
Parse.expression (Lexing.from_string extension_expression)
14+
(* Currently, parsing always succeeds and we only fail during typechecking *)
15+
16+
let report ~name ~text =
17+
Printf.printf "# %s [%s %s]:\n%s\n\n"
18+
name
19+
extension_name
20+
(if Clflags.Extension.is_enabled extension then "enabled" else "disabled")
21+
text
22+
23+
let typecheck_with_extension ?(full_name = false) name =
24+
let success =
25+
match Typecore.type_expression Env.empty extension_parsed_expression with
26+
| _ -> true
27+
| exception (Extensions_parsing.Error.Error _) -> false
28+
in
29+
report
30+
~name:(if full_name
31+
then name
32+
else "\"" ^ extension_name ^ "\" extension " ^ name)
33+
~text:(if success
34+
then "Successfully typechecked \"" ^ extension_expression ^ "\""
35+
else "<extension error>")
36+
;;
37+
38+
let should_succeed name what f =
39+
report ~name ~text:(match f () with
40+
| () ->
41+
"Succeeded at " ^ what
42+
| exception Arg.Bad msg ->
43+
"FAILED at " ^ what ^ ", with the following error\n:" ^ msg)
44+
;;
45+
46+
let should_fail name f =
47+
report ~name ~text:(match f () with
48+
| () -> "<succeeded INCORRECTLY>"
49+
| exception Arg.Bad msg -> "Failed as expected: " ^ msg)
50+
;;
51+
52+
let try_disallowing_extensions name =
53+
should_succeed
54+
name
55+
"disallowing all extensions"
56+
Clflags.Extension.disallow_extensions
57+
;;
58+
59+
type goal = Fail | Succeed
60+
61+
let when_disallowed goal f_str f =
62+
let can_or_can't = match goal with
63+
| Fail -> "can't"
64+
| Succeed -> "can"
65+
in
66+
let f_code = "[" ^ f_str ^ "]" in
67+
let name =
68+
can_or_can't ^ " call " ^ f_code ^ " when extensions are disallowed"
69+
in
70+
let action () = f extension in
71+
match goal with
72+
| Fail -> should_fail name action
73+
| Succeed -> should_succeed name ("redundantly calling " ^ f_code) action
74+
;;
75+
76+
let lift_with with_fn extension = with_fn extension Fun.id;;
77+
78+
(* Test the ground state *)
79+
80+
typecheck_with_extension "in its default state";
81+
82+
(* Disable all extensions for testing *)
83+
84+
List.iter Clflags.Extension.disable Clflags.Extension.all;
85+
typecheck_with_extension ~full_name:true "no extensions enabled";
86+
87+
(* Test globally toggling a language extension *)
88+
89+
Clflags.Extension.enable extension;
90+
typecheck_with_extension "enabled";
91+
92+
Clflags.Extension.enable extension;
93+
typecheck_with_extension "still enabled";
94+
95+
Clflags.Extension.disable extension;
96+
typecheck_with_extension "disabled";
97+
98+
Clflags.Extension.disable extension;
99+
typecheck_with_extension "still disabled";
100+
101+
Clflags.Extension.set extension ~enabled:true;
102+
typecheck_with_extension "enabled via [set]";
103+
104+
Clflags.Extension.enable extension;
105+
typecheck_with_extension "still enabled, via [set] and [enable]";
106+
107+
Clflags.Extension.set extension ~enabled:false;
108+
typecheck_with_extension "disabled via [set]";
109+
110+
Clflags.Extension.disable extension;
111+
typecheck_with_extension "still disabled, via [set] and [disable]";
112+
113+
(* Test locally toggling a language extension *)
114+
115+
(* Globally disable the language extension (idempotent, given the prior tests,
116+
but it's more robust to do this explicitly) *)
117+
Clflags.Extension.disable extension;
118+
119+
Clflags.Extension.with_enabled extension (fun () ->
120+
typecheck_with_extension "enabled locally and disabled globally");
121+
122+
Clflags.Extension.with_disabled extension (fun () ->
123+
typecheck_with_extension "disabled locally and globally");
124+
125+
Clflags.Extension.with_set extension ~enabled:true (fun () ->
126+
typecheck_with_extension
127+
"enabled locally via [with_set] and disabled globally");
128+
129+
Clflags.Extension.with_set extension ~enabled:false (fun () ->
130+
typecheck_with_extension "disabled locally via [with_set] and also globally");
131+
132+
(* Globally enable the language extension *)
133+
Clflags.Extension.enable extension;
134+
135+
Clflags.Extension.with_disabled extension (fun () ->
136+
typecheck_with_extension "disabled locally and enabled globally");
137+
138+
Clflags.Extension.with_enabled extension (fun () ->
139+
typecheck_with_extension "enabled locally and globally");
140+
141+
Clflags.Extension.with_set extension ~enabled:false (fun () ->
142+
typecheck_with_extension
143+
"disabled locally via [with_set] and enabled globally");
144+
145+
Clflags.Extension.with_set extension ~enabled:true (fun () ->
146+
typecheck_with_extension "disabled locally via [with_set] and also globally");
147+
148+
(* Test disallowing extensions *)
149+
150+
try_disallowing_extensions
151+
"can disallow extensions while extensions are enabled";
152+
153+
try_disallowing_extensions
154+
"can disallow extensions while extensions are already disallowed";
155+
156+
(* Test that disallowing extensions prevents other functions from working *)
157+
158+
when_disallowed Fail "set ~enabled:true"
159+
(Clflags.Extension.set ~enabled:true);
160+
161+
when_disallowed Succeed "set ~enabled:false"
162+
(Clflags.Extension.set ~enabled:false);
163+
164+
when_disallowed Fail "enable"
165+
Clflags.Extension.enable;
166+
167+
when_disallowed Succeed "disable"
168+
Clflags.Extension.disable;
169+
170+
when_disallowed Fail "with_set ~enabled:true"
171+
(Clflags.Extension.with_set ~enabled:true |> lift_with);
172+
173+
when_disallowed Succeed "with_set ~enabled:false"
174+
(Clflags.Extension.with_set ~enabled:false |> lift_with);
175+
176+
when_disallowed Fail "with_enabled"
177+
(Clflags.Extension.with_enabled |> lift_with);
178+
179+
when_disallowed Succeed "with_disabled"
180+
(Clflags.Extension.with_disabled |> lift_with);
181+
182+
(* Test explicitly (rather than just via [report]) that [is_enabled] returns
183+
[false] now that we've disallowed all extensions *)
184+
report
185+
~name:"[is_enabled] returns [false] when extensions are disallowed"
186+
~text:("\"" ^ extension_name ^ "\" is " ^
187+
if Clflags.Extension.is_enabled extension
188+
then "INCORRECTLY enabled"
189+
else "correctly disabled")

0 commit comments

Comments
 (0)