Add necessary C++ flags for most common compilers

[voodoos]

This PR is an attempt to answer #3528.

It adds a new module with a small "database" of C++ flags that are necessary to the use of some of the most common C compiler to build C++ code.

Given the actual context and whether C or C++ code is being built, this module provide the set of necessary flags that should be added to the compiler command line : from the database mentioned above for C++ and from the config ocamlc_cflags and ocamlc_cppflags for C. For convenience it also add the FDO flags tot he list and also returns the output args that should be used for the specified compiler.

The main issue is that, because these C++ flags where not added by Dune before, users probably added them themselves and they will be duplicated in the command line. I am not sure whether this would break builds or not, but if it does we should consider enabling this change only for dune 2.8 or with a specific option in the dune-project file.

The added flags for C++ are the following:

Compiler	C++ flags
gcc	-x c++ -lstdc++ -shared-libgcc
clang	-x c++
msvc	/TP

[voodoos]

Also, does someone has a good strategy in mind for testing these changes ?

[dra27]

Not a comprehensive review. This may well work now, but it does feel very brittle - there are Windows stories where the compiler may very well be callable, but this test would fail (and there are both clang and gcc on Windows, not that I actually used clang on Windows before).

The OCaml implementation of this is at https://github.com/ocaml/ocaml/blob/cb32b88aa1205339dc888a69a895e0de40b1bd3c/aclocal.m4#L38-L62 which works by invoking the C compiler.

Calling the C compiler every time might be a bit heavy (although ocamlc -config is acceptable, so maybe not?), but would it be possible to introduce a rule which is depended on by rules requiring the C++ compiler which effectively does this test once? Essentially, were I trying to detect this in dune files directly, I'd have something along the lines of:

(rule
  (with-stdout-to cxxflags.sexp (run configure.exe get-cxx-flags)))

(library
  ..
  (cflags (:include cxxflags.sexp)))

(cf.

[voodoos]

Not a comprehensive review. This may well work now, but it does feel very brittle - there are Windows stories where the compiler may very well be callable, but this test would fail (and there are both clang and gcc on Windows, not that I actually used clang on Windows before).

Thank you for your feedback !

We discussed this matter with @jeremiedimino and decided to keep the current strategy even if it is not complete.
We will make it an opt-in behaviour in the dune-project and ask early users to open an issue when the compiler detection fails to identify the compiler. This should help us identify the cases that need specific handling.

It also feels that this detection being done by OCaml, it could be easily reflected in the ocamlc_config ccomp_type field, shouldn't it ?

[rgrinberg]

Also, does someone has a good strategy in mind for testing these changes ?
Consider using https://github.com/dune-universe/dune-universe

[voodoos]

After discussing the matter with @jeremiedimino, we decided to group both this PR and the one about C-flags under the same option to facilitate users ' conversion.

That led me to the quite ugly option name no_forced_c_flags_and_more_cxx_flags. If you have a better idea I am willing to change it.

This also means that both these PR should be merged in the same release (@rgrinberg).

[ghost]

One idea would be to emphasis the fact that we want to make this the default in the future: future_c_and_cxx_flags_handling

[rgrinberg]

One idea would be to emphasis the fact that we want to make this the default in the future: future_c_and_cxx_flags_handling

IMO, that's a bad name. It's going to be quite confusing when the future this option refers to will be the present. We should stick to names that reflect the behavior as best as possible.

[voodoos]

Compiler detection broke on MacOS Big Sur: cc is not a symlink to clang anymore but some sort of executable proxy.
Now I feel like we really need the detection to be more clever or too many users will receive the warning.

Our options are:

1. Have the compiler add this to ocamlc -config
2. Parse the --version output
3. Test which preprocessor directives are defined (probably le best)

The difficulty is that we want to group this under the same option as Standard C flags but the timing may be bad for having everything on the same release. But if we decide the option not to be a boolean we could have:

(use_standard_c_and_cxx_flags none)
(use_standard_c_and_cxx_flags c_only)
(use_standard_c_and_cxx_flags both)

That way if this PR is not ready for Dune 2.8 we can just ship the first two options.

@jeremiedimino do you have an opinion on that ?

[ghost]

The version without the argument seems simpler to me and one version is not a long time to wait. So I'd say we hold off releasing the first feature to user until the second one is ready.

AFAIU, the first feature is already in master. We don't need to revert it, instead we can "disconnect" the feature, i.e. disable the parsing of the option and let it always be disabled. And also disable the tests until 2.8 is released.

[voodoos]

I reworked the detection of the compiler:

• In the .dune folder a small C header file is written with preprocessor directives that check if some macros are define or not
• When needed Dune will run the C compiler's preprocessor on this file and the output, the compiler type, will be written in a file in the .dune folder
• This file is read when determining the content of the :standard flags that depend on the compiler type.

The current preprocessing file is the following:

#if defined ( __clang__ )
  #define CCOMP clang
#elif defined ( __MINGW32__ ) || defined ( __MINGW64__ )
  #define CCOMP mingw
#elif defined ( _MSC_VER )
  #define CCOMP msvc
#elif defined ( __GNUC__ )
  #define CCOMP gcc
#else
  #define CCOMP other
#endif

CCOMP

@jeremiedimino, @dra27 does that look more robust to you ?

@rgrinberg When do you plan to release dune 2.8 ? It would be great if we could squeeze this in because it blocks the C improvments PR (#3875) which is also ready. However no need to delay further the release, if necessary these will wait for 2.9.

[dra27]

This distinction isn't used anywhere (I think). Two options:

• mingw-w64 defines __GNUC__, so this could be omitted
• I haven't used/tested it, but it's possible (and done) to build mingw-w64 using clang - so if this distinction is to be kept, I think __MINGW32__ should be tested before __clang__ since otherwise __MINGW32__ trumps __GNUC__ but not __clang__

[ghost]

Looks good, except for the conditional generation of the rule in gen_rules.ml

[ghost]

LGTM

[voodoos]

@ejgallego this has already been released in 2.8.0, but with an option in the dune-project to enable it.

I think we want to make that default for 3.0.0, i will change the Milestone accordingly.