weidai11 / cryptopp

free C++ class library of cryptographic schemes
https://cryptopp.com
Other
4.67k stars 1.47k forks source link

Fix macro and member function confilct for min and max #1214

Closed irwir closed 1 year ago

irwir commented 1 year ago

The library builds fine in VS 2022, but compilation of a Windows application ends up with thousads of errors. The reason is that several MS C++ headers define min and max as macros. An example of preprocessed code: if (from > static_cast<word64>(std::numeric_limits<sword64>::((() > ()) ? () : ())))

Quick search in the net gave this hint to add parentheses and prevent macro expansion.

noloader commented 1 year ago

Thanks @irwir.

I think that's new behavior from MSVC. In the past, it used to trigger on min(a, b). I don't recall it triggering on numeric_limits::min().

In fact, cryptest.sh only checks for the former; not the later. https://github.com/weidai11/cryptopp/blob/master/TestScripts/cryptest.sh#L1260 . So in the past, numeric_limits has not been a problem.

noloader commented 1 year ago

I wonder why that's not failing in other files:

$  grep -E 'numeric_limits<.*>::min|numeric_limits<.*>::max' *.h *.cpp
misc.h:///  then the library uses <tt>std::numeric_limits<size_t>::max()</tt>.
misc.h:///  <tt>std::numeric_limits<size_t>::max()</tt> is not always a <tt>constexpr</tt>, and
misc.h:#  define SIZE_MAX ((std::numeric_limits<size_t>::max)())
misc.h: if (from > static_cast<word64>((std::numeric_limits<sword64>::max)()))
misc.h: if (from > static_cast<word64>((std::numeric_limits<word32>::max)()))
misc.h: else if (from > static_cast<sword64>((std::numeric_limits<word32>::max)()))
misc.h: if (from > static_cast<word64>((std::numeric_limits<sword32>::max)()))
misc.h: if (from > static_cast<sword64>((std::numeric_limits<sword32>::max)()))
misc.h: else if (from < static_cast<sword64>((std::numeric_limits<sword32>::min)()))
misc.h: if (from > static_cast<word32>((std::numeric_limits<sword32>::max)()))
misc.h:///  Apple Clang 6.0 and numeric_limits<word128>::max() returns 0</A>.
misc.h: return (std::numeric_limits<T>::min)();
misc.h:///  Apple Clang 6.0 and numeric_limits<word128>::max() returns 0</A>.
misc.h: return (std::numeric_limits<T>::max)();
scrypt.h:    ///   <tt>std::numeric_limits<int>::max()</tt>.
chacha.cpp:    CRYPTOPP_ASSERT(iterationCount <= std::numeric_limits<word32>::max());
files.cpp:                      size = ((std::numeric_limits<std::streamsize>::max)());
scrypt.cpp:    if (std::numeric_limits<size_t>::max() > std::numeric_limits<word32>::max())
scrypt.cpp:    CRYPTOPP_ASSERT(parallelization <= static_cast<word64>(std::numeric_limits<int>::max()));
scrypt.cpp:    if (parallelization > static_cast<word64>(std::numeric_limits<int>::max()))
scrypt.cpp:        oss << std::numeric_limits<int>::max();
scrypt.cpp:        maxParallel = std::numeric_limits<int>::max();
noloader commented 1 year ago

@irwir,

I guarded std::numeric_limits<T>::min and std::numeric_limits<T>::max in the other source files. I also updated cryptest.sh to detect unguarded numeric_limits.

Also see Commit cc920825d2da.

irwir commented 1 year ago

Thanks for merging. Even if it is new for VS, the stackoverflow link dates back to 2014. Appears that preprocessor is triggered by max( without checking for arguments as was typical for functions. Other .cpp files simply do not get recompiled with Windows' headers.