[-Weffc++] warnings

[idelsink]

When including this as a subdirectory using CMake, a lot of C++ efficiency warnings show op when the -Weffc++ flag is set.

I'm doing the following in my root CMakeLists.txt file:

add_compile_options(-Weffc++)
add_subdirectory(googletest)

This can be fixed by passing the SYSTEM variable to the include_directories() command.
This way the headers will be seen as "system" headers.
The user can do the same in his or her CMakeLists.txt

include_directories(${gtest_SOURCE_DIR}/include ${gtest_SOURCE_DIR})

[BillyDonahue]

I don't think Googletest should show up as a "system" header to work around a build config issue.
I don't understand how this makes the -Weffc++ warnings better.

[BillyDonahue]

see discussion in issue#898

[idelsink]

This way if a user want's to enforce the -Weffc++ flag for it's complete project, he or she doesn't have to worry about warnings from googletest showing up.
Another option of course is to actually solve the warnings in the software itself, if I find the time I could take a look at them. :)

[BillyDonahue]

I don't know what warnings this flag produces, but if it's reasonable to work around it, let's.
I am not super keen on writing ugly software patterns to work around false positives though.

[BillyDonahue]

BTW, this flag has nothing to do with "efficiency".

"-Weffc++ (C++ and Objective-C++ only) Warn about violations of the following style guidelines from Scott Meyers' Effective C++ series of books"

https://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Dialect-Options.html

Now I'm less excited about this.

Also:

"When selecting this option, be aware that the standard library headers do not obey all of these guidelines; use ‘grep -v’ to filter out those warnings."

Can you use grep -v similarly on googletest?

[idelsink]

Yeah, I myself am struggling lately if this option is even something I want to use on a daily basis.

the grep -v part you mentioned is only needed to filter out those matching.
E.g. grep -v match would show everything except matching match.

-v --invert-match
Invert the sense of matching, to select non-matching lines. (-v is specified by POSIX.)

For the errors I did the following:

• Add add_compile_options(-Weffc++) to the root CMakeLists.txt
• set the following cmake variables to:
  • BUILD_GMOCK=ON
  • BUILD_GTEST=ON
  • BUILD_SHARED_LIBS=ON
  • gmock_build_tests=ON
  • gtest_build_tests=ON
• compile with: make 2>&1 | grep "\[-Weffc++\]" > build_log.txt thus catching "all" -Weffc++ warnings.

Most of these warnings mention the variables not set in the initialization list.
A few samples are listed below, for fill log see this file:
build_log.txt

:internal::Notification::mutex_’ should be initialized in the member initialization list [-Weffc++]
/home/ingmar/workspace/googletest/googletest/include/gtest/internal/gtest-port.h:1930:7: warning: ‘testing::internal::MutexBase::mutex_’ should be initialized in the member initialization list [-Weffc++]
/home/ingmar/workspace/googletest/googletest/include/gtest/internal/gtest-port.h:1930:7: warning: ‘testing::internal::MutexBase::has_owner_’ should be initialized in the member initialization list [-Weffc++]
...
/home/ingmar/workspace/googletest/googletest/include/gtest/internal/gtest-param-util.h:309:9: warning: ‘class testing::internal::ValuesInIteratorRangeGenerator<bool>::Iterator’ has pointer data members [-Weffc++]
/home/ingmar/workspace/googletest/googletest/include/gtest/internal/gtest-param-util.h:309:9: warning:   but does not override ‘operator=(const testing::internal::ValuesInIteratorRangeGenerator<bool>::Iterator&)’ [-Weffc++]
/home/ingmar/workspace/googletest/googletest/include/gtest/internal/gtest-param-util.h:309:9: warning: ‘class testing::internal::ValuesInIteratorRangeGenerator<int>::Iterator’ has pointer data members [-Weffc++]
/home/ingmar/workspace/googletest/googletest/include/gtest/internal/gtest-param-util.h:309:9: warning:   but does not override ‘operator=(const testing::internal::ValuesInIteratorRangeGenerator<int>::Iterator&)’ [-Weffc++]

[jwakely]

GCC's -Weffc++ is IMHO effectively obsolete, and should not be used, unless you are trying to ensure your code meets outdated advice from more than ten years ago. There are numerous bug reports about -Weffc++ in GCC's bugzilla and nobody is interested in fixing them. Among its problems are that the suggestions it makes are based on the first edition of Effective C++ and those items were heavily revised for the second edition. Some advice is simply inappropriate in modern code (C++11 or later) and some warnings are simply poorly implemented in GCC (e.g requiring a mem-initializer for all member variables even if they have default constructors that do the right thing).

Changing modern projects to avoid unhelpful -Weffc++ warnings is a huge mistake.

[gennadiycivil]

I have been cleaning up older and inactive GitHub googletest issues. You may see an issue "closed" if it appears to be inactive/abandoned
Thank you