Super — thanks, Akim! Derek
> On Jan 20, 2019, at 9:33 AM, Akim Demaille <a...@lrde.epita.fr> wrote: > > Hi! > >> Le 20 janv. 2019 à 16:02, Derek Clegg <de...@me.com> a écrit : >> >> Hi, Akim — >> >> On Jan 19, 2019, at 11:49 PM, Akim Demaille <a...@lrde.epita.fr> wrote: >>> >>> I don't think I will address this warning, the cure is way worse than the >>> problem. >>> >>> To add the destructor, I must make the signature explicit, and it cannot be >>> different from the one in the base class, so I must add noexcept: >>> >>> ~syntax_error () YY_NOEXCEPT; >> >> In C++98 the signature is >> virtual ~exception() throw(); >> and in C++11 the signature is >> virtual ~exception(); >> so this only has to be handled specially for C++98. > > Wow... You are right! I would never have believed they actually removed the > 'throw()', instead of making it 'noexcept'... > > This is what I have in my copy of the C++98 standard: > >> 18.6.1 Class exception [lib.exception] >> >> namespace std { >> class exception { >> public: >> >> exception() throw(); >> exception(const exception&) throw(); >> exception& operator=(const exception&) throw(); >> virtual ~exception() throw(); >> virtual const char* what() const throw(); >> >> }; } > > > and this is n3797, a draft of C++14. I guess they did the same for C++11. > >> 18.8.1 Class exception [exception] >> >> namespace std { >> class exception { >> public: >> >> exception() noexcept; >> exception(const exception&) noexcept; >> exception& operator=(const exception&) noexcept; >> virtual ~exception(); >> virtual const char* what() const noexcept; >> >> }; } > > > The reason I did not even try is that I fell onto this in libc++ while > addressing your bug report: > > In comment: >> class exception >> { >> public: >> exception() noexcept; >> exception(const exception&) noexcept; >> exception& operator=(const exception&) noexcept; >> virtual ~exception() noexcept; >> virtual const char* what() const noexcept; >> }; > > Or actual code: > >> class _LIBCPP_EXCEPTION_ABI runtime_error >> : public exception >> { >> private: >> _VSTD::__libcpp_refstring __imp_; >> public: >> explicit runtime_error(const string&); >> explicit runtime_error(const char*); >> >> runtime_error(const runtime_error&) _NOEXCEPT; >> runtime_error& operator=(const runtime_error&) _NOEXCEPT; >> >> virtual ~runtime_error() _NOEXCEPT; >> >> virtual const char* what() const _NOEXCEPT; >> }; > > with > >> #ifndef _LIBCPP_HAS_NO_NOEXCEPT >> # define _NOEXCEPT noexcept >> # define _NOEXCEPT_(x) noexcept(x) >> #else >> # define _NOEXCEPT throw() >> # define _NOEXCEPT_(x) >> #endif > > Besides, cppreference shows nothing about exception specifiers > (https://en.cppreference.com/w/cpp/error/exception/%7Eexception). > >> Wouldn’t it be sufficient to explicitly write >> >> #if 201103L <= YY_CPLUSPLUS >> ~syntax_error(); >> #else >> ~syntax_error() throw(); >> #endif >> >> and similarly for the definition? > > Yes, it does. Actually, all this is a red-herring, ~syntax_error works fine > with throw() and noexcept. It's the other uses of YY_NOEXCEPT as throw() > that is a problem. I don't feel like investigating right now, but I should > do it at some point. > >> It seems problematic to redefine YY_NOEXCEPT, especially since “noexcept” >> and “throw()” aren’t used equivalently. > > Well, I thought it was the case where I used it. > > I will install the following commit when it's validated by the CI. It > depends on another commit (glr.cc: be more alike lalr1.cc). You can fetch it > from my branch Wweak-vtables on GitHub. > > Again, thanks a lot Derek! > > > > commit 9d792e20f4e3b03226095d9b3389038bf6c14791 > Author: Akim Demaille <akim.demai...@gmail.com> > Date: Sun Jan 20 08:23:41 2019 +0100 > > c++: address -Wweak-vtables warnings > > Reported by Derek Clegg > http://lists.gnu.org/archive/html/bug-bison/2019-01/msg00021.html > > To avoid this warning, we need syntax_error to have a virtual function > defined in a compilation unit. Let it be the destructor. To comply > with C++98, this dtor should be 'throw()'. Merely making YY_NOEXCEPT > be 'throw()' in C++98 triggers > errors (http://lists.gnu.org/archive/html/bug-bison/2019-01/msg00022.html), > so let's introduce YY_NOTHROW and flag only ~syntax_error with it. > > Also, since we add an explicit dtor, we now need to provide an > explicit copy ctor. > > * configure.ac (warn_cxx): Add -Wweak-vtables. > * data/skeletons/c++.m4 (YY_NOTHROW): New. > (syntax_error): Declare the dtor, and define the copy ctor. > * data/skeletons/glr.cc, data/skeletons/lalr1.cc (~syntax_error): > Define. > > diff --git a/configure.ac b/configure.ac > index a3a471af..1b5343e7 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -100,7 +100,7 @@ if test "$enable_gcc_warnings" = yes; then > -Wpointer-arith -Wshadow > -Wwrite-strings' > warn_c='-Wbad-function-cast -Wstrict-prototypes' > - warn_cxx='-Wextra-semi -Wnoexcept -Wundefined-func-template' > + warn_cxx='-Wextra-semi -Wnoexcept -Wundefined-func-template -Wweak-vtables' > # Warnings for the test suite only. > # > # -fno-color-diagnostics: Clang's use of colors in the error > diff --git a/data/skeletons/c++.m4 b/data/skeletons/c++.m4 > index f6f95c4c..acc35b62 100644 > --- a/data/skeletons/c++.m4 > +++ b/data/skeletons/c++.m4 > @@ -77,8 +77,10 @@ m4_define([b4_cxx_portability], > // Support noexcept when possible. > #if 201103L <= YY_CPLUSPLUS > # define YY_NOEXCEPT noexcept > +# define YY_NOTHROW > #else > # define YY_NOEXCEPT > +# define YY_NOTHROW throw () > #endif > > // Support noexcept when possible. > @@ -217,7 +219,14 @@ m4_define([b4_public_types_declare], > syntax_error (]b4_locations_if([const location_type& l, ])[const > std::string& m) > : std::runtime_error (m)]b4_locations_if([ > , location (l)])[ > - {}]b4_locations_if([ > + {} > + > + syntax_error (const syntax_error& s) > + : std::runtime_error (s.what ())]b4_locations_if([ > + , location (s.location)])[ > + {} > + > + ~syntax_error () YY_NOEXCEPT YY_NOTHROW;]b4_locations_if([ > > location_type location;])[ > }; > diff --git a/data/skeletons/glr.cc b/data/skeletons/glr.cc > index 778c65b8..56217341 100644 > --- a/data/skeletons/glr.cc > +++ b/data/skeletons/glr.cc > @@ -160,6 +160,9 @@ m4_pushdef([b4_parse_param], > m4_defn([b4_parse_param_orig]))dnl > ]b4_parser_class::~b4_parser_class[ () > {} > > + ]b4_parser_class[::syntax_error::~syntax_error () YY_NOEXCEPT YY_NOTHROW > + {} > + > int > ]b4_parser_class[::operator() () > { > diff --git a/data/skeletons/lalr1.cc b/data/skeletons/lalr1.cc > index 053ba3be..1dc2851d 100644 > --- a/data/skeletons/lalr1.cc > +++ b/data/skeletons/lalr1.cc > @@ -562,6 +562,8 @@ m4_if(b4_prefix, [yy], [], > ]b4_parser_class::~b4_parser_class[ () > {} > > + ]b4_parser_class[::syntax_error::~syntax_error () YY_NOEXCEPT YY_NOTHROW > + {} > > /*---------------. > | Symbol types. | >