Add default cxx link flags to ocamlmklib and ocamlc calls (#5185)

* Split cxx flags by phase
* Add link flags when building libraries
* Add default cxx link flags for exe with cxx stubs
* Update tests

Signed-off-by: Ulysse Gérard <[email protected]>

src/dune_rules/cxx_flags.ml

@@ -1,16 +1,23 @@
 open! Stdune
 open Dune_engine
 
+type phase =
+  | Compile
+  | Link
+
 type ccomp_type =
   | Gcc
   | Msvc
   | Clang
   | Other of string
 
-let base_cxx_flags = function
-  | Gcc -> [ "-x"; "c++"; "-lstdc++"; "-shared-libgcc" ]
-  | Clang -> [ "-x"; "c++" ]
-  | Msvc -> [ "/TP" ]
+let base_cxx_flags ~for_ cc =
+  match (cc, for_) with
+  | Gcc, Compile -> [ "-x"; "c++" ]
+  | Gcc, Link -> [ "-lstdc++"; "-shared-libgcc" ]
+  | Clang, Compile -> [ "-x"; "c++" ]
+  | Clang, Link -> [ "-lc++" ]
+  | Msvc, Compile -> [ "/TP" ]
   | _ -> []
 
 let preprocessed_filename = "ccomp"
@@ -38,8 +45,8 @@ let check_warn = function
       ]
   | _ -> ()
 
-let get_flags dir =
+let get_flags ~for_ dir =
   let open Action_builder.O in
   let+ ccomp_type = ccomp_type dir in
   check_warn ccomp_type;
-  base_cxx_flags ccomp_type
+  base_cxx_flags ~for_ ccomp_type

src/dune_rules/cxx_flags.mli

@@ -4,10 +4,14 @@ open! Stdune
 
 open Dune_engine
 
+type phase =
+  | Compile
+  | Link
+
 (** The name of the file created in the .dune folder after calling the C
     preprocessor *)
 val preprocessed_filename : string
 
 (** [get_flags c_compiler] returns the necessary flags to turn this compiler
     into a c++ compiler for some of the most common compilers *)
-val get_flags : Path.Build.t -> string list Action_builder.t
+val get_flags : for_:phase -> Path.Build.t -> string list Action_builder.t

src/dune_rules/dune_file.ml

@@ -341,6 +341,11 @@ module Buildable = struct
 
   let has_foreign t =
     List.is_non_empty t.foreign_stubs || List.is_non_empty t.foreign_archives
+
+  let has_foreign_cxx t =
+    List.exists
+      ~f:(fun stub -> Foreign_language.(equal Cxx stub.Foreign.Stubs.language))
+      t.foreign_stubs
 end
 
 module Public_lib = struct
@@ -778,6 +783,8 @@ module Library = struct
 
   let has_foreign t = Buildable.has_foreign t.buildable
 
+  let has_foreign_cxx t = Buildable.has_foreign_cxx t.buildable
+
   let foreign_archives t =
     (if List.is_empty t.buildable.foreign_stubs then
       []
@@ -1526,6 +1533,8 @@ module Executables = struct
 
   let has_foreign t = Buildable.has_foreign t.buildable
 
+  let has_foreign_cxx t = Buildable.has_foreign_cxx t.buildable
+
   let obj_dir t ~dir = Obj_dir.make_exe ~dir ~name:(snd (List.hd t.names))
 end
 

src/dune_rules/dune_file.mli

@@ -49,6 +49,9 @@ module Buildable : sig
 
   (** Check if the buildable has any foreign stubs or archives. *)
   val has_foreign : t -> bool
+
+  (** Check if the buildable has any c++ foreign stubs. *)
+  val has_foreign_cxx : t -> bool
 end
 
 module Public_lib : sig
@@ -162,6 +165,9 @@ module Library : sig
   (** Check if the library has any foreign stubs or archives. *)
   val has_foreign : t -> bool
 
+  (** Check if the library has any c++ foreign stubs. *)
+  val has_foreign_cxx : t -> bool
+
   (** The list of all foreign archives, including the foreign stubs archive. *)
   val foreign_archives : t -> Foreign.Archive.t list
 
@@ -262,6 +268,9 @@ module Executables : sig
   (** Check if the executables have any foreign stubs or archives. *)
   val has_foreign : t -> bool
 
+  (** Check if the executables have any c++ foreign stubs. *)
+  val has_foreign_cxx : t -> bool
+
   val obj_dir : t -> dir:Path.Build.t -> Path.Build.t Obj_dir.t
 end
 

src/dune_rules/exe_rules.ml

@@ -104,6 +104,7 @@ let executables_rules ~sctx ~dir ~expander ~dir_contents ~scope ~compile_info
   in
   let* () = Check_rules.add_obj_dir sctx ~obj_dir in
   let ctx = Super_context.context sctx in
+  let project = Scope.project scope in
   let* pp =
     let instrumentation_backend =
       Lib.DB.instrumentation_backend (Scope.libs scope)
@@ -132,10 +133,8 @@ let executables_rules ~sctx ~dir ~expander ~dir_contents ~scope ~compile_info
         let* m = Pp_spec.pp_module_as pp name m in
         let add_empty_intf =
           (add_empty_intf
-          ||
-          let project = Scope.project scope in
-          Dune_project.executables_implicit_empty_intf project
-          && List.mem executable_names name ~equal:Module_name.equal)
+          || Dune_project.executables_implicit_empty_intf project
+             && List.mem executable_names name ~equal:Module_name.equal)
           && not (Module.has m ~ml_kind:Intf)
         in
         if add_empty_intf then
@@ -144,7 +143,7 @@ let executables_rules ~sctx ~dir ~expander ~dir_contents ~scope ~compile_info
           Memo.Build.return m)
   in
   let programs = programs ~modules ~exes in
-  let explicit_js_mode = Dune_project.explicit_js_mode (Scope.project scope) in
+  let explicit_js_mode = Dune_project.explicit_js_mode project in
   let linkages = linkages ctx ~exes ~explicit_js_mode in
   let* flags = Super_context.ocaml_flags sctx ~dir exes.buildable.flags in
   let cctx =
@@ -174,15 +173,23 @@ let executables_rules ~sctx ~dir ~expander ~dir_contents ~scope ~compile_info
        files directly to improve perf. *)
     let link_deps, sandbox = Dep_conf_eval.unnamed ~expander exes.link_deps in
     let link_args =
-      let standard = Action_builder.return [] in
+      let standard =
+        match Dune_project.use_standard_c_and_cxx_flags project with
+        | Some true when Buildable.has_foreign_cxx exes.buildable ->
+          let open Action_builder.O in
+          let+ flags = Cxx_flags.get_flags ~for_:Link dir in
+          List.concat_map flags ~f:(fun f -> [ "-cclib"; f ])
+        | _ -> Action_builder.return []
+      in
       let open Action_builder.O in
       let link_flags =
         link_deps
         >>> Expander.expand_and_eval_set expander exes.link_flags ~standard
       in
       let+ flags = link_flags
       and+ ctypes_cclib_flags =
-        Ctypes_rules.ctypes_cclib_flags ~scope ~standard ~expander
+        Ctypes_rules.ctypes_cclib_flags ~scope
+          ~standard:(Action_builder.return []) ~expander
           ~buildable:exes.buildable
       in
       Command.Args.S

src/dune_rules/foreign.ml

@@ -191,6 +191,10 @@ module Sources = struct
     String.Map.keys t
     |> List.map ~f:(fun c -> Path.Build.relative dir (c ^ ext_obj))
 
+  let has_cxx_sources (t : t) =
+    String.Map.exists t ~f:(fun (_loc, source) ->
+        Foreign_language.(equal Cxx source.stubs.language))
+
   module Unresolved = struct
     type t = (Foreign_language.t * Path.Build.t) String.Map.Multi.t
 

src/dune_rules/foreign.mli

@@ -166,6 +166,8 @@ module Sources : sig
   val object_files :
     t -> dir:Path.Build.t -> ext_obj:string -> Path.Build.t list
 
+  val has_cxx_sources : t -> bool
+
   (** A map from object names to lists of possible language/path combinations. *)
   module Unresolved : sig
     type t = (Foreign_language.t * Path.Build.t) String.Map.Multi.t

src/dune_rules/lib_rules.ml

@@ -43,10 +43,19 @@ let build_lib (lib : Library.t) ~native_archives ~sctx ~expander ~flags ~dir
         Action_builder.paths (Cm_files.unsorted_objects_and_cms cm_files ~mode)
       in
       let ocaml_flags = Ocaml_flags.get flags mode in
-      let standard = Action_builder.return [] in
+      let standard =
+        let project =
+          Super_context.find_scope_by_dir sctx dir |> Scope.project
+        in
+        match Dune_project.use_standard_c_and_cxx_flags project with
+        | Some true when Buildable.has_foreign_cxx lib.buildable ->
+          Cxx_flags.get_flags ~for_:Link dir
+        | _ -> Action_builder.return []
+      in
       let cclibs =
         Expander.expand_and_eval_set expander lib.c_library_flags ~standard
       in
+      let standard = Action_builder.return [] in
       let library_flags =
         Expander.expand_and_eval_set expander lib.library_flags ~standard
       in
@@ -115,20 +124,26 @@ let gen_wrapped_compat_modules (lib : Library.t) cctx =
       |> Super_context.add_rule sctx ~loc ~dir:(Compilation_context.dir cctx))
 
 (* Rules for building static and dynamic libraries using [ocamlmklib]. *)
-let ocamlmklib ~loc ~c_library_flags ~sctx ~dir ~expander ~o_files ~archive_name
+let ocamlmklib ~loc ~c_library_flags ~sctx ~dir ~o_files ~archive_name
     ~build_targets_together =
   let ctx = Super_context.context sctx in
   let { Lib_config.ext_lib; ext_dll; _ } = ctx.lib_config in
   let static_target =
     Foreign.Archive.Name.lib_file archive_name ~dir ~ext_lib
   in
+  let cclibs =
+    Action_builder.map c_library_flags ~f:(fun cclibs ->
+        (* https://github.com/ocaml/dune/issues/119 *)
+        let cclibs =
+          match ctx.lib_config.ccomp_type with
+          | Msvc -> msvc_hack_cclibs cclibs
+          | Other _ -> cclibs
+        in
+        Command.quote_args "-ldopt" cclibs)
+  in
   let build ~custom ~sandbox targets =
     Super_context.add_rule sctx ~dir ~loc
       (let open Action_builder.With_targets.O in
-      let cclibs_args =
-        Expander.expand_and_eval_set expander c_library_flags
-          ~standard:(Action_builder.return [])
-      in
       let ctx = Super_context.context sctx in
       Command.run ~dir:(Path.build ctx.build_dir) ctx.ocamlmklib
         [ A "-g"
@@ -139,16 +154,9 @@ let ocamlmklib ~loc ~c_library_flags ~sctx ~dir ~expander ~o_files ~archive_name
         ; A "-o"
         ; Path (Path.build (Foreign.Archive.Name.path ~dir archive_name))
         ; Deps o_files
-        ; Dyn
-            (* The [c_library_flags] is needed only for the [dynamic_target]
-               case, but we pass them unconditionally for simplicity. *)
-            (Action_builder.map cclibs_args ~f:(fun cclibs ->
-                 (* https://github.com/ocaml/dune/issues/119 *)
-                 match ctx.lib_config.ccomp_type with
-                 | Msvc ->
-                   let cclibs = msvc_hack_cclibs cclibs in
-                   Command.quote_args "-ldopt" cclibs
-                 | Other _ -> As cclibs))
+          (* The [c_library_flags] is needed only for the [dynamic_target] case,
+             but we pass them unconditionally for simplicity. *)
+        ; Dyn cclibs
         ; Hidden_targets targets
         ]
       >>| Action.Full.add_sandbox sandbox)
@@ -192,30 +200,40 @@ let ocamlmklib ~loc ~c_library_flags ~sctx ~dir ~expander ~o_files ~archive_name
 let foreign_rules (library : Foreign.Library.t) ~sctx ~expander ~dir
     ~dir_contents =
   let archive_name = library.archive_name in
+  let* foreign_sources =
+    Dir_contents.foreign_sources dir_contents
+    >>| Foreign_sources.for_archive ~archive_name
+  in
   let* o_files =
-    let* foreign_sources =
-      Dir_contents.foreign_sources dir_contents
-      >>| Foreign_sources.for_archive ~archive_name
-    in
     Foreign_rules.build_o_files ~sctx ~dir ~expander
       ~requires:(Resolve.return []) ~dir_contents ~foreign_sources
     |> Memo.Build.parallel_map ~f:(Memo.Build.map ~f:Path.build)
   in
   let* () = Check_rules.add_files sctx ~dir o_files in
-  ocamlmklib ~archive_name ~loc:library.stubs.loc
-    ~c_library_flags:Ordered_set_lang.Unexpanded.standard ~sctx ~dir ~expander
+  let standard =
+    let project = Super_context.find_scope_by_dir sctx dir |> Scope.project in
+    match Dune_project.use_standard_c_and_cxx_flags project with
+    | Some true when Foreign.Sources.has_cxx_sources foreign_sources ->
+      Cxx_flags.get_flags ~for_:Link dir
+    | _ -> Action_builder.return []
+  in
+  let c_library_flags =
+    Expander.expand_and_eval_set expander Ordered_set_lang.Unexpanded.standard
+      ~standard
+  in
+  ocamlmklib ~archive_name ~loc:library.stubs.loc ~c_library_flags ~sctx ~dir
     ~o_files ~build_targets_together:false
 
 (* Build a required set of archives for an OCaml library. *)
 let build_stubs lib ~cctx ~dir ~expander ~requires ~dir_contents
     ~vlib_stubs_o_files =
   let sctx = Compilation_context.super_context cctx in
+  let* foreign_sources =
+    let+ foreign_sources = Dir_contents.foreign_sources dir_contents in
+    let name = Library.best_name lib in
+    Foreign_sources.for_lib foreign_sources ~name
+  in
   let* lib_o_files =
-    let* foreign_sources =
-      let+ foreign_sources = Dir_contents.foreign_sources dir_contents in
-      let name = Library.best_name lib in
-      Foreign_sources.for_lib foreign_sources ~name
-    in
     Foreign_rules.build_o_files ~sctx ~dir ~expander ~requires ~dir_contents
       ~foreign_sources
     |> Memo.Build.parallel_map ~f:(Memo.Build.map ~f:Path.build)
@@ -232,8 +250,18 @@ let build_stubs lib ~cctx ~dir ~expander ~requires ~dir_contents
       modes.native && modes.byte
       && Dynlink_supported.get lib.dynlink ctx.supports_shared_libraries
     in
-    ocamlmklib ~archive_name ~loc:lib.buildable.loc ~sctx ~expander ~dir
-      ~o_files ~c_library_flags:lib.c_library_flags ~build_targets_together
+    let standard =
+      let project = Super_context.find_scope_by_dir sctx dir |> Scope.project in
+      match Dune_project.use_standard_c_and_cxx_flags project with
+      | Some true when Foreign.Sources.has_cxx_sources foreign_sources ->
+        Cxx_flags.get_flags ~for_:Link dir
+      | _ -> Action_builder.return []
+    in
+    let c_library_flags =
+      Expander.expand_and_eval_set expander lib.c_library_flags ~standard
+    in
+    ocamlmklib ~archive_name ~loc:lib.buildable.loc ~sctx ~dir ~o_files
+      ~c_library_flags ~build_targets_together
 
 let build_shared lib ~native_archives ~sctx ~dir ~flags =
   let ctx = Super_context.context sctx in

src/dune_rules/super_context.ml

@@ -16,7 +16,7 @@ let default_context_flags (ctx : Context.t) ~project =
       let c = cflags @ Ocaml_config.ocamlc_cppflags ctx.ocaml_config in
       let cxx =
         let open Action_builder.O in
-        let+ db_flags = Cxx_flags.get_flags ctx.build_dir in
+        let+ db_flags = Cxx_flags.get_flags ~for_:Compile ctx.build_dir in
         db_flags @ cxxflags
       in
       (Action_builder.return c, cxx)

test/blackbox-tests/test-cases/cxx-flags.t/baz.cpp

@@ -1,2 +1,12 @@
 #include <caml/mlvalues.h>
+#include <caml/io.h>
+#include <iostream>
+
 extern "C" value baz(value unit) { return Val_int(2046); }
+
+extern "C" void hello_world_baz ();
+
+void hello_world_baz ()
+{
+  std::cout << "Hello World Baz!\n";
+}

test/blackbox-tests/test-cases/cxx-flags.t/bazexe.cpp

@@ -0,0 +1,11 @@
+#include <caml/mlvalues.h>
+#include <iostream>
+
+extern "C" value bazexe(value unit) { return Val_int(4096); }
+
+extern "C" void hello_world_bazexe ();
+
+void hello_world_bazexe ()
+{
+  std::cout << "Hello World Bazexe!";
+}

test/blackbox-tests/test-cases/cxx-flags.t/dune

@@ -6,5 +6,5 @@
 (executable
  (name main)
  (libraries quad)
+ (foreign_stubs (language cxx) (names bazexe))
  (modules main))
-

test/blackbox-tests/test-cases/cxx-flags.t/main.ml

@@ -1 +1,9 @@
-let () = Printf.printf "%d" (Quad.quad ())
+external bazexe : unit -> int = "bazexe"
+external hello_world_bazexe : unit -> unit = "hello_world_bazexe"
+
+let () = Quad.hello (); hello_world_bazexe ()
+
+
+let () = Printf.printf "%d\n%d\n"
+  (Quad.quad ())
+  (bazexe ())

test/blackbox-tests/test-cases/cxx-flags.t/quad.ml

@@ -1,2 +1,6 @@
 external baz : unit -> int = "baz"
+
+external hello_world_baz : unit -> unit = "hello_world_baz"
+
 let quad x = baz x
+let hello () = hello_world_baz ()