Fix compatibility with older versions of VS by YexuanXiao · Pull Request #4271 · fmtlib/fmt · GitHub
Skip to content

Fix compatibility with older versions of VS - #4271

Merged
vitaut merged 4 commits into
fmtlib:masterfrom
YexuanXiao:master
Dec 23, 2024
Merged

Fix compatibility with older versions of VS#4271
vitaut merged 4 commits into
fmtlib:masterfrom
YexuanXiao:master

Conversation

@YexuanXiao

Copy link
Copy Markdown
Contributor

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.

Comment thread include/fmt/base.h Outdated
Comment on lines 66 to 72

Copy link
Copy Markdown
Contributor

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.

@vitaut

vitaut commented Dec 21, 2024

Copy link
Copy Markdown
Contributor

Also to clarify: _MSVC_STL_UPDATE is only supported since MSVC 2019, right?

@YexuanXiao

Copy link
Copy Markdown
Contributor Author

Also to clarify: _MSVC_STL_UPDATE is only supported since MSVC 2019, right?

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.

Comment thread include/fmt/base.h Outdated
Comment on lines +49 to +53
#ifdef _MSVC_LANG
# define FMT_CPLUSPLUS _MSVC_LANG
#else
# define FMT_CPLUSPLUS __cplusplus
#endif

Copy link
Copy Markdown
Contributor

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?

@vitaut vitaut left a comment

Copy link
Copy Markdown
Contributor

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.

@YexuanXiao
YexuanXiao requested a review from vitaut December 23, 2024 01:44
@vitaut
vitaut merged commit 86dae01 into fmtlib:master Dec 23, 2024
@vitaut

vitaut commented Dec 23, 2024

Copy link
Copy Markdown
Contributor

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

Successfully merging this pull request may close these issues.

2 participants