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

Workaround for incompatibility between libstdc++ consteval-based std::is_constant_evaluated() implementation and clang-14 #3281

Merged
merged 1 commit into from
Jan 22, 2023

Conversation

phprus
Copy link
Contributor

@phprus phprus commented Jan 21, 2023

Fix for issue #3247

cc @m5k8

Copy link
Contributor

@vitaut vitaut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.

@@ -329,7 +329,14 @@ template <typename... T> FMT_CONSTEXPR void ignore_unused(const T&...) {}

constexpr FMT_INLINE auto is_constant_evaluated(
bool default_value = false) noexcept -> bool {
#ifdef __cpp_lib_is_constant_evaluated
// Workaround for incompatible between libstdc++ consteval based
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

incompatible -> incompatibility
consteval based -> consteval-based

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

// std::is_constant_evaluated implementation and clang-14+.
// https://github.com/fmtlib/fmt/issues/3247
#if FMT_CPLUSPLUS >= 202002L && defined(_GLIBCXX_RELEASE) && \
_GLIBCXX_RELEASE >= 12 && FMT_CLANG_VERSION >= 14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be FMT_CLANG_VERSION == 14 per #3247 (comment)?

Copy link
Contributor Author

@phprus phprus Jan 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be

(FMT_CLANG_VERSION >= 1400 && FMT_CLANG_VERSION < 1500)

This is my mistake, I forgot about the minor version part (00):

# define FMT_CLANG_VERSION (__clang_major__ * 100 + __clang_minor__)

Since bug is fixed in clang-15, I added a second condition.

…:is_constant_evaluated() implementation and clang-14

Signed-off-by: Vladislav Shchapov <[email protected]>
@phprus phprus changed the title Workaround for incompatible between libstdc++ consteval based std::is_constant_evaluated implementation and clang-14+ Workaround for incompatibility between libstdc++ consteval-based std::is_constant_evaluated() implementation and clang-14 Jan 22, 2023
@phprus phprus requested a review from vitaut January 22, 2023 14:28
@vitaut vitaut merged commit 9409b2e into fmtlib:master Jan 22, 2023
@vitaut
Copy link
Contributor

vitaut commented Jan 22, 2023

Thank you!

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.

2 participants