[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: -Wweak-vtables warnings (was: Bison 3.2.91 released [beta])
From: |
Derek Clegg |
Subject: |
Re: -Wweak-vtables warnings (was: Bison 3.2.91 released [beta]) |
Date: |
Sun, 20 Jan 2019 10:31:14 -0800 |
Super — thanks, Akim!
Derek
> On Jan 20, 2019, at 9:33 AM, Akim Demaille <address@hidden> wrote:
>
> Hi!
>
>> Le 20 janv. 2019 à 16:02, Derek Clegg <address@hidden> a écrit :
>>
>> Hi, Akim —
>>
>> On Jan 19, 2019, at 11:49 PM, Akim Demaille <address@hidden> 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 <address@hidden>
> 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. |
>
Re: [Bison-Announce] Bison 3.2.91 released [beta], Frank Heckenbach, 2019/01/20