Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Better support for C++ #3528

Closed
ghost opened this issue Jun 4, 2020 · 7 comments · Fixed by #3802
Closed

Better support for C++ #3528

ghost opened this issue Jun 4, 2020 · 7 comments · Fixed by #3802
Assignees
@voodoos

Comments

@ghost
Copy link

ghost commented Jun 4, 2020

Right now, OCaml uses the compiler specified in the c_compiler field of ocamlc -config to compile C++ sources. We rely on the C compiler detecting the .cpp extension to decide that the language is C++. This works ok with gcc, but doesn't work so well with other C compilers such as clang.

As a result, people who are compiling C++ stubs with Dune have to manually add -x c++ -lstdc++ to their compilation flags, which is not ideal. Instead, we would like Dune to do this automatically. Since the flags are not the same for all C compilers, we will need to construct a small database with the flags for various well-known C compilers.
@ghost ghost mentioned this issue Jun 8, 2020
Closed
@ghost ghost assigned voodoos Jun 16, 2020
@voodoos
Copy link
Collaborator

voodoos commented Jul 10, 2020

Do you think these flags should be added to the :standard set or systematically prepended to the cxx compiler call ?

@ghost
Copy link
Author

ghost commented Jul 13, 2020

Systematically prepended. This flags seems like part of the basic behaviour, i.e. turning the C compiler into a C++ one.

@voodoos
Copy link
Collaborator

voodoos commented Jul 27, 2020 •
edited
Loading

A first survey of the flags that dune could add by itself:

Compiler C++ flags Notes
gcc -x c++ -lstdc++ The G++ driver automatically adds -shared-libgcc whenever you build a shared library or a main executable. Is that something we should do too ?
clang -x c++ -stdlib=<library> Specifies the C++ standard library to use; supported options are libstdc++ and libc++. If not specified, platform default will be used.
msvc /TP The specification of the output target is a bit different: /Fo <file>

Am I missing any major compiler ? Cygwin-gcc / Mingw should accept the same options as gcc.

Also, ocamlc -config sometimes only identifies the c compiler as cc which could be a link to various compilers. (On my mac a link to clang). What should we do in that case ? Resolve the symlink ?

@ghost
Copy link
Author

ghost commented Jul 27, 2020

@dra27 or @avsm do you have an opinion on these questions?

My intuition is:

  • add -shared-libgcc when building shared libraries with gcc
  • don't pass -stdlib=... when using clang

Follow the cc symlink to decide whether it is clang or gcc.

@dra27
Copy link
Member

dra27 commented Jul 27, 2020

A few thoughts:

  • Dune should be opinionated and trigger this for when the source file ends .cpp - i.e. the file name (or the stanza?!) says it's C++, and Dune then does what's necessary to ensure the C compiler is invoked as a C++ compiler. This would make /TP unnecessary for MSVC.
  • -shared-libgcc should be used for C++ yes, or exceptions don't work across shared libraries. Agree for Clang that -stdlib should not be used if there's an expected default. I'm not sure what happens if you use -shared-libgcc -static - i.e. it should be probably be possible to override it (there's some weirdness with this with ocaml-mccs, for example, but I can't right now remember the exact details)
  • Isn't probing the binary more reliable than reading the symlink? (i.e. reading cc --version or equivalent) to determine the compiler flavour. Regardless, if Dune would benefit from knowing the precise flavour of the compiler, then it would be worth an upstream PR for OCaml to include it in -config (OCaml determines it during configure - it's essentially extra information on top of ccomp_type).

@ghost
Copy link
Author

ghost commented Aug 6, 2020

Isn't probing the binary more reliable than reading the symlink? (i.e. reading cc --version or equivalent) to determine the compiler flavour. Regardless, if Dune would benefit from knowing the precise flavour of the compiler, then it would be worth an upstream PR for OCaml to include it in -config (OCaml determines it during configure - it's essentially extra information on top of ccomp_type).

@dra27, we discussed these points with @voodoos yesterday. Regarding changing the configure script, our thinking is that it would be quite a lot more work than we anticipated for this feature, making the ratio work/benefit less worth it. Additionally, we would need a story for older OCaml compilers anyway. So for now we plan to stick to doing this in Dune only, which will be enough for solving the original problem.

Regarding probing binary, agreed that this feels more reliable. At the same time it is a bigger change as well as we need to turn a static value into a dynamic one, and from what we know checking the binary name seems enough. Do you have existing cases in mind where this would break?

@dra27
Copy link
Member

dra27 commented Aug 6, 2020

Reading the symlink might go wrong on Windows (there’s an open issue for absolute symlinks), but it’s only theoretical - I don’t know of any issues with this, indeed

@voodoos voodoos mentioned this issue Sep 18, 2020
Merged
@voodoos voodoos linked a pull request Sep 18, 2020 that will close this issue
Add necessary C++ flags for most common compilers #3802
Merged
@voodoos voodoos mentioned this issue Nov 25, 2020
Merged
@rgrinberg rgrinberg mentioned this issue Jan 13, 2021
Merged
rgrinberg added a commit to rgrinberg/opam-repository that referenced this issue Jan 13, 2021
@rgrinberg
[new release] dune-build-info, dune, dune-configurator, dune-site, du…
dbe0014 
…ne-action-plugin, dune-private-libs and dune-glob (2.8.0)

CHANGES:

- `dune rules` accepts aliases and other non-path rules (ocaml/dune#4063, @mrmr1993)

- Action `(diff reference test_result)` now accept `reference` to be absent and
  in that case consider that the reference is empty. Then running `dune promote`
  will create the reference file. (ocaml/dune#3795, @bobot)

- Ignore special files (BLK, CHR, FIFO, SOCKET), (ocaml/dune#3570, fixes ocaml/dune#3124, ocaml/dune#3546,
  @ejgallego)

- Experimental: Simplify loading of additional files (data or code) at runtime
  in programs by introducing specific installation sites. In particular it allow
  to define plugins to be installed in these sites. (ocaml/dune#3104, ocaml/dune#3794, fixes ocaml/dune#1185,
  @bobot)

- Move all temporary files created by dune to run actions to a single directory
  and make sure that actions executed by dune also use this directory by setting
  `TMPDIR` (or `TEMP` on Windows). (ocaml/dune#3691, fixes ocaml/dune#3422, @rgrinberg)

- Fix bootstrap script with custom configuration. (ocaml/dune#3757, fixes ocaml/dune#3774, @marsam)

- Add the `executable` field to `inline_tests` to customize the compilation
  flags of the test runner executable (ocaml/dune#3747, fixes ocaml/dune#3679, @lubegasimon)

- Add `(enabled_if ...)` to `(copy_files ...)` (ocaml/dune#3756, @nojb)

- Make sure Dune cleans up the status line before exiting (ocaml/dune#3767,
  fixes ocaml/dune#3737, @alan-j-hu)

- Add `{gitlab,bitbucket}` as options for defining project sources with `source`
  stanza `(source (<host> user/repo))` in the `dune-project` file.  (ocaml/dune#3813,
  @rgrinberg)

- Fix generation of `META` and `dune-package` files when some targets (byte,
  native, dynlink) are disabled. Previously, dune would generate all archives
  for regardless of settings. (ocaml/dune#3829, ocaml/dune#4041, @rgrinberg)

- Do not run ocamldep to for single module executables & libraries. The
  dependency graph for such artifacts is trivial (ocaml/dune#3847, @rgrinberg)

- Fix cram tests inside vendored directories not being interpreted correctly.
  (ocaml/dune#3860, fixes ocaml/dune#3843, @rgrinberg)

- Add `package` field to private libraries. This allows such libraries to be
  installed and to be usable by other public libraries in the same project
  (ocaml/dune#3655, fixes ocaml/dune#1017, @rgrinberg)

- Fix the `%{make}` variable on Windows by only checking for a `gmake` binary
  on UNIX-like systems as a unrelated `gmake` binary might exist on Windows.
  (ocaml/dune#3853, @kit-ty-kate)

- Fix `$ dune install` modifying the build directory. This made the build
  directory unusable when `$ sudo dune install` modified permissions. (fix
  ocaml/dune#3857, @rgrinberg)

- Fix handling of aliases given on the command line (using the `@` and `@@`
  syntax) so as to correctly handle relative paths. (ocaml/dune#3874, fixes ocaml/dune#3850, @nojb)

- Allow link time code generation to be used in preprocessing executable. This
  makes it possible to use the build info module inside the preprocessor.
  (ocaml/dune#3848, fix ocaml/dune#3848, @rgrinberg)

- Correctly call `git ls-tree` so unicode files are not quoted, this fixes
  problems with `dune subst` in the presence of unicode files. Fixes ocaml/dune#3219
  (ocaml/dune#3879, @ejgallego)

- `dune subst` now accepts common command-line arguments such as
  `--debug-backtraces` (ocaml/dune#3878, @ejgallego)

- `dune describe` now also includes information about executables in addition to
  that of libraries. (ocaml/dune#3892, ocaml/dune#3895, @nojb)

- instrumentation backends can now receive arguments via `(instrumentation
  (backend <name> <args>))`. (ocaml/dune#3906, ocaml/dune#3932, @nojb)

- Tweak auto-formatting of `dune` files to improve readability. (ocaml/dune#3928, @nojb)

- Add a switch argument to opam when context is not default. (ocaml/dune#3951, @tmattio)

- Avoid pager when running `$ git diff` (ocaml/dune#3912, @AltGr)

- Add `(root_module ..)` field to libraries & executables. This makes it
  possible to use library dependencies shadowed by local modules (ocaml/dune#3825,
  @rgrinberg)

- Allow `(formatting ...)` field in `(env ...)` stanza to set per-directory
  formatting specification. (ocaml/dune#3942, @nojb)

- [coq] In `coq.theory`, `:standard` for the `flags` field now uses the
  flags set in `env` profile flags (ocaml/dune#3931 , @ejgallego @rgrinberg)

- [coq] Add `-q` flag to `:standard` `coqc` flags , fixes ocaml/dune#3924, (ocaml/dune#3931 , @ejgallego)

- Add support for Coq's native compute compilation mode (@ejgallego, ocaml/dune#3210)

- Add a `SUFFIX` directive in `.merlin` files for each dialect with no
  preprocessing, to let merlin know of additional file extensions (ocaml/dune#3977,
  @vouillon)

- Stop promoting `.merlin` files. Write per-stanza Merlin configurations in
  binary form. Add a new subcommand `dune ocaml-merlin` that Merlin can use to
  query the configuration files. The `allow_approximate_merlin` option is now
  useless and deprecated. Dune now conflicts with `merlin < 3.4.0` and
  `ocaml-lsp-server < 1.3.0` (ocaml/dune#3554, @voodoos)

- Configurator: fix a bug introduced in 2.6.0 where the configurator V1 API
  doesn't work at all when used outside of dune. (ocaml/dune#4046, @aalekseyev)

- Fix `libexec` and `libexec-private` variables. In cross-compilation settings,
  they now point to the file in the host context. (ocaml/dune#4058, fixes ocaml/dune#4057,
  @TheLortex)

- When running `$ dune subst`, use project metadata as a fallback when package
  metadata is missing. We also generate a warning when `(name ..)` is missing in
  `dune-project` files to avoid failures in production builds.

- Remove support for passing `-nodynlink` for executables. It was bypassed in
  most cases and not correct in other cases in particular on arm32.
  (ocaml/dune#4085, fixes ocaml/dune#4069, fixes ocaml/dune#2527, @emillon)

- Generate archive rules compatible with 4.12. Dune longer attempt to generate
  an archive file if it's unnecessary (ocaml/dune#3973, fixes ocaml/dune#3766, @rgrinberg)

- Fix generated Merlin configurations when multiple preprocessors are defined
  for different modules in the same folder. (ocaml/dune#4092, fixes ocaml/dune#2596, ocaml/dune#1212 and
  ocaml/dune#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. (ocaml/dune#3875, ocaml/dune#3802, fix ocaml/dune#3718 and ocaml/dune#3528, @voodoos)
@ghost ghost mentioned this issue Nov 18, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@voodoos voodoos

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add necessary C++ flags for most common compilers voodoos/dune
2 participants
@voodoos @dra27