safe-string: clarify the relation between configure- and compile-time options

13 files changed, 70 insertions, 6 deletions

diff --git a/Changes b/Changes
index de5075815a..e7486fa32a 100644
--- a/Changes
+++ b/Changes
@@ -86,6 +86,17 @@ Release branch for 4.06:
   "S with type _ t = int".
   (Valentin Gatien-Baron, report by Jeremy Yallop)
 
+- GPR#1386: provide configure-time options to fine-tune the safe-string
+  options and default settings changed by GPR#1525.
+
+  At configure-time, -safe-string forces all module to use immutable
+  strings (this disables the per-module, compile-time -unsafe-string
+  option). The new (un)safe-string-default options let you set the
+  default choice for the per-module compile-time option. (The new
+  GPR#1252 behavior corresponds to having -safe-string-default, while
+  4.05 and older had -unsafe-string-default).
+  (Gabriel Scherer, review by Jacques-Pascal Deplaix)
+
 ### Code generation and optimizations:
 
 - MPR#5324, GPR#375: An alternative Linear Scan register allocator for
diff --git a/Makefile b/Makefile
index ea5f53b58f..f280dda930 100644
--- a/Makefile
+++ b/Makefile
@@ -344,6 +344,7 @@ utils/config.ml: utils/config.mlp config/Makefile
 	    -e 's|%%PROFINFO_WIDTH%%|$(PROFINFO_WIDTH)|' \
 	    -e 's|%%RANLIBCMD%%|$(RANLIBCMD)|' \
 	    -e 's|%%SAFE_STRING%%|$(SAFE_STRING)|' \
+	    -e 's|%%SAFE_STRING_DEFAULT%%|$(SAFE_STRING_DEFAULT)|' \
 	    -e 's|%%WINDOWS_UNICODE%%|$(WINDOWS_UNICODE)|' \
 	    -e 's|%%SYSTEM%%|$(SYSTEM)|' \
 	    -e 's|%%SYSTHREAD_SUPPORT%%|$(SYSTHREAD_SUPPORT)|' \
diff --git a/config/Makefile.mingw b/config/Makefile.mingw
index 2223bf1bc7..adb2da2f0f 100644
--- a/config/Makefile.mingw
+++ b/config/Makefile.mingw
@@ -103,6 +103,7 @@ LIBUNWIND_AVAILABLE=false
 LIBUNWIND_LINK_FLAGS=
 PROFINFO_WIDTH=26
 SAFE_STRING=false
+SAFE_STRING_DEFAULT=true
 WINDOWS_UNICODE=1
 AFL_INSTRUMENT=false
 
diff --git a/config/Makefile.mingw64 b/config/Makefile.mingw64
index ccd3405f68..2c346fbdf8 100644
--- a/config/Makefile.mingw64
+++ b/config/Makefile.mingw64
@@ -103,6 +103,7 @@ LIBUNWIND_AVAILABLE=false
 LIBUNWIND_LINK_FLAGS=
 PROFINFO_WIDTH=26
 SAFE_STRING=false
+SAFE_STRING_DEFAULT=true
 WINDOWS_UNICODE=1
 AFL_INSTRUMENT=false
 
diff --git a/config/Makefile.msvc b/config/Makefile.msvc
index 26d41f6d66..d1cee3fe8b 100644
--- a/config/Makefile.msvc
+++ b/config/Makefile.msvc
@@ -96,6 +96,7 @@ LIBUNWIND_AVAILABLE=false
 LIBUNWIND_LINK_FLAGS=
 PROFINFO_WIDTH=26
 SAFE_STRING=false
+SAFE_STRING_DEFAULT=true
 WINDOWS_UNICODE=1
 AFL_INSTRUMENT=false
 
diff --git a/config/Makefile.msvc64 b/config/Makefile.msvc64
index 1b20f10c46..eaffb31889 100644
--- a/config/Makefile.msvc64
+++ b/config/Makefile.msvc64
@@ -95,6 +95,7 @@ LIBUNWIND_AVAILABLE=false
 LIBUNWIND_LINK_FLAGS=
 PROFINFO_WIDTH=26
 SAFE_STRING=false
+SAFE_STRING_DEFAULT=true
 WINDOWS_UNICODE=1
 AFL_INSTRUMENT=false
 
diff --git a/configure b/configure
index e854b86217..bbc5b62b80 100755
--- a/configure
+++ b/configure
@@ -64,6 +64,7 @@ TOOLPREF=""
 with_cfi=true
 flambda=false
 safe_string=false
+safe_string_default=true
 afl_instrument=false
 max_testsuite_dir_retries=0
 with_cplugins=false
@@ -214,8 +215,36 @@ while : ; do
         ;; # Ignored for backward compatibility
     -fPIC|--fPIC)
         with_fpic=true;;
+    # There are two configure-time string safety option,
+    # (un)safe-string and (un)safe-string-default that interact
+    # with a compile-time (un)safe-string option.
+    #
+    # If -safe-string is set at configure time, then the compiler will
+    # always enforce that string and bytes are distinct: the
+    # compile-time -unsafe-string option is disabled. This lets us
+    # assume pervasive string immutability, for code optimizations and
+    # in the C layer.
+    #
+    # If -safe-string is not set at configure-time (if its
+    # negation -unsafe-string is used), the compiler will use the
+    # compile-time (un)safe-string setting to decide whether string
+    # and bytes are compatible on a per-module basis. The
+    # configure-time options (un)safe-string-default decide which
+    # setting will be chosen by default, if no compile-time option
+    # is explicitly passed.
+    #
+    # The configure-time behavior of OCaml 4.05 and older was
+    # equivalent to -unsafe-string -unsafe-string-default.
+    # then). OCaml 4.06 uses -unsafe-string -safe-string-default. We
+    # expect -safe-string to become the default in the future.
     -safe-string|--safe-string)
         safe_string=true;;
+    -unsafe-string|--unsafe-string)
+        safe_string=false;;
+    -safe-string-default|--safe-string-default)
+        safe_string_default=true;;
+    -unsafe-string-default|--unsafe-string-default)
+        safe_string_default=false;;
     -flat-float-array|--flat-float-array)
         flat_float_array=true;;
     -no-flat-float-array|--no-flat-float-array)
@@ -2101,6 +2130,7 @@ if [ "$ostype" = Cygwin ]; then
 fi
 config FLAMBDA "$flambda"
 config SAFE_STRING "$safe_string"
+config SAFE_STRING_DEFAULT "$safe_string_default"
 config WINDOWS_UNICODE "0"
 config AFL_INSTRUMENT "$afl_instrument"
 config MAX_TESTSUITE_DIR_RETRIES "$max_testsuite_dir_retries"
@@ -2221,10 +2251,15 @@ else
   else
   inf "        using flambda middle-end . no"
   fi
-  if test "$safe_string" = "true"; then
+  if $safe_string; then
   inf "        safe strings ............. yes"
   else
   inf "        safe strings ............. no"
+  if $safe_string_default; then
+  inf "        (-safe-string is the default per-module option)"
+  else
+  inf "        (-unsafe-string is the default per-module option)"
+  fi
   fi
   if $flat_float_array; then
   inf "        flat float arrays ........ yes"
diff --git a/driver/compmisc.ml b/driver/compmisc.ml
index bf45f6658a..a0839f34ca 100644
--- a/driver/compmisc.ml
+++ b/driver/compmisc.ml
@@ -52,7 +52,8 @@ let open_implicit_module m env =
 let initial_env () =
   Ident.reinit();
   let initial =
-    if !Clflags.unsafe_string then Env.initial_unsafe_string
+    if Config.safe_string then Env.initial_safe_string
+    else if !Clflags.unsafe_string then Env.initial_unsafe_string
     else Env.initial_safe_string
   in
   let env =
diff --git a/driver/main_args.ml b/driver/main_args.ml
index a4782ceedb..9b0c945cde 100644
--- a/driver/main_args.ml
+++ b/driver/main_args.ml
@@ -428,7 +428,8 @@ let mk_S f =
 
 let mk_safe_string f =
   "-safe-string", Arg.Unit f,
-  if Config.safe_string then " Make strings immutable (default)"
+  if Config.safe_string then " (was set when configuring the compiler)"
+  else if Config.safe_string_default then " Make strings immutable (default)"
   else " Make strings immutable"
 ;;
 
@@ -502,6 +503,8 @@ let mk_unsafe_string f =
                       -unsafe-string is not available")
     in
     "-unsafe-string", Arg.Unit err, " (option not available)"
+  else if Config.safe_string_default then
+    "-unsafe-string", Arg.Unit f, " Make strings mutable"
   else
     "-unsafe-string", Arg.Unit f, " Make strings mutable (default)"
 ;;
diff --git a/ocamldoc/odoc_analyse.ml b/ocamldoc/odoc_analyse.ml
index e6736e98df..1393d571c1 100644
--- a/ocamldoc/odoc_analyse.ml
+++ b/ocamldoc/odoc_analyse.ml
@@ -34,7 +34,8 @@ let init_path () =
 (** Return the initial environment in which compilation proceeds. *)
 let initial_env () =
   let initial =
-    if !Clflags.unsafe_string then Env.initial_unsafe_string
+    if Config.safe_string then Env.initial_safe_string
+    else if !Clflags.unsafe_string then Env.initial_unsafe_string
     else Env.initial_safe_string
   in
   let open_mod env m =
diff --git a/utils/clflags.ml b/utils/clflags.ml
index 0ca376d3bf..34f02ea2f0 100644
--- a/utils/clflags.ml
+++ b/utils/clflags.ml
@@ -158,7 +158,9 @@ let runtime_variant = ref "";;      (* -runtime-variant *)
 
 let keep_docs = ref false              (* -keep-docs *)
 let keep_locs = ref true               (* -keep-locs *)
-let unsafe_string = ref false
+let unsafe_string =
+  if Config.safe_string then ref false
+  else ref (not Config.safe_string_default)
                                    (* -safe-string / -unsafe-string *)
 
 let classic_inlining = ref false       (* -Oclassic *)
diff --git a/utils/config.mli b/utils/config.mli
index bae48e2122..4ec5f10b25 100644
--- a/utils/config.mli
+++ b/utils/config.mli
@@ -166,7 +166,11 @@ val libunwind_link_flags : string
         (* Linker flags to use libunwind *)
 
 val safe_string: bool
-        (* Whether the compiler was configured with -safe-string *)
+        (* Whether the compiler was configured with -safe-string;
+           in that case, the -unsafe-string compile-time option is unavailable *)
+val safe_string_default: bool
+        (* Whether the compiler was configured to use the -safe-string
+           or -unsafe-string compile-time option by default. *)
 val flat_float_array : bool
         (* Whether the compiler and runtime automagically flatten float
            arrays *)
diff --git a/utils/config.mlp b/utils/config.mlp
index 0e568ad2a7..77a98e1068 100644
--- a/utils/config.mlp
+++ b/utils/config.mlp
@@ -64,6 +64,7 @@ let mkdll, mkexe, mkmaindll =
 let profiling = %%PROFILING%%
 let flambda = %%FLAMBDA%%
 let safe_string = %%SAFE_STRING%%
+let safe_string_default = %%SAFE_STRING_DEFAULT%%
 let windows_unicode = %%WINDOWS_UNICODE%% != 0
 
 let flat_float_array = %%FLAT_FLOAT_ARRAY%%
@@ -175,6 +176,7 @@ let print_config oc =
   p_bool "flambda" flambda;
   p_bool "spacetime" spacetime;
   p_bool "safe_string" safe_string;
+  p_bool "safe_string_default" safe_string_default;
   p_bool "flat_float_array" flat_float_array;
   p_bool "windows_unicode" windows_unicode;