-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Fix compatibility with older versions of VS #4271
Conversation
include/fmt/base.h
Outdated
#ifdef _MSVC_STL_UPDATE | ||
# define FMT_MSVC_STL_UPDATE _MSVC_STL_UPDATE | ||
#elif defined(_MSC_VER) && _MSC_VER < 1912 // VS 15.5 | ||
# define FMT_MSVC_STL_UPDATE _MSVC_LANG | ||
#else | ||
# define FMT_CPLUSPLUS __cplusplus | ||
# define FMT_MSVC_STL_UPDATE 0 | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move this to ostream.h
where it is used.
Also to clarify: |
To be precise, it has been supported since MSVC 19.12. Therefore, for versions from 19.00 to 19.12, use _MSVC_LANG instead. fmt has already declared that it does not support MSVC before version 19.00. Here are two version reference tables: _MSC_VER and _MSVC_STL_UPDATE. |
include/fmt/base.h
Outdated
#ifdef _MSVC_LANG | ||
# define FMT_CPLUSPLUS _MSVC_LANG | ||
#else | ||
# define FMT_CPLUSPLUS __cplusplus | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also revert this unnecessary change to base.h
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just one more minor comment, otherwise LGTM.
Merged, thanks. |
In the previous PR, I mentioned that there were no compatibility issues, but I was wrong. I confused the versions of VS and MSVC. _MSVC_STL_UPDATE appeared later than the VS version that fmt promised to support. This PR will fix this issue.