Skip to content

Conversation

@ChristophStrehle
Copy link

Fixed issue: #1311

Fix a compile break when building with the Microsoft STL and a compiler
other then MSVC. For example when building with clang-cl
@irwir
Copy link
Contributor

irwir commented Jun 12, 2025

Is it necessary to define STL version?
If not, the changes could be as short as:

#if defined(_MSVC_STL_VERSION) // VS2017 (14.1) and above
# define CRYPTOPP_MSSTL
#elif defined(_CPPLIB_VER) && _CPPLIB_VER >= 503 // VS2008 (also 9.0)
# define CRYPTOPP_MSSTL
#endif

and
#ifdef CRYPTOPP_MSSTL

@ChristophStrehle
Copy link
Author

Hello irwir,

first of all sorry for the late reply. To be honest I don't know if this would be enough but maybe I can explain better how I came up with this.

This check was introduced with this commit in the following form:

#if _MSC_VER >= 1500

and according to the Microsoft documentation 1500 means Visual Studio 2008 (9.0).

For Visual Studio 2008 the corresponding STL version can be detected with #if defined(_CPPLIB_VER) && _CPPLIB_VER >= 503. See for example the dinkumware.hpp header file in the boost config.

As this code is not needed to fix a compile break with the MSVC compiler but with the Microsoft STL I changed the check to the corresponding STL version instead of the MSVC compiler. And I added all the other versions for completeness.

@irwir
Copy link
Contributor

irwir commented Jun 29, 2025

As this code is not needed to fix a compile break with the MSVC compiler but with the Microsoft STL I changed the check to the corresponding STL version instead of the MSVC compiler.

Your STL version detection relies on _MSC_VER, hence this change looks pointless - STL versions are tied to MSC versions. At least, this is how your code was written.
Even more, the original check in secblock.h was and still is valid - but the library switched to indirection for compiler version and now clang fails.

And I added all the other versions for completeness.

More like over-engeneering than completeness.
The list of STL versions is never used, all we need to know if the version was above or below certain value.
It should be sufficient to change config_ver.h only in the most trivial way:

#if defined(_MSC_VER)
#  define CRYPTOPP_MSC_VERSION (_MSC_VER)
# if !defined(__clang__)
#  undef CRYPTOPP_LLVM_CLANG_VERSION
# endif
#endif

@ChristophStrehle
Copy link
Author

Hello irwi,
the suggestion you made would resolve this specific issue. But it would also define CRYPTOPP_MSC_VERSION when building with clang-cl. I think the intention of a89a27b was to have CRYPTOPP_MSC_VERSION only defined when building with MSVC but not when building with clang-cl.

@irwir
Copy link
Contributor

irwir commented Jun 30, 2025

Have you read carefully the issue 147 referenced in the mentioned PR?
The "attempt to unbind clang from others" mostly failed, because on Windows clang is not self-sufficient and works on top of an other compiler.

If definition of CRYPTOPP_MSC_VERSION was undesirable - even though it did not break anything, then nothing would be simpler than reverting to original #if _MSC_VER >= 1500 check in secblock.h.
Or, in more formal way, see #1315 (comment)

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