boostorg / wave

Boost.org wave module
http://boost.org/libs/wave
21 stars 49 forks source link

GCC suggest-override warnings #88

Closed EugeneZelenko closed 4 years ago

EugeneZelenko commented 4 years ago

Complete list of warnings when Boost 1.72 is built with GCC 7.5 with -Wsuggest-override added to cxxflags. Duplicated warnings from same location are omitted:

./boost/wave/cpp_exceptions.hpp:164:25: warning: ‘virtual const char* boost::wave::preprocess_exception::what() const’ can be marked override [-Wsuggest-override]
./boost/wave/cpp_exceptions.hpp:168:25: warning: ‘virtual const char* boost::wave::preprocess_exception::description() const’ can be marked override [-Wsuggest-override]
./boost/wave/cpp_exceptions.hpp:172:17: warning: ‘virtual int boost::wave::preprocess_exception::get_severity() const’ can be marked override [-Wsuggest-override]
./boost/wave/cpp_exceptions.hpp:176:17: warning: ‘virtual int boost::wave::preprocess_exception::get_errorcode() const’ can be marked override [-Wsuggest-override]
./boost/wave/cpp_exceptions.hpp:180:25: warning: ‘virtual const char* boost::wave::preprocess_exception::get_related_name() const’ can be marked override [-Wsuggest-override]
./boost/wave/cpp_exceptions.hpp:184:18: warning: ‘virtual bool boost::wave::preprocess_exception::is_recoverable() const’ can be marked override [-Wsuggest-override]
./boost/wave/cpp_exceptions.hpp:384:25: warning: ‘virtual const char* boost::wave::macro_handling_exception::what() const’ can be marked override [-Wsuggest-override]
./boost/wave/cpp_exceptions.hpp:388:17: warning: ‘virtual const char* boost::wave::macro_handling_exception::get_related_name() const’ can be marked override [-Wsuggest-override]
./boost/wave/cpp_exceptions.hpp:79:25: warning: ‘virtual const char* boost::wave::cpp_exception::what() const’ can be marked override [-Wsuggest-override]
./boost/wave/cpplexer/cpplexer_exceptions.hpp:145:25: warning: ‘virtual const char* boost::wave::cpplexer::cpplexer_exception::what() const’ can be marked override [-Wsuggest-override]
./boost/wave/cpplexer/cpplexer_exceptions.hpp:189:25: warning: ‘virtual const char* boost::wave::cpplexer::lexing_exception::what() const’ can be marked override [-Wsuggest-override]
./boost/wave/cpplexer/cpplexer_exceptions.hpp:193:25: warning: ‘virtual const char* boost::wave::cpplexer::lexing_exception::description() const’ can be marked override [-Wsuggest-override]
./boost/wave/cpplexer/cpplexer_exceptions.hpp:197:17: warning: ‘virtual int boost::wave::cpplexer::lexing_exception::get_severity() const’ can be marked override [-Wsuggest-override]
./boost/wave/cpplexer/cpplexer_exceptions.hpp:201:17: warning: ‘virtual int boost::wave::cpplexer::lexing_exception::get_errorcode() const’ can be marked override [-Wsuggest-override]
./boost/wave/cpplexer/cpplexer_exceptions.hpp:205:18: warning: ‘virtual bool boost::wave::cpplexer::lexing_exception::is_recoverable() const’ can be marked override [-Wsuggest-override]
./boost/wave/cpplexer/re2clex/cpp_re2c_lexer.hpp:346:17: warning: ‘boost::wave::cpplexer::re2clex::lex_functor::token_type& boost::wave::cpplexer::re2clex::lex_functor::get(boost::wave::cpplexer::re2clex::lex_functor::token_type&) [with IteratorT = char*; PositionT = boost::wave::util::file_position, std::allocator, boost::wave::util::CowString > > >; TokenT = boost::wave::cpplexer::lex_token, std::allocator, boost::wave::util::CowString > > > >; boost::wave::cpplexer::re2clex::lex_functor::token_type = boost::wave::cpplexer::lex_token, std::allocator, boost::wave::util::CowString > > > >]’ can be marked override [-Wsuggest-override]
./boost/wave/cpplexer/re2clex/cpp_re2c_lexer.hpp:347:10: warning: ‘void boost::wave::cpplexer::re2clex::lex_functor::set_position(const PositionT&) [with IteratorT = char*; PositionT = boost::wave::util::file_position, std::allocator, boost::wave::util::CowString > > >; TokenT = boost::wave::cpplexer::lex_token, std::allocator, boost::wave::util::CowString > > > >]’ can be marked override [-Wsuggest-override]
./boost/wave/cpplexer/re2clex/cpp_re2c_lexer.hpp:349:10: warning: ‘bool boost::wave::cpplexer::re2clex::lex_functor::has_include_guards(std::__cxx11::string&) const [with IteratorT = char*; PositionT = boost::wave::util::file_position, std::allocator, boost::wave::util::CowString > > >; TokenT = boost::wave::cpplexer::lex_token, std::allocator, boost::wave::util::CowString > > > >; std::__cxx11::string = std::__cxx11::basic_string]’ can be marked override [-Wsuggest-override]

Depends on boostorg/config#329.

jefftrull commented 4 years ago

I believe Wave is staying with C++03 for now. There are a lot of things we could be doing if that were loosened :) @hkaiser any thoughts?

hkaiser commented 4 years ago

I believe Wave is staying with C++03 for now. There are a lot of things we could be doing if that were loosened :) @hkaiser any thoughts?

I'm not sure what Boost's policy is nowadays. If it was for me, I had no objections to slowly migrating the codebase to something more modern (C++17)?

Alternatively - just related to the override keyword - I would assume Boost will add a macro for this we could certainly use (or does is have already?).

EugeneZelenko commented 4 years ago

This is not about changing language standard. Macro, introduced in boostorg/config#329, would expand as override in C++11 or newer.

jefftrull commented 4 years ago

Ah, I see, that's great. I can take on making this update...

Are there other macros we should be using?

EugeneZelenko commented 4 years ago

I think you should look on doc/macro_reference.qbk or more readable formats generated from this file.

jefftrull commented 4 years ago

Looks like the actual macro is awaiting merge approval :) In the meantime I worked out a clang-tidy command line that does this for us:

clang-tidy -header-filter='.*hpp' -checks=-\*,modernize-use-override  -config="{CheckOptions: [{key: modernize-use-override.OverrideSpelling, value: BOOST_OVERRIDE}, {key: modernize-use-override.IgnoreDestructors, value: 1}]}" -p build -fix include/boost/wave/cpp_exceptions.hpp

in case it is useful for others. Caveats:

EugeneZelenko commented 4 years ago

BOOST_OVERRIDE was introduced in https://github.com/boostorg/config/commit/ffe4e0f5a448578cce14e3eed0cf7163333a81d9.

jefftrull commented 4 years ago

I found a few more places :)

EugeneZelenko commented 4 years ago

Great! Thank you for so quick fix!