Add a test checking that all the C headers are also compatible with C++ #11557
Conversation
kit-ty-kate
force-pushed
the
cpp-tests
branch
from
de1c4ee to
730f48e
Compare
|
I would also suggest to backport it to |
kit-ty-kate
force-pushed
the
cpp-tests
branch
from
51b768b to
93a3ca8
Compare
kit-ty-kate
force-pushed
the
cpp-tests
branch
2 times, most recently
from
558c109 to
77a8cf8
Compare
|
To resume an offline discussion, backporting the header fixes to 5.0 is fine with me. |
driver/compenv.ml
Outdated
| ProcessInterface name | ||
| else if Filename.check_suffix name ".c" then | ||
| ProcessCFile name | ||
| else if Filename.check_suffix name ".cpp" then |
There was a problem hiding this comment.
There are other common extensions for C++: .cxx, .cc, etc, do we want to support them here?
| else if Filename.check_suffix name ".cpp" then | |
| else if Filename.check_suffix name ".cpp" then |
driver/compenv.ml
Outdated
| let c_object_of_filename name = | ||
| Filename.chop_suffix (Filename.basename name) ".c" ^ Config.ext_obj | ||
| let c_object_of_filename ~ext name = | ||
| Filename.chop_suffix (Filename.basename name) ext ^ Config.ext_obj |
There was a problem hiding this comment.
Could use Filename.remove_extension to avoid having to pass ~ext.
|
Globally: I like the idea of the test but I'm a bit nervous with adding C++ compilation support to ocamlc and ocamlopt. It's always possible to do |
|
Xavier Leroy (2022/09/23 03:06 -0700):
Globally: I like the idea of the test but I'm a bit nervous with
adding C++ compilation support to ocamlc and ocamlopt. It's always
possible to do `gcc -I$(ocamlc -where) -c foo.cpp` to compile a C++
file that refers to OCaml's headers.
But, doing so, you loose the ability to call the C compiler with the
right options. Isn't that a problem / risk?
|
We're not calling a C compiler in this situation, we're calling a C++ compiler that may be the same command as the C compiler. But the right options for a C compilation may not be the right option for a C++ compilation. Just passing the same options as for a C compilation is a hail Mary. Eh, even choosing between |
|
Okay, sorry. I assumed that C++ compilers merely extended the set of
command-line options of their C counterpart, but if that assumption is
wrong, it's indeed not a good idea.
|
3 tests are defined: - raw include - include inside an extern "C" - raw include after #define CAML_INTERNALS
kit-ty-kate
force-pushed
the
cpp-tests
branch
from
77a8cf8 to
f58ad45
Compare
xavierleroy
left a comment
xavierleroy
left a comment
There was a problem hiding this comment.
Thank you for resurrecting this PR, it's a useful test to have.
| @@ -0,0 +1,17 @@ | |||
| #define CAML_INTERNALS | |||
There was a problem hiding this comment.
The CAML_INTERNALS sections of the OCaml 5 header files contain too much C code (as inline functions) to be compilable by a C++ compiler. So, this 3rd test will never work, I'm afraid. Let's just test without CAML_INTERNALS.
| (* TEST | ||
| modules = "cpp_stubs1.c cpp_stubs2.c cpp_stubs3.c"; | ||
| readonly_files = "all-includes.h"; | ||
| flags = "-ccopt -x -ccopt c++ -ccopt -std=c++11"; |
There was a problem hiding this comment.
These flags work with GCC and Clang, but probably not with MSVC. Can the test be turned off for MSVC?
There was a problem hiding this comment.
Yes - leftover from #13476, we have a not-msvc action!
6 participants
C++ projects have been broken quite often due to the C headers not being checked with a C++ compiler.
This PR fixes all problems in the headers as well as add a test checking all headers to be sure they are compatible with C++ in the future.
As you can see in the last commit, this also fixed an issue that made C++ compiler break when using
CAML_INTERNALS. This case can be seen in the wild here: ocaml/opam-repository#22140 (comment)Side note: I've been trying to make this branch work a year and a half ago and never could make it succeed for some reason, but somehow i tried it today again and it works fine with trunk now. I've been trying to be careful when rebasing so hopefully I haven't ported back some old behaviour to the driver somehow.