Thanks for this fix. I had some problems applying the patch, so I’ll wait until
the next release candidate to test it. It looks okay, however, based on a brief
inspection. One thing:
> if test "$enable_gcc_warnings" = yes; then
> warn_common='-Wall -Wextra -Wno-sign-compare -Wcast-align
> -fparse-all-comments -Wdocumentation
> - -Wformat -Wnull-dereference -Wpointer-arith -Wshadow -Wwrite-strings'
> + -Wformat -Wnull-dereference -Wpointer-arith -Wshadow
> + -Wundefined-func-template -Wwrite-strings'
> warn_c='-Wbad-function-cast -Wstrict-prototypes'
> warn_cxx='-Wextra-semi -Wnoexcept'
> # Warnings for the test suite only.
I would expect “-Wundefined-func-template” to appear in “warn_cxx”, not
“warn_common”. But I may not fully understand what’s going on here.
Thanks!
Derek
> On Jan 14, 2019, at 10:30 PM, Akim Demaille <a...@lrde.epita.fr> wrote:
>
> Hi Derek,
>
>> Le 13 janv. 2019 à 23:01, Derek Clegg <de...@me.com> a écrit :
>>
>> This is a more serious problem for me:
>>
>> aux/parser-internal.h:767:7: error: instantiation of function
>> 'yy::parser::basic_symbol<yy::parser::by_type>::basic_symbol' required
>> here, but no definition is available [-Werror,-Wundefined-func-template]
>> symbol_type () {}
>> ^
>> aux/parser-internal.h:530:7: note: forward declaration of template entity is
>> here
>> basic_symbol ();
>> ^
>> aux/parser-internal.h:767:7: note: add an explicit instantiation declaration
>> to
>> suppress this warning if
>> 'yy::parser::basic_symbol<yy::parser::by_type>::basic_symbol' is
>> explicitly instantiated in another translation unit
>> symbol_type () {}
>> ^
>> 1 error generated.
>>
>> I don’t know think it’s possible to declare a forward declaration of a
>> template entity within a class, which makes the fix nontrivial. The only fix
>> I can see is to move the definitions of the symbol_type methods outside of
>> the header file and into the .cc file (which I think would be clearer, in
>> any case).
>
> That it would be clearer, I agree, it was like this initially. But it's too
> much effort to maintain it this way, and no-one answered my question about
> that:
>
> https://lists.gnu.org/archive/html/bison-patches/2018-12/msg00058.html
>
> So I installed this one:
>
> https://lists.gnu.org/archive/html/bison-patches/2018-12/msg00075.html
>
> and several others afterwards.
>
> So I'd rather not go "backwards" (in the chronological sense, not judgmental
> on the style, I tend to agree with you).
>
> I'll install this, once validated by the CI.
>
> Thanks a lot for this report.
>
>
>
> commit a049509d0437046e57a0e96f71452ddb33f8eecc
> Author: Akim Demaille <akim.demai...@gmail.com>
> Date: Mon Jan 14 19:57:02 2019 +0100
>
> c++: avoid -Wundefined-func-template warnings from clang
>
> Reported by Derek Clegg.
> http://lists.gnu.org/archive/html/bug-bison/2019-01/msg00006.html
>
> Clang does not like this:
>
> template <typename D>
> struct basic_symbol : D
> {
> basic_symbol();
> };
>
> struct by_type {};
>
> struct symbol_type : basic_symbol<by_type>
> {
> symbol_type(){}
> };
>
> It gives:
>
> $ clang++-mp-7.0 -Wundefined-func-template foo.cc -c
> foo.cc:11:3: warning: instantiation of function
> 'basic_symbol<by_type>::basic_symbol'
> required here, but no definition is available
> [-Wundefined-func-template]
> symbol_type(){}
> ^
> foo.cc:4:3: note: forward declaration of template entity is here
> basic_symbol();
> ^
> foo.cc:11:3: note: add an explicit instantiation declaration to
> suppress this warning
> if 'basic_symbol<by_type>::basic_symbol' is explicitly
> instantiated in
> another translation unit
> symbol_type(){}
> ^
> 1 warning generated.
>
> The same applies for the basic_symbol's destructor and `clear()`.
>
> * configure.ac (warn_cxx): Add -Wundefined-func-template.
> This triggered one failure in the test suite:
> * tests/headers.at (Sane headers): here, where we check that we can
> compile the generated headers in other compilation units than the
> parser's.
> Add a variant type to make sure that basic_symbol and symbol_type are
> properly generated in this case.
> * data/skeletons/c++.m4 (basic_symbol): Inline the definitions of the
> destructor and of `clear` in the class definition.
>
> diff --git a/configure.ac b/configure.ac
> index e60af602..b7ba45dc 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -96,7 +96,8 @@ AM_CONDITIONAL([ENABLE_GCC_WARNINGS], [test
> "$enable_gcc_warnings" = yes])
> if test "$enable_gcc_warnings" = yes; then
> warn_common='-Wall -Wextra -Wno-sign-compare -Wcast-align
> -fparse-all-comments -Wdocumentation
> - -Wformat -Wnull-dereference -Wpointer-arith -Wshadow -Wwrite-strings'
> + -Wformat -Wnull-dereference -Wpointer-arith -Wshadow
> + -Wundefined-func-template -Wwrite-strings'
> warn_c='-Wbad-function-cast -Wstrict-prototypes'
> warn_cxx='-Wextra-semi -Wnoexcept'
> # Warnings for the test suite only.
> diff --git a/data/skeletons/c++.m4 b/data/skeletons/c++.m4
> index c985cb98..6f2d75d2 100644
> --- a/data/skeletons/c++.m4
> +++ b/data/skeletons/c++.m4
> @@ -260,7 +260,10 @@ m4_define([b4_symbol_type_define],
> typedef Base super_type;
>
> /// Default constructor.
> - basic_symbol ();
> + basic_symbol ()
> + : value ()]b4_locations_if([
> + , location ()])[
> + {}
>
> #if 201103L <= YY_CPLUSPLUS
> /// Move constructor.
> @@ -282,10 +285,29 @@ m4_define([b4_symbol_type_define],
> YY_RVREF (location_type) l])[);
> ]])[
> /// Destroy the symbol.
> - ~basic_symbol ();
> + ~basic_symbol ()
> + {
> + clear ();
> + }
>
> /// Destroy contents, and record that is empty.
> - void clear ();
> + void clear ()
> + {]b4_variant_if([[
> + // User destructor.
> + symbol_number_type yytype = this->type_get ();
> + basic_symbol<Base>& yysym = *this;
> + (void) yysym;
> + switch (yytype)
> + {
> +]b4_symbol_foreach([b4_symbol_destructor])dnl
> +[ default:
> + break;
> + }
> +
> + // Type destructor.
> +]b4_symbol_variant([[yytype]], [[value]], [[template destroy]])])[
> + Base::clear ();
> + }
>
> /// Whether empty.
> bool empty () const YY_NOEXCEPT;
> @@ -365,12 +387,6 @@ m4_define([b4_symbol_type_define],
> # Provide the implementation needed by the public types.
> m4_define([b4_public_types_define],
> [[ // basic_symbol.
> - template <typename Base>
> - ]b4_parser_class[::basic_symbol<Base>::basic_symbol ()
> - : value ()]b4_locations_if([
> - , location ()])[
> - {}
> -
> #if 201103L <= YY_CPLUSPLUS
> template <typename Base>
> ]b4_parser_class[::basic_symbol<Base>::basic_symbol (basic_symbol&& that)
> @@ -416,32 +432,6 @@ m4_define([b4_public_types_define],
> (void) v;
> ]b4_symbol_variant([this->type_get ()], [value], [YY_MOVE_OR_COPY],
> [YY_MOVE (v)])])[}]])[
>
> - template <typename Base>
> - ]b4_parser_class[::basic_symbol<Base>::~basic_symbol ()
> - {
> - clear ();
> - }
> -
> - template <typename Base>
> - void
> - ]b4_parser_class[::basic_symbol<Base>::clear ()
> - {]b4_variant_if([[
> - // User destructor.
> - symbol_number_type yytype = this->type_get ();
> - basic_symbol<Base>& yysym = *this;
> - (void) yysym;
> - switch (yytype)
> - {
> -]b4_symbol_foreach([b4_symbol_destructor])dnl
> -[ default:
> - break;
> - }
> -
> - // Type destructor.
> - ]b4_symbol_variant([[yytype]], [[value]], [[template destroy]])])[
> - Base::clear ();
> - }
> -
> template <typename Base>
> bool
> ]b4_parser_class[::basic_symbol<Base>::empty () const YY_NOEXCEPT
> diff --git a/tests/headers.at b/tests/headers.at
> index 32c8d856..a58074fe 100644
> --- a/tests/headers.at
> +++ b/tests/headers.at
> @@ -125,7 +125,7 @@ AT_BISON_OPTION_PUSHDEFS([$1])
> AT_DATA_GRAMMAR([input.y],
> [[$1
> %define parse.error verbose
> -]AT_VARIANT_IF([], [%union {int integer;}])[
> +]AT_VARIANT_IF([%token <int> 'x'], [%union {int integer;}])[
> %code {
> ]AT_PUSH_IF([[
> #if defined __GNUC__ && 7 == __GNUC__
>
