-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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
[catch2] Set CMAKE_CXX_STANDARD to 17. #25019
Conversation
microsoft#24762 (comment) indicates that building for C++17 will include extra catch2 formatters for types like std::string_view, so we should build catch2 with that version selected.
fix-install-path.patch | ||
) | ||
|
||
vcpkg_cmake_configure( | ||
SOURCE_PATH "${SOURCE_PATH}" | ||
OPTIONS | ||
-DCATCH_INSTALL_DOCS=OFF | ||
-DCMAKE_CXX_STANDARD=17 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a toolchain setting not a port setting.....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree that it's a toolchain setting in general since it usually isn't ABI affecting. There's no reason everything in the tree needs to agree on the same answer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BillyONeal, should we add a feature to catch2 instead? like abseil, https://github.com/microsoft/vcpkg/blob/master/docs/maintainers/maintainer-guide.md#a-feature-may-replace-polyfills-with-aliases-provided-that-replacement-is-baked-into-the-installed-tree
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PhoebeHui No: abseil's feature controls whether it provides a polyfill. That is, absl::string_view
will be std::string_view
or absl::string_view
depending on that feature. In contrast, this merely enables or disables functionality: it allows CHECK(some_string_view == "a literal")
to compile and link.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, thanks for your clarification.
Vcpkg problem has since been fixed upstream, cf. microsoft/vcpkg#25019 .
#24762 (comment) indicates that building for C++17 will include extra catch2 formatters for types like std::string_view, so we should build catch2 with that version selected.