[RFC] Rationalize C compiler flags

[voodoos]

The present situation

In order to make things clearer I tried to retrace the processing of the C compilation flags, here is what I got:
(you can click to enlarge the image)

Please tell me if I forgot something or misunderstood part of it.

What is wrong

• ocamlc_cflags can end duplicated in the compiler command-line (if :standard was passed all the way down)
• Dune silently adds ocamlc_cflags + ocamlc_cppflags to the command line making the :standard set of flags SC.default_c_cxx_flags irrelevant. This was unavoidable before because dune was calling ocamlc to compile C code.
• The CC variable will always miss the ocamlc_cppflags and sometimes the ocamlc_cflags if the standard value was discarded, while these are in fact part of the compiler command line.

Proposition

Stop adding silently the flags from the config. These should be present in the :standard field and the users should be reminded that they need to use :standard when they want to add flags to the default set. This was not possible when dune was calling ocamlc to build C code but it is now that the C compiler is directly summoned.

This would solve the previous problem:

• No more duplication of flags
• The CC variable would really contain the base set of flags used for compiling
• Users will be in total control of the flags set

The new flow would be:

Of course this change will break some projects in which the :standard set of flags was overridden by the user. So we should add an option to the dune-project file for users that want to use the new system before 3.0.

[voodoos]

After discussing this proposal with @jeremiedimino an interrogation remained: what should the variable %{CC} contain ?

• In current dune it may or may not contain flags from ocamlc_cflags depending on the use of :standard and user-defined flags in env stanzas. But may not reflect the base compiler command line because more flags are added at the last minute (ocamlc_cflags / cppflags).

• In the proposal it would always reflect the "base" (without stub-specific flags) compiler command line used by dune to compile C stubs: cflags and cppflags depending on :standard use and environment-defined flags. Nothing is added silently.

We decided that the CC variable should behave as it does in other tools like auto-conf, however the manual is a bit confusing. @dra27, can you confirm whether or not the proposed behaviour is similar to the meaning of CC in autoconf ?

[samoht]

That seems like a very good proposal. For MirageOS we would like to cross-compile packages only using the flags declared in the current workspace context. A typical example would be:

(foreign_library
 (archive_name mirage-xen_bindings)
 (language c)
 (include_dirs ../include)
 (names main bmap evtchn gnttab
        alloc_pages_stubs atomic_stubs barrier_stubs checksum_stubs clock_stubs
        cstruct_stubs mm_stubs)
 (flags :standard -O2 -std=c99 -Wall -Werror
        -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__))

Here :standard means: the C flags inherited from dune-worspace (which is initialised with pkg-config to get the correct flags). We really don't want to have the default flags from the OCaml compiler (e.g. we don't want -fPIC). So removing that magic addition of C flags is really critical for mirage.

The current workaround is to build the archive in a sandboxing build:

(env (_ (c_flags :standard -O2 -std=c99 -Wall -Werror
        -D__XEN_INTERFACE_VERSION__=__XEN_LATEST_INTERFACE_VERSION__)))

(rule
 (deps (source_tree .) (source_tree ../include))
 (targets mirage-xen-bindings.a)
 (action (run make CC=%{cc})))

But this needs we have to modify every foreign_library stanza to make that work (and thus we can't import foreign code). The envisaged workaround is to set-up a complete compiler toolchain in each different contexts, but that's a bit heavy-weight when the only necessary thing that we need is to change the C flags.

[voodoos]

@rgrinberg, do you agree that this proposal is an improvement on the current flags control ?

It should not necessitate a lot of changes now that Dune is calling the C compiler without the ocamlc proxy.
Of course it will break compatibility with users who didn't include the :standard set of flags when defining some c_flags.
We can either:

• Limit the change to the next dune-lang version and users will need to update their (arguably wrong) files only if they want to update their dune lang version.
• Make these changes opt-in via an option in the dune-project file, and make this new behavior (which appears more sane) default only in the next major dune version.

[mato]

But this needs we have to modify every foreign_library stanza to make that work (and thus we can't import foreign code). The envisaged workaround is to set-up a complete compiler toolchain in each different contexts, but that's a bit heavy-weight when the only necessary thing that we need is to change the C flags.

Just to add some context here: Having full control over what goes into the C compiler flags is only part of the solution. The larger issue stems from the somewhat arbitrary mixing of PIC and non-PIC C code, and the fact that shared objects are built unconditionally / foreign libraries are linked into .cmxs files. If it were possible to disable the latter on a case-by-case basis then I could just force PIC off with -fno-pic for the relevant libraries.

The only workaround which currently works without requiring any hacks or foreign library changes is to create a custom compiler variant where that compiler is configured with --disable-shared, which propagates down to dune nicely.

[rgrinberg]

@voodoos this proposal looks good. Could also use this opportunity to standardize flag passing when compiling things with configurator?

[rgrinberg]

@mato @samoht here's a related proposal that might help you guys out.

We really don't want to have the default flags from the OCaml compiler (e.g. we don't want -fPIC)

Makes sense. Would it also makes sense to modify this default set of flags?

The only workaround which currently works without requiring any hacks or foreign library changes is to create a custom compiler variant where that compiler is configured with --disable-shared, which propagates down to dune nicely.

That doesn't seem like a hack. It seems like there should be a way to set this option for any context from inside dune.

How about we make it possible to override the -config options for any context from inside dune? This could be done in the workspace file:

(context
 (default
  (config
   (supports_shared _libraries false))))

[voodoos]

@voodoos this proposal looks good. Could also use this opportunity to standardize flag passing when compiling things with configurator?

@rgrinberg I have very little knowledge on what dune configurator is doing, can you elaborate on that or give me some pointers ?

[rgrinberg]

Binaries that use configurator also compile C sources with Configurator.V1.c_test and the like. It would be nice if these could use the same flags dune is using to compile executables in the same directory.

[mato]

I've tested #3875 against a version of mirage-xen with the Makefile-based build reverted (branch: https://github.com/mato/mirage-xen/tree/test-dune-cflags), and can confirm that it works as advertised.

However, it only solves part of the problem for Mirage, as the build breaks later when shared libraries are produced. To fix that, we indeed need something like what @rgrinberg mentioned in #3718 (comment).

Shall I create a separate issue to discuss that? If so, where?

[ghost]

Creating a separate issue to discuss this seems good to me. We should always make sure we discuss one subject per issue/PR, to keep the discussion easy to follow.

[mato]

Creating a separate issue to discuss this seems good to me.

Done, #3934.