Re: Bison 3.2.90 released [beta]

[Akim Demaille]

Hi Derek,

> Le 13 janv. 2019 à 23:01, Derek Clegg <address@hidden> 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 <address@hidden>
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__