Re: Bug: duplicate c++ constructors

[Akim Demaille]

Le 28 juil. 2013 à 13:41, Valentin Tolmer <address@hidden> a écrit :

> Hello,

Hi Valentin,

> After working on a patch about symbol aliases, I got a very unexpected bug:
> when running make check, the generated examples/calc++/calc++-parser.hh has 
> duplicate definitions for the basic_symbol.

I can reproduce this with your branch.  Do you have this somewhere?

> Around line 317, I get
> 
> 
>  basic_symbol (typename Base::kind_type t, const location_type& l);
> 
>  basic_symbol (typename Base::kind_type t, const std::string v, const 
> location_type& l);
> 
>  basic_symbol (typename Base::kind_type t, const int v, const location_type& 
> l);
> 
>  basic_symbol (typename Base::kind_type t, const location_type& l);
> 
>  basic_symbol (typename Base::kind_type t, const int v, const location_type& 
> l);
> 
>  basic_symbol (typename Base::kind_type t, const location_type& l);

That does not look good.  It should read:

  basic_symbol (typename Base::kind_type t, const location_type& l);

  basic_symbol (typename Base::kind_type t, const int v, const location_type& 
l);

  basic_symbol (typename Base::kind_type t, const std::string v, const 
location_type& l);


>  // Implementation of basic_symbol constructor for each type.
> 
>  template <typename Base>
>  calcxx_parser::basic_symbol<Base>::basic_symbol (typename Base::kind_type t, 
> const location_type& l)
>    : Base (t)
>    , value ()
>    , location (l)
>  {}
> 
>  template <typename Base>
>  calcxx_parser::basic_symbol<Base>::basic_symbol (typename Base::kind_type t, 
> const std::string v, const location_type& l)
>    : Base (t)
>    , value (v)
>    , location (l)
>  {}
> 
>  template <typename Base>
>  calcxx_parser::basic_symbol<Base>::basic_symbol (typename Base::kind_type t, 
> const int v, const location_type& l)
>    : Base (t)
>    , value (v)
>    , location (l)
>  {}
> 
>  template <typename Base>
>  calcxx_parser::basic_symbol<Base>::basic_symbol (typename Base::kind_type t, 
> const location_type& l)
>    : Base (t)
>    , value ()
>    , location (l)
>  {}
> 
>  template <typename Base>
>  calcxx_parser::basic_symbol<Base>::basic_symbol (typename Base::kind_type t, 
> const int v, const location_type& l)
>    : Base (t)
>    , value (v)
>    , location (l)
>  {}
> 
>  template <typename Base>
>  calcxx_parser::basic_symbol<Base>::basic_symbol (typename Base::kind_type t, 
> const location_type& l)
>    : Base (t)
>    , value ()
>    , location (l)
>  {}
> 
> 
> The nature of my patch is basically this: instead of having duplicate 
> information in symbols and their aliases, and to be forced to resolve the 
> differences later, the symbols now contain only their intrinsic information 
> (name, location), and share with the alias a pointer to a structure 
> containing the rest (precedence, etc...).
> 
> If you have a clue as to what is causing this bug, please tell me. Otherwise, 
> if you could at least point me to the place in the code where the file is 
> generated, I'd be grateful.

This is all generated by M4 from data provided by Bison.
I suggest that you take some simple input file,
and pass it to your bison with --report=muscles.  This asks
bison to dump what information it is passing to M4.  Compare
this with what a previous version of Bison did.

Beware that these routines (generated by data/variant.hh)
iterate on the *types* of the symbols, not the symbols themselves.
But to iterate on the types of the symbol, they use a list of
symbols grouped by type.

If you read

$ ./tests/bison --trace=muscles ./examples/calc++/calc++-parser.yy

you'll see

m4_define([b4_symbol_numbers],
[[0], [1], [2], [3], [4], [5], [6], [7], [8], [9], [10], [11], [12], [13], 
[14], [15], [16]])

m4_define([b4_type_names],
[[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 12, 13, 14, 15],
[11, 16],
[10]])

which means that this grammar has 17 symbols (0 to 16), in three
groups of types.

If you want to know "who" are these symbols, read below in the output,
looking for instance at symbol number 4:

$ ./_build/48s-speed/tests/bison --trace=muscles 
_build/48s-speed/examples/calc++/calc++-parser.yy |& grep -F -A2 'symbol(4' 
m4_define([b4_symbol(4, has_type)],
[[0]])

--
m4_define([b4_symbol(4, user_number)],
[[259]])

--
m4_define([b4_symbol(4, tag)],
[["-"]])

--
m4_define([b4_symbol(4, has_destructor)],
[[0]])

--
m4_define([b4_symbol(4, id)],
[[MINUS]])

--
m4_define([b4_symbol(4, has_printer)],
[[0]])

--
m4_define([b4_symbol(4, has_id)],
[[1]])

--
m4_define([b4_symbol(4, type)],
[[]])

--
m4_define([b4_symbol(4, number)],
[[4]])

--
m4_define([b4_symbol(4, is_token)],
[[1]])



The output you read (in the *hh file) come from:

# b4_basic_symbol_constructor_declare
# -----------------------------------
# Generate a constructor declaration for basic_symbol from given type.
m4_define([b4_basic_symbol_constructor_declare],
[[
  basic_symbol (]b4_join(
          [typename Base::kind_type t],
          b4_symbol_if([$1], [has_type], const b4_symbol([$1], [type])[ v]),
          b4_locations_if([const location_type& l]))[);
]])

Which is called as:

]b4_type_foreach([b4_basic_symbol_constructor_declare])], [[


Notice the _type_ (as opposed to b4_symbol_foreach): this will iterate
on the first symbol of each list in b4_type_names, shown above.  So I
guess your b4_type_names is wrecked.

Then, you have to dive into the C code to see where and how type_names is
computed.  This communication from bison to m4 is in output.c.

/*------------------------------------------------------------------.
| Define b4_type_names, which is a list of (lists of the numbers of |
| symbols with same type-name).                                     |
`------------------------------------------------------------------*/

static void
type_names_output (FILE *out)
{
  int i;
  symbol **syms = symbols_by_type_name ();
  fputs ("m4_define([b4_type_names],\n[", out);
  for (i = 0; i < nsyms; /* nothing */)
    {
      /* The index of the first symbol of the current type-name.  */
      int i0 = i;
      fputs (i ? ",\n[" : "[", out);
      for (; i < nsyms && syms[i]->type_name == syms[i0]->type_name; ++i)
        fprintf (out, "%s%d", i != i0 ? ", " : "", syms[i]->number);
      fputs ("]", out);
    }
  fputs ("])\n\n", out);
  free (syms);
}