Modify the condition of SPDLOG_CONSTEXPR_FUNC to match that of fmt

[ShujianQian]

• fix the issue where constexpr function in spdlog may call non-constexpr function in the bundled fmt because FMT_USE_CONSTEXPR is not defined

fix #2856

[gabime]

Thanks, however thats way too much additions and new macros for this simple fix.

[ShujianQian]

I was hoping to match the condition in fmt for ease of maintenance in the future. Would you prefer a simpler condition for the SPDLOG_CONSTEXPR_FUNC macro or marking the affected function as inline instead?

[gabime]

Since this is nvcc specific, simple ifdef nvcc is sufficient.

[ShujianQian]

I believe this will affect other compilers too. The compile time error will happen whenever SPDLOG_CONSTEXPR_FUNC is defined but FMT_USE_CONSTEXPR is not.

The FMT_USE_CONSTEXPR condition in the bundled fmt is the following:

#  if (FMT_HAS_FEATURE(cxx_relaxed_constexpr) || FMT_MSC_VERSION >= 1912 || \
       (FMT_GCC_VERSION >= 600 && FMT_CPLUSPLUS >= 201402L)) &&             \
      !FMT_ICC_VERSION && !defined(__NVCC__)

The issue is also very visible because it happens on any kind of logging as simple as spdlog::info("text");.

If you believe ifdef nvcc is sufficient, I can compose another PR.

[gabime]

@ShujianQian I believe this will affect other compilers too

So how come it is not a common problem and wasn't reported many times so far?

[ShujianQian]

I guess the condition is rather hard to meet and some compilers do not raise an error in some cases. However, I'm able to verify that the same issue happens on gcc-5 with the following source file:

#include "spdlog/spdlog.h"

int main(int argc, char **argv)
{
        spdlog::info("info");
        return 0;
}

> gcc-5 --version
gcc-5 (Homebrew GCC 5.5.0_8) 5.5.0
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

> gcc-5 ./test_spdlog.cpp -I ./spdlog/include -o test_spdlog -std=c++14
In file included from ./spdlog/include/spdlog/spdlog.h:12:0,
                 from ./test_spdlog.cpp:1:
./spdlog/include/spdlog/common.h: In function 'constexpr spdlog::string_view_t spdlog::details::to_string_view(const memory_buf_t&)':
./spdlog/include/spdlog/common.h:351:42: error: call to non-constexpr function 'const T* fmt::v9::detail::buffer<T>::data() const [with T = char]'
     return spdlog::string_view_t{buf.data(), buf.size()};
                                          ^

Some versions of ICC (which is another corner case in the FMT_CONSTEXPR condition) allows constexpr functions to call non-constexpr functions (as shown in this example). I have verified that spdlog compiles with the latest ICC.

I'm not able to test on MSVC.

[gabime]

I don't mimd a minimal fix, with minimal or no new macros. sodlog defines too many macros already imo.