Conditionally disabled -Weffc++ and -Wshadow for compilers with broken implementations

[rjk9w5]

trafficstars

Ref. #937 This will turn of these flags which trigger false positives in GNU and Intel compilers.

[TedLyngmo]

I don't know CMake very well, but instead of turning -Weffc++ off completely for those compilers, would it be possible to make it disable the warning only for the line for which it gives a false positive so that the result becomes something like the below?

include/yaml-cpp/node/impl.h

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Weffc++"
  return *this;                             // this is currently line 206
#pragma GCC diagnostic pop

[rjk9w5]

For sure that can be done. The benefit of this approach is it is less invasive to the code. In general I am not a huge fan of polluting the code base with compiler specific pragmas. It also doesn't guarantee in the future when things are added people will remember to add the pragmas.

The scope of bring these into compliance is rather larger than makes sense for flags that aren't really that important. With -Wall -Wextra -pedantic -pedantic-errors it is already pretty strict. -Wshadow is only disable for Intel because it can't do scope checking correctly

Here are just a couple from Intel 2019. It is possible, but really gross, to put pragmas around all of the locations it is needed. Some of them could be fixed with #pragma once instead of using the generated header guards, but many of them are things that aren't even really in the guide...

Fun -Weffc++

../include/yaml-cpp/null.h(2): warning #2012: Effective C++ Item 1 prefer const and inline to #define
  #define NULL_H_62B23520_7C8E_11DE_8A39_0800200C9A66

This isn't even item 4, not sure what they are doing -Weffc++

../include/yaml-cpp/eventhandler.h(38): warning #2015: Effective C++ Item 4 prefer C++ style comments
    virtual void OnAnchor(const Mark& /*mark*/,

static member functions are not in the same scope as non-static member variables... -Wshadow

../include/yaml-cpp/exceptions.h(165): warning #3868: declaration hides member "YAML::Exception::mark" (declared at line 161) in same scope
    static const std::string build_what(const Mark& mark,

[TedLyngmo]

I agree that adding the compiler specific pragma directly into the code isn't nice. I was thining of tagging it somehow so that CMake could turn it into whatever the compiler needs.

Perhaps -Weffc++ is both a bit broken and perhaps the advice it gives is a bit outdated. I added it because it helped me to find some minor initialization bug if I remember correctly and I thought it would be good to keep it in, but perhaps it should be removed completely instead?

[rjk9w5]

Well this patch only turns it off for compilers with incorrect implementations. The clang implementation works great, and I agree when it is working correctly it can be very beneficial. The CMake change is just disabling it where the compiler is reporting more false positives than catching actual useful errors.