From b0ad64743dbfcee25cd03811f37a29e69715a07c Mon Sep 17 00:00:00 2001 From: Shujian Qian Date: Fri, 18 Aug 2023 18:48:12 -0400 Subject: [PATCH 1/5] Modify the condition of SPDLOG_CONSTEXPR_FUNC to match that of fmt fix the issue where constexpr function in spdlog may call non-constexpr function in the bundled fmt because FMT_USE_CONSTEXPR is not defined. --- include/spdlog/common.h | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/include/spdlog/common.h b/include/spdlog/common.h index c46c1531d..2c98789fc 100644 --- a/include/spdlog/common.h +++ b/include/spdlog/common.h @@ -65,15 +65,21 @@ #if defined(_MSC_VER) && (_MSC_VER < 1900) #define SPDLOG_NOEXCEPT _NOEXCEPT #define SPDLOG_CONSTEXPR - #define SPDLOG_CONSTEXPR_FUNC inline #else #define SPDLOG_NOEXCEPT noexcept #define SPDLOG_CONSTEXPR constexpr - #if __cplusplus >= 201402L - #define SPDLOG_CONSTEXPR_FUNC constexpr - #else - #define SPDLOG_CONSTEXPR_FUNC inline - #endif +#endif + +#ifndef __has_feature +# define __has_feature(x) 0 +#endif + +#if (__has_feature(cxx_relaxed_constexpr) || (defined(_MSC_VER) && (_MSC_VER >= 1912)) || \ + (defined(__GNUC__) && __GNUC__ >= 6 && defined(__cplusplus) && __cplusplus >= 201402L)) && \ + !defined(__ICL) && !defined(__INTEL_COMPILER) && !defined(__NVCC__) + #define SPDLOG_CONSTEXPR_FUNC constexpr +#else + #define SPDLOG_CONSTEXPR_FUNC inline #endif #if defined(__GNUC__) || defined(__clang__) From 23c69f8e841fc2b5adcfa004e34f2c5e88586632 Mon Sep 17 00:00:00 2001 From: Keith Kraus Date: Wed, 11 Oct 2023 16:17:06 -0400 Subject: [PATCH 2/5] add SPDLOG_HAS_FEATURE macro to avoid redefining built-in, add comment explaining the constexpr check --- include/spdlog/common.h | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/include/spdlog/common.h b/include/spdlog/common.h index 2c98789fc..2a32c4c04 100644 --- a/include/spdlog/common.h +++ b/include/spdlog/common.h @@ -70,11 +70,19 @@ #define SPDLOG_CONSTEXPR constexpr #endif -#ifndef __has_feature -# define __has_feature(x) 0 +#ifdef __has_feature +# define SPDLOG_HAS_FEATURE(x) __has_feature(x) +#else +# define SPDLOG_HAS_FEATURE(x) 0 #endif -#if (__has_feature(cxx_relaxed_constexpr) || (defined(_MSC_VER) && (_MSC_VER >= 1912)) || \ +// Check if relaxed C++14 constexpr is supported +// GCC doesn't allow throw in constexpr until version 6 (bug 67371) +// This needs to stay synchronized with FMT_CONSTEXPR otherwise we can run into +// situations where spdlog constexpr functions can try to call non-constexpr +// fmt functions +// See https://github.com/gabime/spdlog/issues/2856 where this happens with nvcc +#if (SPDLOG_HAS_FEATURE(cxx_relaxed_constexpr) || (defined(_MSC_VER) && (_MSC_VER >= 1912)) || \ (defined(__GNUC__) && __GNUC__ >= 6 && defined(__cplusplus) && __cplusplus >= 201402L)) && \ !defined(__ICL) && !defined(__INTEL_COMPILER) && !defined(__NVCC__) #define SPDLOG_CONSTEXPR_FUNC constexpr From 2421b09c19da94936e81d13717693abaf370fb33 Mon Sep 17 00:00:00 2001 From: Keith Kraus Date: Wed, 11 Oct 2023 18:35:43 -0400 Subject: [PATCH 3/5] Update implementation to reuse FMT_CONSTEXPR where possible --- include/spdlog/common.h | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/include/spdlog/common.h b/include/spdlog/common.h index 2a32c4c04..1269c14af 100644 --- a/include/spdlog/common.h +++ b/include/spdlog/common.h @@ -70,24 +70,19 @@ #define SPDLOG_CONSTEXPR constexpr #endif -#ifdef __has_feature -# define SPDLOG_HAS_FEATURE(x) __has_feature(x) -#else -# define SPDLOG_HAS_FEATURE(x) 0 -#endif - -// Check if relaxed C++14 constexpr is supported -// GCC doesn't allow throw in constexpr until version 6 (bug 67371) -// This needs to stay synchronized with FMT_CONSTEXPR otherwise we can run into -// situations where spdlog constexpr functions can try to call non-constexpr -// fmt functions -// See https://github.com/gabime/spdlog/issues/2856 where this happens with nvcc -#if (SPDLOG_HAS_FEATURE(cxx_relaxed_constexpr) || (defined(_MSC_VER) && (_MSC_VER >= 1912)) || \ - (defined(__GNUC__) && __GNUC__ >= 6 && defined(__cplusplus) && __cplusplus >= 201402L)) && \ - !defined(__ICL) && !defined(__INTEL_COMPILER) && !defined(__NVCC__) +// If building with std::format, can just use constexpr, otherwise if building with fmt +// SPDLOG_CONSTEXPR_FUNC needs to be set the same as FMT_CONSTEXPR to avoid situations where +// a constexpr function in spdlog could end up calling a non-constexpr function in fmt +// depending on the compiler +// If fmt determines it can't use constexpr, we should inline the function instead +#ifdef SPDLOG_USE_STD_FORMAT #define SPDLOG_CONSTEXPR_FUNC constexpr -#else - #define SPDLOG_CONSTEXPR_FUNC inline +#else // Being built with fmt + #if FMT_USE_CONSTEXPR + #define SPDLOG_CONSTEXPR_FUNC FMT_CONSTEXPR + #else + #define SPDLOG_CONSTEXPR_FUNC inline + #endif #endif #if defined(__GNUC__) || defined(__clang__) From 1897f7025d4f2dc9ea5e2ac970ceb08ed993b2cc Mon Sep 17 00:00:00 2001 From: Keith Kraus Date: Thu, 12 Oct 2023 00:10:44 -0400 Subject: [PATCH 4/5] don't need to inline, simplify to always use FMT_CONSTEXPR if it's defined --- include/spdlog/common.h | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/include/spdlog/common.h b/include/spdlog/common.h index 1269c14af..9dfdf6d69 100644 --- a/include/spdlog/common.h +++ b/include/spdlog/common.h @@ -70,19 +70,13 @@ #define SPDLOG_CONSTEXPR constexpr #endif -// If building with std::format, can just use constexpr, otherwise if building with fmt -// SPDLOG_CONSTEXPR_FUNC needs to be set the same as FMT_CONSTEXPR to avoid situations where -// a constexpr function in spdlog could end up calling a non-constexpr function in fmt -// depending on the compiler -// If fmt determines it can't use constexpr, we should inline the function instead -#ifdef SPDLOG_USE_STD_FORMAT +// If building with fmt SPDLOG_CONSTEXPR_FUNC needs to be set the same as FMT_CONSTEXPR +// to avoid situations where a constexpr function in spdlog could end up calling +// a non-constexpr function in fmt depending on the compiler +#ifdef FMT_CONSTEXPR + #define SPDLOG_CONSTEXPR_FUNC FMT_CONSTEXPR +#else #define SPDLOG_CONSTEXPR_FUNC constexpr -#else // Being built with fmt - #if FMT_USE_CONSTEXPR - #define SPDLOG_CONSTEXPR_FUNC FMT_CONSTEXPR - #else - #define SPDLOG_CONSTEXPR_FUNC inline - #endif #endif #if defined(__GNUC__) || defined(__clang__) From fab780c196ef5ff9d5ae48e2bb482b6f8d812ab2 Mon Sep 17 00:00:00 2001 From: Keith Kraus Date: Thu, 12 Oct 2023 12:58:05 -0400 Subject: [PATCH 5/5] Revert "don't need to inline, simplify to always use FMT_CONSTEXPR if it's defined" This reverts commit 1897f7025d4f2dc9ea5e2ac970ceb08ed993b2cc. --- include/spdlog/common.h | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/include/spdlog/common.h b/include/spdlog/common.h index 9dfdf6d69..1269c14af 100644 --- a/include/spdlog/common.h +++ b/include/spdlog/common.h @@ -70,13 +70,19 @@ #define SPDLOG_CONSTEXPR constexpr #endif -// If building with fmt SPDLOG_CONSTEXPR_FUNC needs to be set the same as FMT_CONSTEXPR -// to avoid situations where a constexpr function in spdlog could end up calling -// a non-constexpr function in fmt depending on the compiler -#ifdef FMT_CONSTEXPR - #define SPDLOG_CONSTEXPR_FUNC FMT_CONSTEXPR -#else +// If building with std::format, can just use constexpr, otherwise if building with fmt +// SPDLOG_CONSTEXPR_FUNC needs to be set the same as FMT_CONSTEXPR to avoid situations where +// a constexpr function in spdlog could end up calling a non-constexpr function in fmt +// depending on the compiler +// If fmt determines it can't use constexpr, we should inline the function instead +#ifdef SPDLOG_USE_STD_FORMAT #define SPDLOG_CONSTEXPR_FUNC constexpr +#else // Being built with fmt + #if FMT_USE_CONSTEXPR + #define SPDLOG_CONSTEXPR_FUNC FMT_CONSTEXPR + #else + #define SPDLOG_CONSTEXPR_FUNC inline + #endif #endif #if defined(__GNUC__) || defined(__clang__)