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

Building spdlog with Visual Studio 17.8 fails #2912

Closed
jaimecbernardo opened this issue Oct 19, 2023 · 9 comments
Closed

Building spdlog with Visual Studio 17.8 fails #2912

jaimecbernardo opened this issue Oct 19, 2023 · 9 comments

Comments

@jaimecbernardo
Copy link

Hi,

Currently still on Preview 4, Visual Studio 17.8 C++ STL deprecates the use of checked_array_iterator with this change: microsoft/STL#3818

spdlog uses it here:

// Make a checked iterator to avoid MSVC warnings.
template <typename T> using checked_ptr = stdext::checked_array_iterator<T*>;

This causes a build error due to the deprecation if warnings are treated as errors.

On PowerToys we use spdlog and are getting around this by adding the _SILENCE_STDEXT_ARR_ITERS_DEPRECATION_WARNING definition, but this stdext::checked_array_iterator feature is expected to be removed entirely in the future.
https://github.com/microsoft/PowerToys/pull/29303/files#diff-db47b6c13dd626082ddffbfd3e6d5c7087bfbd39e92c99f4b28a1561a1f19593

From my understanding, this is used in spdlog to work around one warning. With the changes in STL is this still needed?
If this code just comes from the fmt liv, I believe it's been dealt with already in fmtlib/fmt#3540 and microsoft/vcpkg#32652 .

Thanks, in advance.

@tt4g
Copy link
Contributor

tt4g commented Oct 19, 2023

Yes, spdlog/include/spdlog/fmt/bundled/format.h copied from fmt repository.
You can external fmt library with CMake variable SPDLOG_FMT_EXTERNAL=ON.

spdlog/CMakeLists.txt

Lines 91 to 92 in ff205fd

option(SPDLOG_FMT_EXTERNAL "Use external fmt library instead of bundled" OFF)
option(SPDLOG_FMT_EXTERNAL_HO "Use external fmt header-only library instead of bundled" OFF)

@MiKom
Copy link

MiKom commented Jan 12, 2024

Hey.

Using the bundled fmt library is very useful as using the own one requires additional setup. Is there some showstopper for including newer fmt? Or is it more a matter of no one yet getting to it? If it's the latter I may have some time to take a look at it.

@tt4g
Copy link
Contributor

tt4g commented Jan 12, 2024

@MiKom Use external fmt if you want to use new features of fmt or if another library depends on a different version of fmt than spdlog.

@gabime
Copy link
Owner

gabime commented Jan 12, 2024

Spdlog ver 1.x cant bundle new fmt versions without breaking backward compatibility. spdlog ver 2.x will use the latest fmt version soon. As @tt4g said, you can use easily different fmt version using the right macro.

@MiKom
Copy link

MiKom commented Jan 12, 2024

@MiKom Use external fmt if you want to use new features of fmt or if another library depends on a different version of fmt than spdlog.

Yes, I know that. My question was different though. I was wondering if there's some showstopper for inclusion of the new fmt. And as @gabime pointed out, indeed there is for spdlog 1.x.

Now I understand the situation fully and can act accordingly. Thanks @gabime

@gabime gabime closed this as completed Jan 12, 2024
@MiKom
Copy link

MiKom commented Jan 12, 2024

@tt4g

Sadly spdlog 1.x is not compatible with fmt 10.1.0 where it was fixed so using custom fmt won't help here.

Until spdlog 2.x is out, it's necessary to use define _SILENCE_STDEXT_ARR_ITERS_DEPRECATION_WARNING for recent Visual Studio MSVC.

This comment was incorrect. spdlog is compatible with fmt 10.x, you needs to use spdlog 1.12.0 or later though.

@tt4g
Copy link
Contributor

tt4g commented Jan 12, 2024

@MiKom _SILENCE_STDEXT_ARR_ITERS_DEPRECATION_WARNING macro is referenced by MSVC, so I believe it can be defined regardless of the fmt and spdlog versions.

@gabime
Copy link
Owner

gabime commented Jan 12, 2024

Sadly spdlog 1.x is not compatible with fmt 10.1.0

@MiKom Actually it is compatible with fmt 10.x. It just doesn't bundle it.

@MiKom
Copy link

MiKom commented Jan 15, 2024

@gabime You're right of course. My mistake was using spdlog 1.11.0 whereas the fix for fmt 10.x was done in 1.12.0. Thanks!

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

No branches or pull requests

4 participants