Audit the installed headers for C++ compatibility #13591
Conversation
NickBarnes
left a comment
NickBarnes
left a comment
There was a problem hiding this comment.
Looks good to me; consistency is a good thing.
MisterDA
force-pushed
the
headers-cpp-extern-c-compat
branch
3 times, most recently
from
96a7d40 to
0a67d38
Compare
C++ name mangling applies to symbols: variable and function names. The
rule of thumb is to enclose public symbols declarations in blocks:
#ifdef __cplusplus
extern "C" {
#endif
/* symbols go here */
#ifdef __cplusplus
}
#endif
Symbols protected by CAML_INTERNALS blocks need not to be covered.
Headers that contain definitions incompatible with C++, such as
_Atomic, also need protection.
MisterDA
force-pushed
the
headers-cpp-extern-c-compat
branch
from
0a67d38 to
9ba790d
Compare
|
I've added a test that checks whether C++ stubs can be compiled and linked. I think it replaces and should close #11557. Thanks @kit-ty-kate for the tip! This should be considered for 5.3 and 5.2. |
- Test whether OCaml C headers are also valid in C++;
- Test whether C++ files can be linked with the runtime. Symbols that
are not covered by CAML_INTERNALS need to have C linkage (under
extern "C" { ... }). There are too many to check exhaustively, so
use a dummy stub for now.
Co-authored-by: Kate <kit-ty-kate@outlook.com>
MisterDA
force-pushed
the
headers-cpp-extern-c-compat
branch
from
9ba790d to
1addeb9
Compare
|
Mangling of variable names only matters when namespaces are used, which is obviously not the case in OCaml. Therefore several of the added guards are unnecessary, e.g. |
Is that a strong blocker? Can we keep them for future use, or should I remove them? |
|
No, it's not a blocker. |
|
@NickBarnes , did you have the time to finish your review? If possible I would like to include this fix in the next beta for 5.3 (that should happen next week hopefully). |
|
I'm tempted to add the C++ guards to all headers, unconditionally, even encompassing |
NickBarnes
left a comment
NickBarnes
left a comment
There was a problem hiding this comment.
LGTM. Having a test is great. As noted in your comment, it would be nice if the test's #includes were auto-generated, but that's a problem for another time.
|
@Octachron I apologise for the delay. |
Audit the installed headers for C++ compatibility (cherry picked from commit b0b5f92)
|
Merged and cherry-picked to 5.3. |
|
I agree that cherry-picking 38b9c9e to 5.3 is a good idea too to avoid diverging ocamltest tests. |
|
Done under @dra27's close supervision .... |
4 participants
C++ name mangling applies to symbols: variable and function names. The rule of thumb is to enclose public symbols declarations in blocks, to avoid C++ linkers look for C++ symbol names instead of C.
Symbols protected by
CAML_INTERNALSblocks need not to be covered. I've sometimes reordered a few functions to merge two sections protected byCAML_INTERNALS.Headers that contain definitions incompatible with C++, such as
_Atomic, also need protection.There shouldn't be anything but comments before header guards to allow for multiple include optimization.
Fix #13541. Close #11557.