Re: m4_map_sep bug (introduced by 2005-06-06 Autoconf patch) breaks Bison

[Stepan Kasal]

Hello,

On Wed, Oct 05, 2005 at 04:20:10PM -0700, Paul Eggert wrote:
> In <http://lists.gnu.org/archive/html/bug-bison/2005-10/msg00004.html>
> "John P. Hartmann" <address@hidden> writes:
> 
> > http://lists.gnu.org/archive/html/autoconf-patches/2005-06/txtRXb8U1rojj.txt
> > ... The change to m4sugar generates a spurious semicolon before the
> > function body (opening {) when a function takes no parameters.
> >
> > This causes bison not to build.
> 
> Thanks for reporting this.  I have installed the following patch into
> both GNU Autoconf and Bison, in an attempt to work around the problem.
> 
> Stepan, can you please double-check this patch?  It still feels
> vaguely wrong.

well, I cannot agree with your patch, and I'd like to revert it.
But lets resolve the bison problem first.

And even before it, some explanations:
Let's have a macro FOO(LIST), where the first parameter is an empty list.
If you want to pass a two-member list, the fully quoted notation is:
        FOO([[one], [two]])
... thus one member list would be:
        FOO([[one]])
... and an empty list would be:
        FOO([])

With this, if the macro FOO want's to checked whether the LIST is empty,
it has to do:
        m4_if([$1], [], empty, non-empty)
or
        m4_ifval([$1], non-empty, empty)

The empty list is represented by an empty string.

This is a simple and consistent definition.

Before my 2005-06-06 change, m4sugar didn't follow this definition, and
the empty list was often represented by the string "[]".

After the 2005-06-06, things are consitent, but unfortunately the interface
has changed.  In many cases, the change is correct, I think.
But in other cases, the caller has to be fixed to comply with this new,
clean definition.

Your patch returns to the inconsistent interface, on the top of the correct
internals.  I don't agree with it.

I looked at some of the uses of m4_map and m4_map_sep in bison:

1) c.m4 and lalr1.cc contain:
        m4_map([b4_symbol_actions], m4_defn([b4_symbol_destructors]))
        m4_map([b4_symbol_actions], m4_defn([b4_symbol_printers]))

Is it possible that there are no symbols?  (I don't understand the
context, sorry.)  If it were possible, then symbol_destructors_output()
would print
        m4_define([b4_symbol_destructors], \n[])\n\n


What should the m4_map expand to?  With my definition, the expansion is
empty.  With the previous definition (or your modification), one would
get
        b4_symbol_actions([])

But I'm afraid that nsyms==0 is not possible, so this cannot happen,
anyway.

2) Then I investigated b4_token_defines, b4_token_enums, and
b4_token_enums_defines, in c.m4. (And b4_token_enums in c++.m4.)

There is one problem there: though the comment says

        # b4_token_enums(LIST-OF-PAIRS-TOKEN-NAME-TOKEN-NUMBER)

which seems to indicate that the macros take an m4 list as the first
and only parameter, they in fact take the members of the list as
separate parameters.  Thus the comment should read:

        # b4_token_enums(PAIR-TOKEN-NAME-TOKEN-NUMBER, ...)

But this interface is unfortunate, because it is then impossible to
distinguish between the empty list and the "[]" one-member list.

I think the code would become cleaner if the implementation were
changed to match the current declaration in the comment.
(All callers have to be changed, of course.)

3) A note b4_parse_param_decl in c++.m4 contains
        m4_map_sep(.., .., [b4_parse_param])
while it should contain
        m4_map_sep(.., .., m4_defn([b4_parse_param]))

4) Then there are some other usages of m4_map_sep, which can be easily
fixed.  After a quick glance, it seems it would be cleaner to change
their interface from
        # b4_c_ansi_formals([DECL1, NAME1], ...)
(which is consistent with the implementation here), to
        # b4_c_ansi_formals(DECL-NAME-LIST)
        # =================================
        # DECL-NAME-LIST is an m4 list of this form:
        #       [DECL1, NAME1], ...

Again, having an m4 list as one parameter is a good idea if all the
macro does is that it passes the whole list to m4_map_sep.

To sum up, it seems that I'll come back with quite a huge bison cleanup
patch, when I find some time.

Have a nice day,
        Stepan Kasal