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 _______________________________________________ Autoconf mailing list Autoconf@xxxxxxx http://lists.gnu.org/mailman/listinfo/autoconf
