Move default C flags to the :standard set

CHANGES.md

@@ -127,6 +127,13 @@ Unreleased
   for different modules in the same folder. (#4092, fixes #2596, #1212 and
   #3409, @voodoos)
 
+- Add the option `use_standard_c_and_cxx_flags` to `dune-project` that 1.
+  disables the unconditional use of the `ocamlc_cflags` and `ocamlc_cppflags`
+  from `ocamlc -config` in C compiler calls, these flags will be present in the
+  `:standard` set instead; and 2. enables the detection of the C compiler family
+  and populates the `:standard` set of flags with common default values when
+  building CXX stubs. (#3875, #3802, fix #3718 and #3528, @voodoos)
+
 2.7.1 (2/09/2020)
 -----------------
 

bin/import.ml

@@ -105,12 +105,7 @@ module Main = struct
           Package.Name.Map.filter workspace.conf.packages ~f:(fun pkg ->
               let vendored =
                 let dir = Package.dir pkg in
-                match Dune_engine.File_tree.find_dir dir with
-                | None -> assert false
-                | Some d -> (
-                  match Dune_engine.File_tree.Dir.status d with
-                  | Vendored -> true
-                  | _ -> false )
+                Dune_engine.File_tree.is_vendored dir
               in
               let name = Package.name pkg in
               let included = Package.Name.Set.mem names name in

bin/install_uninstall.ml

@@ -314,12 +314,7 @@ let file_operations ~dry_run ~workspace : (module File_operations) =
 
 let package_is_vendored (pkg : Dune_engine.Package.t) =
   let dir = Package.dir pkg in
-  match Dune_engine.File_tree.find_dir dir with
-  | None -> assert false
-  | Some d -> (
-    match Dune_engine.File_tree.Dir.status d with
-    | Vendored -> true
-    | _ -> false )
+  Dune_engine.File_tree.is_vendored dir
 
 let install_uninstall ~what =
   let doc = sprintf "%s packages." (String.capitalize what) in

doc/concepts.rst

@@ -140,8 +140,7 @@ Dune supports the following variables:
   the value of ``workspace_root`` is not constant and depends on
   whether your project is vendored or not
 -  ``CC`` is the C compiler command line (list made of the compiler
-   name followed by its flags) that was used to compile OCaml in the
-   current build context
+   name followed by its flags) that will be used to compile foreign code. For more details about its content see :ref:`this section <flags-flow>`.
 -  ``CXX`` is the C++ compiler command line being used in the
    current build context
 -  ``ocaml_bin`` is the path where ``ocamlc`` lives
@@ -1177,6 +1176,10 @@ Here is a complete list of supported subfields:
 - ``flags`` are passed when compiling source files. This field is specified
   using the :ref:`ordered-set-language`, where the ``:standard`` value comes
   from the environment settings ``c_flags`` and ``cxx_flags``, respectively.
+  Note that, for C subs, Dune unconditionally adds the flags present in the
+  fields ``ocamlc_cflags`` and ``ocamlc_cppflags`` of the OCaml config to the
+  compiler command line. This behavior can be disabled since Dune 2.8 via the
+  ``dune-project`` option :ref:`always-add-cflags`.
 - ``include_dirs`` are tracked as dependencies and passed to the compiler
   via the ``-I`` flag. You can use :ref:`variables` in this field, and
   refer to a library source directory using the ``(lib library-name)`` syntax.
@@ -1239,3 +1242,24 @@ foreign archive is a bit like a foreign library, hence the name of the stanza.
 Foreign archives are particularly useful when embedding a library written in
 a foreign language and/or built with another build system. See
 :ref:`foreign-sandboxing` for more details.
+
+.. _flags-flow:
+
+Flags
+-----
+
+Depending on the :ref:`always-add-cflags` option, the base `:standard` set of
+flags for C will contain only ``ocamlc_cflags`` or both ``ocamlc_cflags`` and
+`ocamlc_cflags`.
+
+There are multiple levels where one can declare custom flags (using the
+:ref:`ordered-set-language`), and each level inherits the flags of the previous
+one in its `:standard` set:
+
+- In the global `env` definition of a `dune-workspace` file
+- In the per-context `env` definitions in a `dune-workspace` file
+- In the env definition of a `dune` file
+- In a `foreign_` field of an executable or a library
+
+The ``%{cc}`` :ref:`variable <variables>` will contain the flags from the first
+three levels only.

doc/dune-files.rst

@@ -285,6 +285,23 @@ language: The syntax is as a list of the following elements:
 
    dep-specification = dep+
 
+.. _always-add-cflags:
+
+use_standard_c_and_cxx_flags
+----------------------------
+
+Since Dune 2.8, it is possible to deactivate the systematic prepending of flags
+coming from ``ocamlc -config`` to the C compiler command line. This is done
+adding the following field to the ``dune-project`` file:
+
+.. code:: scheme
+
+    (use_standard_c_and_cxx_flags true)
+
+In this mode, dune will populate the ``:standard`` set of C flags with the
+content of ``ocamlc_cflags`` and  ``ocamlc_cppflags``. These flags can be
+completed or overridden using the :ref:`ordered-set-language`.
+
 dune
 ====
 

src/dune_engine/dune_project.ml

@@ -160,8 +160,8 @@ type t =
   ; implicit_transitive_deps : bool
   ; wrapped_executables : bool
   ; dune_version : Dune_lang.Syntax.Version.t
-  ; use_standard_c_and_cxx_flags : bool
   ; generate_opam_files : bool
+  ; use_standard_c_and_cxx_flags : bool option
   ; file_key : File_key.t
   ; dialects : Dialect.DB.t
   ; explicit_js_mode : bool
@@ -190,10 +190,10 @@ let file_key t = t.file_key
 
 let implicit_transitive_deps t = t.implicit_transitive_deps
 
-let use_standard_c_and_cxx_flags t = t.use_standard_c_and_cxx_flags
-
 let generate_opam_files t = t.generate_opam_files
 
+let use_standard_c_and_cxx_flags t = t.use_standard_c_and_cxx_flags
+
 let dialects t = t.dialects
 
 let explicit_js_mode t = t.explicit_js_mode
@@ -211,8 +211,8 @@ let to_dyn
     ; implicit_transitive_deps
     ; wrapped_executables
     ; dune_version
-    ; use_standard_c_and_cxx_flags
     ; generate_opam_files
+    ; use_standard_c_and_cxx_flags
     ; file_key
     ; dialects
     ; explicit_js_mode
@@ -233,8 +233,8 @@ let to_dyn
     ; ("implicit_transitive_deps", bool implicit_transitive_deps)
     ; ("wrapped_executables", bool wrapped_executables)
     ; ("dune_version", Dune_lang.Syntax.Version.to_dyn dune_version)
-    ; ("use_standard_c_and_cxx_flags", bool use_standard_c_and_cxx_flags)
     ; ("generate_opam_files", bool generate_opam_files)
+    ; ("use_standard_c_and_cxx_flags", option bool use_standard_c_and_cxx_flags)
     ; ("file_key", string file_key)
     ; ("dialects", Dialect.DB.to_dyn dialects)
     ; ("explicit_js_mode", bool explicit_js_mode)
@@ -609,8 +609,12 @@ let infer ~dir packages =
   ; extension_args
   ; parsing_context
   ; dune_version = lang.version
-  ; use_standard_c_and_cxx_flags = false
   ; generate_opam_files = false
+  ; use_standard_c_and_cxx_flags =
+      ( if lang.version < (3, 0) then
+        None
+      else
+        Some true )
   ; file_key
   ; dialects = Dialect.DB.builtin
   ; explicit_js_mode
@@ -691,13 +695,13 @@ let parse ~dir ~lang ~opam_packages ~file ~dir_status =
              "It is useless since the Merlin configurations are not ambiguous \
               anymore."
            loc lang.syntax (2, 8) ~what:"This field"
-     and+ use_standard_c_and_cxx_flags =
-       field_o_b "use_standard_c_and_cxx_flags"
-         ~check:(Dune_lang.Syntax.since Stanza.syntax (2, 8))
      and+ () = Dune_lang.Versioned_file.no_more_lang
      and+ generate_opam_files =
        field_o_b "generate_opam_files"
          ~check:(Dune_lang.Syntax.since Stanza.syntax (1, 10))
+     and+ use_standard_c_and_cxx_flags =
+       field_o_b "use_standard_c_and_cxx_flags"
+         ~check:(Dune_lang.Syntax.since Stanza.syntax (2, 8))
      and+ dialects =
        multi_field "dialect"
          ( Dune_lang.Syntax.since Stanza.syntax (1, 11)
@@ -806,15 +810,18 @@ let parse ~dir ~lang ~opam_packages ~file ~dir_status =
          ~default:(strict_package_deps_default ~lang)
      in
      let dune_version = lang.version in
-     let use_standard_c_and_cxx_flags =
-       Option.value ~default:false use_standard_c_and_cxx_flags
-     in
      let explicit_js_mode =
        Option.value explicit_js_mode ~default:(explicit_js_mode_default ~lang)
      in
      let generate_opam_files =
        Option.value ~default:false generate_opam_files
      in
+     let use_standard_c_and_cxx_flags =
+       match use_standard_c_and_cxx_flags with
+       | None when dune_version >= (3, 0) -> Some false
+       | None -> None
+       | some -> some
+     in
      let cram =
        match cram with
        | None -> false
@@ -858,8 +865,8 @@ let parse ~dir ~lang ~opam_packages ~file ~dir_status =
      ; implicit_transitive_deps
      ; wrapped_executables
      ; dune_version
-     ; use_standard_c_and_cxx_flags
      ; generate_opam_files
+     ; use_standard_c_and_cxx_flags
      ; dialects
      ; explicit_js_mode
      ; format_config

src/dune_engine/dune_project.mli

@@ -63,13 +63,13 @@ val root : t -> Path.Source.t
 
 val stanza_parser : t -> Stanza.t list Dune_lang.Decoder.t
 
+val generate_opam_files : t -> bool
+
 (** The option [use_standard_c_and_cxx_flags] enables the automatic addition of
     flags necessary to build c++ files with the active C compiler. It also
     disables the automatic addition of C flags from [ocamlc -config] to the
     compiler command line when building C stubs. *)
-val use_standard_c_and_cxx_flags : t -> bool
-
-val generate_opam_files : t -> bool
+val use_standard_c_and_cxx_flags : t -> bool option
 
 val dialects : t -> Dialect.DB.t
 

src/dune_engine/file_tree.ml

@@ -782,3 +782,8 @@ let find_dir_specified_on_command_line ~dir =
       [ Pp.textf "Don't know about directory %s specified on the command line!"
           (Path.Source.to_string_maybe_quoted dir)
       ]
+
+let is_vendored dir =
+  match find_dir dir with
+  | None -> false
+  | Some d -> Dir.status d = Vendored

src/dune_engine/file_tree.mli

@@ -93,6 +93,9 @@ val files_of : Path.Source.t -> Path.Source.Set.t
 (** [true] iff the path is a directory *)
 val dir_exists : Path.Source.t -> bool
 
+(** [true] iff the path is a vendored directory *)
+val is_vendored : Path.Source.t -> bool
+
 (** [true] iff the path is a file *)
 val file_exists : Path.Source.t -> bool
 

src/dune_rules/foreign_rules.ml

@@ -75,19 +75,53 @@ let include_dir_flags ~expander ~dir (stubs : Foreign.Stubs.t) =
 
 let build_c ~kind ~sctx ~dir ~expander ~include_flags (loc, src, dst) =
   let ctx = Super_context.context sctx in
+  let project = Super_context.find_scope_by_dir sctx dir |> Scope.project in
+  let use_standard_flags = Dune_project.use_standard_c_and_cxx_flags project in
   let base_flags =
     let cfg = ctx.ocaml_config in
     match kind with
-    | Foreign_language.C ->
-      List.concat
-        [ Ocaml_config.ocamlc_cflags cfg
-        ; Ocaml_config.ocamlc_cppflags cfg
-        ; Fdo.c_flags ctx
-        ]
+    | Foreign_language.C -> (
+      match use_standard_flags with
+      | None
+      | Some false ->
+        (* In dune < 2.8 flags from ocamlc_config are always added *)
+        List.concat
+          [ Ocaml_config.ocamlc_cflags cfg
+          ; Ocaml_config.ocamlc_cppflags cfg
+          ; Fdo.c_flags ctx
+          ]
+      | Some true -> Fdo.c_flags ctx )
     | Foreign_language.Cxx -> Fdo.cxx_flags ctx
   in
   let with_user_and_std_flags =
     let flags = Foreign.Source.flags src in
+    (* DUNE3 will have [use_standard_c_and_cxx_flags] enabled by default. To
+       guide users toward this change we emit a warning when dune_lang is >=
+       1.8, [use_standard_c_and_cxx_flags] is not specified in the
+       [dune-project] file (thus defaulting to [true]), the [:standard] set of
+       flags has been overriden and we are not in a vendored project *)
+    let has_standard = Ordered_set_lang.Unexpanded.has_standard flags in
+    let is_vendored =
+      match Path.Build.drop_build_context dir with
+      | Some src_dir -> Dune_engine.File_tree.is_vendored src_dir
+      | None -> false
+    in
+    if
+      Dune_project.dune_version project >= (2, 8)
+      && Option.is_none use_standard_flags
+      && (not is_vendored) && not has_standard
+    then
+      User_warning.emit ~loc
+        [ Pp.text
+            "The flag set for these foreign sources overrides the `:standard` \
+             set of flags. However the flags in this standard set are still \
+             added to the compiler arguments by Dune. This might cause \
+             unexpected issues. You can disable this warning by defining the \
+             option `(use_standard_c_and_cxx_flags <bool>)` in your \
+             `dune-project` file. Setting this option to `true` will \
+             effectively prevent Dune from silently adding c-flags to the \
+             compiler arguments which is the new recommended behaviour."
+        ];
     Super_context.foreign_flags sctx ~dir ~expander ~flags ~language:kind
     |> Build.map ~f:(List.append base_flags)
   in

src/dune_rules/ordered_set_lang.ml

@@ -300,6 +300,17 @@ module Unexpanded = struct
     in
     loop t.ast
 
+  let has_standard t =
+    let rec loop ast =
+      match ast with
+      | Ast.Standard -> true
+      | Ast.Element _ -> false
+      | Ast.Union l -> List.exists ~f:loop l
+      | Ast.Diff (l, r) -> loop l || loop r
+      | Ast.Include _ -> false
+    in
+    loop t.ast
+
   type position =
     | Pos
     | Neg

src/dune_rules/ordered_set_lang.mli

@@ -63,6 +63,8 @@ module Unexpanded : sig
 
   val has_special_forms : t -> bool
 
+  val has_standard : t -> bool
+
   (** List of files needed to expand this set *)
   val files : t -> f:(String_with_vars.t -> Path.t) -> Path.Set.t