Re: iterating over arguments

[Eric Blake]

Sam Steingold <sds <at> gnu.org> writes:

> > And if clisp ever comes up with a feature that resembles an m4 macro name, 
or 
> > contains (), ',', [], or $, (for example, if you added a "dnl" feature to 
> > clisp), then you should consider a more robust solution
> 
> OH HORROR!!!

So just be sure you cover your bases, and document that 
CL_CLISP_REQUIRE_FEATURE requires that its arguments have no special characters 
and not match macro names.  Really, the safety of m4_defn is only necessary if 
you plan to support arbitrary arguments, but in your case, you have the context 
that you know the finite set of feature names of clisp and what people are 
likely to ever want to pass to your macro, including the fact that clisp 
feature names should never collide with m4 macro names.

> OK, if you are still with me, how about the following extra tweak:

Yep, still here.

> as you probably know, the CL reader is (by default) case-converting (the
> default case being upcase), so "#+FOO" and "#+foo" have the same meaning.
> This makes me want to use upcase for both messages and variables names, so 
that
> CL_CLISP([ffi]) and CL_CLISP([FFI]) will not define two separate variables
> cl_cv_clisp_ffi and cl_cv_clisp_FFI which answer the same question.
> So, I applied this patch:
> and it seems to be doing what I want.
> is it OK?

One definition of OK is that it does what you want, so by that definition, yes, 
you can stop hacking on this macro ;)  Or, you can read on for more of my 
thoughts...

> (drop m4_popdef, replace m4_pushdef with m4_define)

m4_pushdef/m4_popdef is nicer than m4_define, in that your use of the macro 
name becomes a local variable (you aren't corrupting state if anything else in 
the configure.ac happened to use the same name for an unrelated macro).  But 
m4_define works all right if you have no reason to suspect that the macro name 
will ever collide.

>   m4_foreach_w([cl_feat], [$1],
> -[m4_pushdef([CL_FEAT], m4_toupper(cl_feat))dnl
> -AC_CACHE_CHECK([for CL_FEAT in CLISP], [cl_cv_clisp_]cl_feat,
> +[m4_define([cl_feat], m4_toupper(cl_feat))dnl
> +AC_CACHE_CHECK([for cl_feat in CLISP], [cl_cv_clisp_]cl_feat,

But if you are ALWAYS going to use the upper case version, then why not skip 
the m4_define altogether and do the upper-casing up front?

m4_foreach_w([cl_feat], m4_toupper([$1]),
[AC_CACHE_CHECK([for cl_feat in CLISP], ....

>   [CLISP_SET([cl_cv_clisp_]cl_feat,[[#+]]cl_feat[[ "yes" #-]]cl_feat
[[ "no"]])])
> -test $cl_cv_clisp_[]cl_feat = no && AC_MSG_ERROR([no ]CL_FEAT[ in CLISP])
> -m4_popdef([CL_FEAT])])])
> +test $cl_cv_clisp_[]cl_feat = no && AC_MSG_ERROR([no ]cl_feat[ in CLISP])

Do you really want the cache variable to be named upper case cl_cv_clisp_FFI?  
There's nothing wrong with that; it's just a bit harder to type if someone 
wants to prime the cache.  On the other hand, using all caps everywhere makes 
your maintenance burden less, so I see no reason to go out of your way to avoid 
upper case in a cache variable name.

> Thanks.

Not a problem - it's been a fun conversation, trying to figure out why our back-
of-the-envelope solutions weren't perfect, and a relief to know that we weren't 
too far off base in the original proposal (an extra quote here, a missing quote 
there, but mostly correct).  Plus, it helped us fix a latent quoting bug in 
m4_toupper, even though that bug will never interfere with your planned usage 
pattern of never passing macro names to CL_CLISP_REQUIRE_FEATURE in the first 
place.

-- 
Eric Blake