Skip to content

Regression fix: Allow to include caml/custom.h in C++ code - #13895

Merged
gasche merged 1 commit into
ocaml:trunkfrom
kit-ty-kate:custom_h-cpp
Mar 25, 2025
Merged

gasche merged 1 commit into
ocaml:trunkfrom
kit-ty-kate:custom_h-cpp

Conversation

@kit-ty-kate

Copy link
Copy Markdown
Member

This is a regression fix for trunk compared to 5.3.0.
Using trunk, including caml/custom.h will result in the following error when using g++:

# /home/kit_ty_kate/.opam/5.4/lib/ocaml/caml/custom.h:54:12: error: ‘_Atomic’ does not name a type; did you mean ‘Atom’?
#    54 | CAMLextern _Atomic uintnat caml_custom_major_ratio;
#       |            ^~~~~~~
#       |            Atom

Related to #13777

@gasche

gasche commented Mar 25, 2025

Copy link
Copy Markdown
Member

Your proposed fix removes the variable instead of formulating it in a C++-compatible way. I'm not convinced that this is the right fix. The discussions in #13777 suggest that atomic_uintnat should be used instead of Atomic_ uintnat. Would doing that restore C++ compatibility?

@MisterDA

Copy link
Copy Markdown
Contributor

Sorry that my work on the previous PR has stalled.

Would doing that restore C++ compatibility?

Yes, it would. This commit could be cherry-picked: 6537fe8.

_Atomic is standard C11 but is not standard C++ (although sometimes
available as a compiler extension). Use atomic_uintnat from our
camlatomic.h.

    In file included from all-includes.h:6:
    /Users/antonin/Tarides/ocaml/trunk/runtime/caml/custom.h:54:12: error: '_Atomic' does not name a type; did you mean 'Atom'?
       54 | CAMLextern _Atomic uintnat caml_custom_major_ratio;
          |            ^~~~~~~
          |            Atom
@kit-ty-kate

Copy link
Copy Markdown
Member Author

Done

@gasche
gasche merged commit f234d80 into ocaml:trunk Mar 25, 2025
@gasche

gasche commented Mar 25, 2025

Copy link
Copy Markdown
Member

Merged, thank you both!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants