Harmless but annoying -Woverloaded-virtual warnings from gcc

[vadz]

Compiling the header with g++ -Wall results in several warnings:

In file included from lyra/include/lyra/arg.hpp:10,
                 from lyra/include/lyra/lyra.hpp:12,
                 from main.cpp:31:
lyra/include/lyra/parser.hpp:165:23: warning: ‘virtual lyra::parse_result lyra::parser::parse(const string&, const lyra::detail::token_iterator&, const lyra::parser_customization&) const’ was hidden [-Woverloaded-virtual]
  virtual parse_result parse(std::string const & exe_name,
                       ^~~~~
In file included from lyra/include/lyra/arguments.hpp:10,
                 from lyra/include/lyra/lyra.hpp:13,
                 from main.cpp:31:
lyra/include/lyra/exe_name.hpp:45:23: warning:   by ‘virtual lyra::parse_result lyra::exe_name::parse(const lyra::detail::token_iterator&, const lyra::parser_customization&) const’ [-Woverloaded-virtual]
  virtual parse_result parse(
                       ^~~~~
In file included from lyra/include/lyra/arg.hpp:10,
                 from lyra/include/lyra/lyra.hpp:12,
                 from main.cpp:31:
lyra/include/lyra/parser.hpp:165:23: warning: ‘virtual lyra::parse_result lyra::parser::parse(const string&, const lyra::detail::token_iterator&, const lyra::parser_customization&) const’ was hidden [-Woverloaded-virtual]
  virtual parse_result parse(std::string const & exe_name,
                       ^~~~~
In file included from lyra/include/lyra/lyra.hpp:13,
                 from main.cpp:31:
lyra/include/lyra/arguments.hpp:146:15: warning:   by ‘virtual lyra::parse_result lyra::arguments::parse(const lyra::detail::token_iterator&, const lyra::parser_customization&) const’ [-Woverloaded-virtual]
  parse_result parse(detail::token_iterator const & tokens,
               ^~~~~

Getting rid of them seems to be relatively simple using the usual trick of making parse() itself non-virtual and adding pure virtual do_parse(), but maybe it would be even better to avoid having two overloaded parse() versions just to use it in cli? It looks like this could be solved by just setting m_exeName in the "normal" parse() overload, but maybe I'm missing something here, as I've just started using Lyra and don't know it at all.

I could propose a PR doing this if you'd be interested.

[grafikrobot]

What version of GCC, and Lyra, are you seeing that warning with? Asking because I run a warning-free CI test for the latest supported compilers (gcc, clang, msvc). And I don't see that issue.

[vadz]

Sorry, I was wrong about -Woverloaded-virtual being included in -Wall, it isn't (especially ironic because I've created a special table to be able to quickly look up things like this, but didn't use it). I.e. you have to explicitly have -Woverloaded-virtual on the command line to see it. This is a useful warning, however, and we enable it for all our code, so it would still be nice if Lyra didn't give it.

With this option on, all versions of g++ I have, i.e. 4.9, 5, 6, 7, 8, 9 and 10 (the last one is gcc version 10.2.1 20210110 (Debian 10.2.1-6)) give it. Here is the minimal reproducer:

#include <lyra/lyra.hpp>

int main(int argc, char* argv[])
{
    std::string input;
    bool help = false;
    auto cli = lyra::help(help)
      | lyra::arg(input, "input file").required()
      ;

    auto const result = cli.parse({argc, argv});
    if ( !result ) {
        std::cerr << result.errorMessage() << "\n" << cli;
        return 1;
    }

    std::cout << "Input: " << input << "\n";
    return 0;
}

but it can really be seen with any example using Lyra.

And, FWIW, here is a brute force way of disabling it:

commit 51bb86e93572070f4120f1b404950df060be38aa
Author: Vadim Zeitlin <vadim@zeitlins.org>
Date:   2020-12-17 00:40:29 +0100

    Work around -Woverloaded-virtual in Lyra headers

    Do this at least until https://github.com/bfgroup/Lyra/issues/42 is
    resolved upstream.

diff --git a/include/lyra/lyra.hpp b/include/lyra/lyra.hpp
index 467e30d..e1e5b08 100644
--- a/include/lyra/lyra.hpp
+++ b/include/lyra/lyra.hpp
@@ -7,6 +7,11 @@
 #ifndef LYRA_LYRA_HPP
 #define LYRA_LYRA_HPP

+#ifdef __GNUC__
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Woverloaded-virtual"
+#endif
+
 #include "lyra/version.hpp"

 #include "lyra/arg.hpp"
@@ -24,4 +29,8 @@
 #include "lyra/parser_result.hpp"
 #include "lyra/val.hpp"

+#ifdef __GNUC__
+#pragma GCC diagnostic pop
+#endif
+
 #endif // LYRA_HPP_INCLUDED