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.

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

Only set C++ standard if not defined #145

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gummif
Copy link

@gummif gummif commented Dec 20, 2024

This allows us use add_subdirectory(avcpp) without overwriting the set standard (C++23 e.g.)

This allows us use `add_subdirectory(avcpp)` without overwriting the set standard (C++23 e.g.)
@KKhanhH
Copy link

KKhanhH commented Jan 15, 2025

Rather than setting the C++ standard of the project, it might be better to simply best to set the minimum standard version for a specific target rather than globally, so target_compile_features(avcpp PUBLIC cxx_std_17)

@h4tr3d
Copy link
Owner

h4tr3d commented Jan 15, 2025

@KKhanhH , currently WIP: 251d97e

I still need support C++17 for now due to support for Ubuntu 20.04 with default compiler. But I wont use new standard features too, when possible.

And I am not sure about next scenario:

  1. set avcpp compiler feature to cxx_std_17
  2. export it via cmake targets exporting as a part of the public interface
  3. link it to the target...
  4. point another, for example, cxx_std_23 for for target...

I added example target, that compiles with cxx_std_20 and only if compiler support it. But it is not checked yet completely:

set_property(TARGET api2-decode-raw-h264 PROPERTY CXX_STANDARD 20)

Also, I am not sure, if it good solution, when headers make some C++ standard checks when binaries already build with some another options. Sounds like, that CXX standard for the whole library and headers must be fixed via config.h.in to make headers and binaries consistent.

This PR will be obviously rejected in current form, but only when work above will be done.

@KKhanhH
Copy link

KKhanhH commented Jan 15, 2025

@h4tr3d Would it not be possible to use the __cplusplus macro to check the current standard used by the compiler? I think that would be better supported and wouldn't require a separate define, although you might need to set the /Zc:__cplusplus flag for MSVC. Also I believe if you set the target_compile_features to a specific standard and the parent project has a higher standard set, the higher one will be picked.

If you mean to match the headers and the compiled binary standard version, that should be handled by the IDE. If the user uses CMAKE_EXPORT_COMPILE_COMMANDS to generate a compile_commands.json then the IDE should be able to detect what version is being used and what defines to set.

@h4tr3d
Copy link
Owner

h4tr3d commented Jan 15, 2025

Would it not be possible to use the __cplusplus macro to check the current standard used by the compiler?

Obvious solution. But if some ABI incompatibility will be ocurred. For example: some method available only when library built with C++20. But it was built with C++17 and later header used in the application with C++20 enabled: macro check passed but function implementation does not present.

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