autoconf-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: FYI: loop in libtool m4 macros


From: Ralf Wildenhues
Subject: Re: FYI: loop in libtool m4 macros
Date: Wed, 22 Jun 2005 18:45:35 +0200
User-agent: Mutt/1.4.1i

[ Unintentional private reply? ]

Hi Stepan,

* Stepan Kasal wrote on Wed, Jun 22, 2005 at 02:22:38PM CEST:
> 
> > > 1)
> > > m4/ltsugar.m4:
> > > 
> > > If you want to be compatible with both old and new m4_cdr, you have to
> > > use:
> > >   m4_if([$2], [[]], [],
> > >         [$2], [], [],
> > >         [lt_join(...
> > > 
> > > I apologize for the inconvinience, but I believe this was a good step
> > > in the long time perspective.
> > 
> > Well, the interface was not published, so I guess Libtool can't complain
> > too much.  Your change looks much better than my hack.  Thanks.
> 
> No, your change is corrent, mine is a hack.
> 
> Correct code is:
> m4_if([$2], [], [], [lt_join(...
> 
> or, equivalently
> m4_ifval([$2], [lt_join(...
> 
> But this _correct_ code doesn't work with previous buggy versions of m4_cdr.

ACK.

> To make the code portable, there are two ways:
> a) my hack cited above

OK.

> b) m4_define([my_cdr], ...) -- copy the definition of m4_cdr from current
>    autoconf
>    Then use my_cdr instead of m4_cdr in your code.

This is what I ended up doing:
http://lists.gnu.org/archive/html/libtool-patches/2005-06/msg00121.html

> I think b) is better.  Would you agree?

b is fine with me, it's already in place.  :)
a would have been shorter.  But b is also more straightforward to rip
out once Autoconf-2.60 is established (and m4_cdr published).  So let's
leave it like it is currently.

> 2)
> > |   m4_quote(m4_default([$1], [[, ]])
> 
> m4_quote doesn't do what it should, as it uses [$*].
> That means that all whitespace after commas is stripped.

Is this a bug in m4_quote?   Is this an unfixable bug (because that
thing is published and used)?

> It's better to add a pair  of quotes in advance.
> 
> The above code is equivalent to:
> m4_quote(m4_ifval([$1], [$1], [[, ]]))
> So if $1 is " a, b", both spaces will be removed.

> I suggested
>       m4_ifval([$1], [[$1]], [[, ]])
> which preserves whitespace in both cases.

Hmm.  This idiom is used in several places in Libtool HEAD's m4 files.
Just replacing them with your suggestion leads to brackets in the
resulting `configure'.  So there is either some more workarounds to make
up for the different style, or some other misunderstanding.

See the patch below for what I have tried (and does not work), and bear
with me, I still read m4 like a foreign language I just started.

> > > 3)
> > > m4/ltoptions.m4
> > > >  m4_define([_LT_SET_OPTIONS],
> > > > -[AC_FOREACH([_LT_Option], [$1], [...]
> > > > +[m4_if([$1], [], [],
> > > > +       [AC_FOREACH([_LT_Option], [$1], [...]
> > > 
> > > This change is a workaround for a bug I introduced.
> > > Please remove the workaround, as the bug is now fixed in the Autoconf CVS.
> > 
> > When was the bug introduced?  If only with the m4_cdr change, then OK.
> 
> Yes, the bug was introduced when I fixed m4_cdr and forgot to fix m4_strip.
> So the bug lived only for very short in the CVS version.

Thank you.  I will remove this patch then.

Regards,
Ralf

This part seems OK:

Index: m4/ltoptions.m4
===================================================================
RCS file: /cvsroot/libtool/libtool/m4/ltoptions.m4,v
retrieving revision 1.11
diff -u -r1.11 ltoptions.m4
--- m4/ltoptions.m4     11 Jun 2005 11:11:45 -0000      1.11
+++ m4/ltoptions.m4     22 Jun 2005 13:44:05 -0000
@@ -51,14 +51,12 @@
 # dispatch to that macro; otherwise complain about the unknown option
 # and exit.
 m4_define([_LT_SET_OPTIONS],
-[m4_if([$1], [], [],
-       [AC_FOREACH([_LT_Option], [$1],
-                  [_LT_SET_OPTION(_LT_Option)
-                       m4_ifdef(_LT_MANGLE_DEFUN(_LT_Option),
-                                _LT_MANGLE_DEFUN(_LT_Option),
-               [m4_fatal([Unknown option `]_LT_Option[' to 
LT][_INIT_LIBTOOL])])
-                  ])dnl
-       ])
+[AC_FOREACH([_LT_Option], [$1],
+    [_LT_SET_OPTION(_LT_Option)
+    m4_ifdef(_LT_MANGLE_DEFUN(_LT_Option),
+            _LT_MANGLE_DEFUN(_LT_Option),
+       [m4_fatal([Unknown option `]_LT_Option[' to LT][_INIT_LIBTOOL])])
+    ])dnl
 dnl
 dnl Simply set some default values (i.e off) if boolean options were not
 dnl specified:



This patch is not OK:

Index: m4/libtool.m4
===================================================================
RCS file: /cvsroot/libtool/libtool/m4/libtool.m4,v
retrieving revision 1.197
diff -u -r1.197 libtool.m4
--- m4/libtool.m4       18 Jun 2005 16:49:52 -0000      1.197
+++ m4/libtool.m4       22 Jun 2005 13:44:05 -0000
@@ -357,7 +357,7 @@
 # lt_decl_varnames_tagged([SEPARATOR], [VARNAME1...])
 # ----------------------------------------------------
 m4_define([lt_decl_varnames_tagged],
-[_$0(m4_quote(m4_default([$1], [[, ]])),
+[_$0(m4_ifval([$1], [[$1]], [[, ]]),
      m4_quote(m4_if([$2], [],
                     m4_quote(lt_decl_tag_varnames),
                  m4_quote(m4_shift($@)))),
@@ -368,7 +368,7 @@
 # lt_decl_all_varnames([SEPARATOR], [VARNAME1...])
 # ------------------------------------------------
 m4_define([lt_decl_all_varnames],
-[_$0(m4_quote(m4_default([$1], [[, ]])),
+[_$0(m4_ifval([$1], [[$1]], [[, ]]),
      m4_if([$2], [],
           m4_quote(lt_decl_varnames),
        m4_quote(m4_shift($@))))[]dnl
Index: m4/ltsugar.m4
===================================================================
RCS file: /cvsroot/libtool/libtool/m4/ltsugar.m4,v
retrieving revision 1.5
diff -u -r1.5 ltsugar.m4
--- m4/ltsugar.m4       17 Jun 2005 12:42:34 -0000      1.5
+++ m4/ltsugar.m4       22 Jun 2005 13:44:05 -0000
@@ -50,14 +50,14 @@
 # Produce a SEP delimited list of all paired combinations of elements of
 # PREFIX-LIST with SUFFIX1 through SUFFIXn.  Each element of the list
 # has the form PREFIXmINFIXSUFFIXn.
 m4_define([lt_combine],
 [m4_if([$2], [], [],
-       [lt_join(m4_quote(m4_default([$1], [, ])),
+       [lt_join(m4_ifval([$1], [[$1]], [[, ]]),
                _$0([$1], lt_car($2)[$3], m4_shiftn(3, $@)),
                $0([$1], lt_cdr($2), m4_shiftn(2, $@)))])])
 m4_define([_lt_combine],
 [m4_if([$3], [], [],
-       [lt_join(m4_quote(m4_default([$1], [, ])),
+       [lt_join(m4_ifval([$1], [[$1]], [[, ]]),
                [$2$3],
                $0([$1], [$2], m4_shiftn(3, $@)))])[]dnl
 ])
@@ -107,7 +108,7 @@
 # ------------------------------------------------------------
 m4_define([lt_dict_filter],
 [m4_if([$5], [], [],
-  [lt_join(m4_quote(m4_default([$4], [[, ]])),
+  [lt_join(m4_ifval([$4], [[$4]], [[, ]]),
           m4_quote(lt_if_dict_fetch([$1], [$5], [$2], [$3], [$5])),
           m4_quote($0([$1], [$2], [$3], [$4], m4_shiftn(5, $@))))])dnl
 ])




reply via email to

[Prev in Thread] Current Thread [Next in Thread]