Fix msvc compilation #70
Conversation
VLAs are not allowed in ISO C++, and MSVC doesn't support VLAs at all. The empty() check is needed, because front() on both std::vector and std::string while empty is UB. Use std::memcpy() instead std::copy(), because _data holds uint8_t's instead of chars; otherwise std::copy() could be optimized to just memmove.
MSVC only supports designated initializers since C++20.
MSVC defines min and max in windows.h when NOMINMAX is not defined. Define it when compiling with MSVC to prevent a collision with std::numeric_limits::max.
|
Thank you for the contribution. I have no problem with this. I'll go ahead and merge and we'll fix with new pull request if it causes any issues |
|
Hi, Thanks for the contribution, however this PR breaks the implementation of CipBaseString. E.g. with discovery the product name is now empty. Vector/string.reserve() only requests a change in capacity, not in size and thus e.g. functions such as .empty() or .size() fail in all where this is used. E.g. vector/string.resize() is required in the constructor and string getter to be able to use size/length functions. Created a PR #73 - std::copy can be used freely here, both std::memcpy and std::copy implicitly cast char to uint8. Since the vector is always properly resized now, we can safely use begin/end for copy actions (i.e. _length is not used aside from it being required for the ShortString type). |
While #47 greatly improved the Windows experience, the current master cannot be build with MSVC.
There are three issues here:
windows.h#definesminandmax, which conflict withstd::numeric_limits::minandmax.For 1, I already found it weird that the contents of the input string and vector (CipBaseString constructor and toStdString respectively) are copied into a temporary (VLA) buffer, after which they are copied again, to _data and the returned string. To work around, just use memcpy to copy the contents.
For 2, either the C++ standard needs to be bumped to C++20, or the designated initializers need to be removed (#ifdef-ed out?).
No. 3 is trivial to fix; it requires
#define NOMINMAX(but it affects the examples and tests as well).Let me know what you think.