Cryptopp: Crypto++ messes with user's warning config

[MarcelRaad]

config.h is included in public headers and enables and disables compiler warnings globally:
https://github.com/weidai11/cryptopp/blob/CRYPTOPP_5_6_5/config.h#L414

Maybe this block could be moved to some library-internal header like pch.h? Or alternatively, if it is meant to protect also the public headers when building user code, every public header should have a warning push/pop pair.

[noloader]

Yeah, that's been there forever. Wei is kind of old school, and he believes the compiler should be smart enough to figure out most things given a context within a function or source file. When the compiler couldn't do it, he just hit it with a sledge hammer. There's a mailing list post about it (from years ago), but I can't find it at the moment.

I know its a philosophical difference, but I've never liked the practice or the cross-pollination very much. I think the future strategy should probably follow:

1. Clear the warning if possible
2. Push/pop if its a hardship to clear the warning

Since we dropped VC++ 6.0 support, I believe we have push/pop available in all MSVC compilers. .Net 2002 and .Net 2003 were unintended casualties of dropping VC++ 6.0 support. What it means is, we effectively support VS2005 and above.

Related note 1: I think we should leave the security related warnings enabled even if the user does not want them. Many users are not aware of some of the security implications, so its education. If they fix the issue that caused the warning, then they have improved their app or program.

Related note 2: we've push/pop'd macros in some places they may not be needed any longer.

[MarcelRaad]

Regarding your "related note 1": I agree these warnings should be fixed in user code and I always compile my own code with all fixable warnings enabled as errors. The problem is when a Crypto++ header is included before another 3rd-party library.

My use case is:

#pragma warning(push)
#pragma warning(disable: 4826)
#include <cryptlib.h>
#include <gmock.h>
#pragma warning(pop)

Only with many more includes and many more disabled warnings. Unfortunately a Google Test header emits warning C4826 in 32-bit mode as a pointer is cast to a uint64_t and back to a pointer to address a problem with another compiler.

[noloader]

@MarcelRaad,

Things were not as bad as they looked. Here's a quick first pass that removed 9 of the suppression: Commit 5f0cbde9804f.

[MarcelRaad]

Thanks @noloader , looks much better already!

[noloader]

@MarcelRaad,

Do you have anything for C4231? Its loading up build logs. From our Appveyor build (which is only partially working at the moment):

  pch.cpp
c:\projects\cryptopp\secblock.h(261): warning C4231: nonstandard extension used : 'extern' before template explicit instantiation [C:\projects\cryptopp\cryptlib.vcxproj]
c:\projects\cryptopp\secblock.h(262): warning C4231: nonstandard extension used : 'extern' before template explicit instantiation [C:\projects\cryptopp\cryptlib.vcxproj]
c:\projects\cryptopp\secblock.h(263): warning C4231: nonstandard extension used : 'extern' before template explicit instantiation [C:\projects\cryptopp\cryptlib.vcxproj]
c:\projects\cryptopp\secblock.h(264): warning C4231: nonstandard extension used : 'extern' before template explicit instantiation [C:\projects\cryptopp\cryptlib.vcxproj]
c:\projects\cryptopp\secblock.h(269): warning C4231: nonstandard extension used : 'extern' before template explicit instantiation [C:\projects\cryptopp\cryptlib.vcxproj]
c:\projects\cryptopp\cryptlib.h(1175): warning C4505: 'CryptoPP::AuthenticatedSymmetricCipher::AlgorithmName' : unreferenced local function has been removed [C:\projects\cryptopp\cryptlib.vcxproj]
...

[MarcelRaad]

@noloader Strange, I can't reproduce this warning and it's also not documented for VS2013 and later anymore:
https://msdn.microsoft.com/en-us/library/t460hcc3(v=vs.140).aspx
https://msdn.microsoft.com/en-us/library/9d0x3403(v=vs.110).aspx

On this page, extern explicit instantiation declararations are listed as "Microsoft-specific' until VS2012:
https://msdn.microsoft.com/en-us/library/by56e477(v=vs.110).aspx
vs.
https://msdn.microsoft.com/en-us/library/by56e477(v=vs.120).aspx

Which makes sense as it's a C++11 feature.

So I thought maybe AppVeyor is using the VS2010 compiler as configured in the project file? But I don't even get the warning in VS2010 :-(

[noloader]

@MarcelRaad,

I cleared a bunch of the warnings under VS2010 and below using per-include pragmas at Commit eb3b27a6a543.

It only clears them for the static libs, which I hope is OK. The static libs are the dominant use case; and they are mostly straight forward because we can drop extern if neither CRYPTOPP_IMPORT or CRYPTOPP_EXPORT.

[noloader]

@MarcelRaad,

I added <DisableSpecificWarnings>4231; 4355; 4505</DisableSpecificWarnings> to the property page when compiling our source files. It will not cross-pollinate into other people's projects; and it will keep the warnings enabled for other people's projects.

I'm closing this out.

[noloader]

Cleared at Commit 5f0cbde9804f, Commit eb3b27a6a543 and Commit fe9eb75dd289 (among others).

[MarcelRaad]

Thanks!