[MSVC][std:c++latest] LightGBM failed to build with /std:c++latest on MSVC

[xiaoxiao-Xu]

Description

Hi all,
Recently, we found some errors when building LightGBM with '/std:c++latest'. Could someone please take a first look? Thanks in advance.

Reproducible example

1. git clone https://github.com/microsoft/LightGBM F:\gitP\microsoft\LightGBM
2. git -C "F:\gitP\microsoft\LightGBM" submodule sync
3. git -C "F:\gitP\microsoft\LightGBM" submodule foreach git reset --hard
4. git -C "F:\gitP\microsoft\LightGBM" submodule foreach git clean -xdf
5. git -C "F:\gitP\microsoft\LightGBM" submodule update --init --recursive
6. cd F:\gitP\microsoft\LightGBM & mkdir build_amd64
7. set CL= /std:c++latest
8. cd F:\gitP\microsoft\LightGBM\build_amd64
9. cmake -G "Visual Studio 16 2019" -A x64 -DCMAKE_SYSTEM_VERSION=10.0.18362.0 -DCMAKE_BUILD_TYPE=Release -DBUILD_CPP_TEST=ON ..
10. msbuild /m /p:Platform=x64 /p:Configuration=Release lightgbm.sln /t:Rebuild

Environment info

LightGBM version or commit hash: 7e47804
Visual Studio Version: Release 16.11.12
Platform: Windows

Additional Comments

build.log
Actual Behavior:
F:\gitP\microsoft\LightGBM\include\LightGBM/utils/common.h(1209,53): error C7595: 'fmt::v8::basic_format_string<char,const T &>::basic_format_string': call to immediate function is not a constant expression
F:\gitP\microsoft\LightGBM\include\LightGBM/utils/common.h(1209,53): error C7595: 'fmt::v8::basic_format_string<char,const T &>::basic_format_string': call to immediate function is not a constant expression
F:\gitP\microsoft\LightGBM\include\LightGBM/utils/common.h(1209,53): error C7595: 'fmt::v8::basic_format_string<char,const T &>::basic_format_string': call to immediate function is not a constant expression
F:\gitP\microsoft\LightGBM\include\LightGBM/utils/common.h(1209,53): error C7595: 'fmt::v8::basic_format_string<char,const T &>::basic_format_string': call to immediate function is not a constant expression

[jmoralez]

Hi @xiaoxiao-Xu, thank you for the report and for the example. I believe you may be missing the submodules, you need to use the --recursive flag when cloning the project, i.e. step 1 should be git clone --recursive https://github.com/microsoft/LightGBM F:\gitP\microsoft\LightGBM. Can you try it that way and see if that helps?

[xiaoxiao-Xu]

@jmoralez I'm so sorry that I didn't attach the complete build steps. Actually, I have updated submodules when cloning the project and I have updatd my build steps as above. So, this could not help for the issue.

[jameslamb]

LightGBM uses the C++11 standard. Could you explain why it's important for your use case to use c++latest instead?

[xiaoxiao-Xu]

Ok. Because we are testing the msvc compiler under the behavior of '/std:c++latest'.

[jameslamb]

Oh I see. The specific errors you've shared come from fmt, a project that LightGBM vendors in as a git submodule. You might try visiting https://github.com/fmtlib/fmt if you'd like to understand whether what you're seeing is expected.

But LightGBM uses the C++11 standard and I believe it is not expected that users will try to compile it targeting other standards.

@guolinke or @StrikerRUS please correct me if I'm wrong, I'm not totally confident in my understandong of this topic.

[xiaoxiao-Xu]

Ok. What you mean is LightGBM doesn't support under '/std:c++latest', is it?

[jameslamb]

LightGBM doesn't support under /std:c++latest

I think that's correct. I don't believe any of LightGBM's continuous integration jobs test compiling with MSVC using /std:c++latest.

Some evidence that the project follows the C++11 standard:

1. Using -std=c++11 for all compilers other than MSVC

LightGBM/CMakeLists.txt

Lines 302 to 305 in 6105ba9

	if(UNIX OR MINGW OR CYGWIN)
	set(
	CMAKE_CXX_FLAGS
	"${CMAKE_CXX_FLAGS} -std=c++11 -pthread -Wextra -Wall -Wno-ignored-attributes -Wno-unknown-pragmas -Wno-return-type"

2. R package using C++11

LightGBM/R-package/src/Makevars.in

Line 1 in 6105ba9

CXX_STD = CXX11

[xiaoxiao-Xu]

Fine. I see. Thanks for your attention.

[StrikerRUS]

build.log

Looks like these logs are not related to the compilation of LightGBM.

Also, you are building cpp tests according to

cmake -G "Visual Studio 16 2019" -A x64 -DCMAKE_SYSTEM_VERSION=10.0.18362.0 -DCMAKE_BUILD_TYPE=Release -DBUILD_CPP_TEST=ON ..

Is it possible to check without -DBUILD_CPP_TEST=ON CMake flag?

[xiaoxiao-Xu]

Sorry. It doesn't work.

[guolinke]

@shiyu1994 do you have a windows machine to test?
I think our CI should covert the msvc build, and it shouldn't fail.

[QuellaZhang]

@guolinke @StrikerRUS The MSVC developer provided a local patch(as below) for this issue, can you help check in? Thanks!

The third argument to fmt::format_to_n() cannot be a function parameter (see LightGBM/include/LightGBM/utils/common.h).

template <typename T>
inline static void format_to_buf(char* buffer, const size_t buf_len, const char* format, const T value) {
    auto result = fmt::format_to_n(buffer, buf_len, format, value);
    if (result.size >= buf_len) {
      Log::Fatal("Numerical conversion failed. Buffer is too small.");
    }
    buffer[result.size] = '\0';
}

template<typename T, bool is_float, bool high_precision>
struct __TToStringHelper {
  void operator()(T value, char* buffer, size_t buf_len) const {
    format_to_buf(buffer, buf_len, "{}", value);
  }
};

template<typename T>
struct __TToStringHelper<T, true, false> {
  void operator()(T value, char* buffer, size_t buf_len) const {
    format_to_buf(buffer, buf_len, "{:g}", value);
  }
};

template<typename T>
struct __TToStringHelper<T, true, true> {
  void operator()(T value, char* buffer, size_t buf_len) const {
    format_to_buf(buffer, buf_len, "{:.17g}", value);
  }
};

To avoid this issue, you can replace the above (and get rid of LightGBM::CommonC::format_to_buf()) with the following:

template<typename T, bool is_float, bool high_precision>
struct __TToStringHelper {
  void operator()(T value, char* buffer, size_t buf_len) const {
    auto result = fmt::format_to_n(buffer, buf_len, "{}", value);
    if (result.size >= buf_len) {
      Log::Fatal("Numerical conversion failed. Buffer is too small.");
    }
    buffer[result.size] = '\0';
  }
};

template<typename T>
struct __TToStringHelper<T, true, false> {
  void operator()(T value, char* buffer, size_t buf_len) const {
    auto result = fmt::format_to_n(buffer, buf_len, "{:g}", value);
    if (result.size >= buf_len) {
      Log::Fatal("Numerical conversion failed. Buffer is too small.");
    }
    buffer[result.size] = '\0';
  }
};

template<typename T>
struct __TToStringHelper<T, true, true> {
  void operator()(T value, char* buffer, size_t buf_len) const {
    auto result = fmt::format_to_n(buffer, buf_len, "{:.17g}", value);
    if (result.size >= buf_len) {
      Log::Fatal("Numerical conversion failed. Buffer is too small.");
    }
    buffer[result.size] = '\0';
  }
};

lightgbm_error_c7595_OSSOptionCppLatest.patch.txt

[StrikerRUS]

@QuellaZhang Thanks for your comment! Would you like to create a PR with the proposed solution?

[QuellaZhang]

@StrikerRUS fix by #5500.