#if when #ifdef should be used - inconsistent with AWS SDK C++

[M-Pixel]

The include headers contain a mix of #if _MSC_VER and #ifdef _MSC_VER. When compiled with strict warning-as-error settings, this produces the error '_MSC_VER' is not defined, evaluates to 0. The problem cannot be solved by defining _MSC_VER=0 in the build system, because this would cause the #ifdef statements to produce false-positives. The #ifs should be replaced with #ifdefs. This occurs with at least one other symbol in addition to _MSC_VER.

[jmklix]

Thanks for pointing this out. Looking into making these changes

[jmklix]

All of the changes have been merged. Please allow for a few days before all of the dependencies are updated. Or you can manually update them yourself if you would like to test the changes now.

[graebm]

aws-crt-cpp v0.19.8 contains all fixes

[JBakamovic]

@graebm Unfortunately the same happens with a few other symbols and which makes it incredibly easy to miss the actual important warnings because the build output is quite bloated with these warnings.

Could you please address the following symbols as well, please?

• __clang_analyzer__
• __APPLE__
• __linux__
• AWS_DEEP_CHECKS

P.S. I would do it myself but looks more involved than what I would expect for s/#if /#ifdef /g look alike change so there must be a context I am missing.

[graebm]

what compiler version, and what warning level are you setting?

Are you setting this warning level on your application? (valid)

Or are you setting it globally via CFLAGS and CXXFLAGS environment variables before compiling aws-crt-cpp? (workaround: don't do that)

[graebm]

Ah, you're using MSVC
You can disable warnings from external headers via:
https://learn.microsoft.com/en-us/cpp/build/reference/external-external-headers-diagnostics

[JBakamovic]

The warning level is set on the application level - through cmake. The code is built with GCC (11.1.0, 12.1.0), clang (14.0.5, 15.0.6) and MSVC (19.32) and described issues persist on all of those compilers.

[jmklix]

This was fixed with the above pr. Please let us know if you notice any other inconsistencies with these repo's