bug-gnulib
[Top][All Lists]
Advanced

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

Re: m4: use AC_SUBST with two arguments when applicable


From: Akim Demaille
Subject: Re: m4: use AC_SUBST with two arguments when applicable
Date: Sat, 9 May 2020 19:45:29 +0200

Hi Bruno,

> Le 9 mai 2020 à 17:25, Bruno Haible <address@hidden> a écrit :
> 
> Hi Akim,
> 
>>  AC_SUBST([GNULIB_ISWBLANK], [0])
>>  AC_SUBST([GNULIB_ISWDIGIT], [0])
>>  AC_SUBST([GNULIB_ISWXDIGIT], [0])
>>  AC_SUBST([GNULIB_WCTYPE], [0])
>>  AC_SUBST([GNULIB_ISWCTYPE], [0])
>>  AC_SUBST([GNULIB_WCTRANS], [0])
>>  AC_SUBST([GNULIB_TOWCTRANS], [0])
> 
> No, no, noooooo!! This would make the *.m4 code even harder to maintain.

:)

>>  gl_SUBSTS(
>>    [[GNULIB_ISWBLANK], [0]],
>>    [[GNULIB_ISWDIGIT], [0]],
>>    [[GNULIB_ISWXDIGIT], [0]],
>>    [[GNULIB_WCTYPE], [0]],
>>    [[GNULIB_ISWCTYPE], [0]],
>>    [[GNULIB_WCTRANS], [0]],
>>    [[GNULIB_TOWCTRANS], [0]]
>>  )
> 
> Eeek! This is even worse!!!

:)

> Really, you need to consider two things when proposing new coding patterns:
> 
> 1) The semantics.
> 2) The way a programmer works with the code on a daily basis.
> 
> 1) Let's take an example: HAVE_OPENDIR. See how it's referenced in the *.m4
> files:
> 
> $ grep -rw HAVE_OPENDIR m4 modules
> m4/opendir.m4:    HAVE_OPENDIR=0
> m4/opendir.m4:      if test $HAVE_OPENDIR = 1; then
> m4/opendir.m4:  case $host_os,$HAVE_OPENDIR in
> m4/dirent_h.m4:  HAVE_OPENDIR=1;       AC_SUBST([HAVE_OPENDIR])
> modules/opendir:filename        [test $HAVE_OPENDIR = 0 || test 
> $REPLACE_OPENDIR = 1]
> modules/opendir:unistd          [test $HAVE_OPENDIR = 0 || test 
> $REPLACE_OPENDIR = 1]
> modules/opendir:closedir        [test $HAVE_OPENDIR = 0 || test 
> $REPLACE_OPENDIR = 1]
> modules/opendir:dirfd           [test $HAVE_OPENDIR = 0 || test 
> $REPLACE_OPENDIR = 1]
> modules/opendir:if test $HAVE_OPENDIR = 0 || test $REPLACE_OPENDIR = 1; then
> modules/dirent:       -e 's/@''HAVE_OPENDIR''@/$(HAVE_OPENDIR)/g' \
> 
> What you see is:
>  - 2 assignments, at different places (even in different files),
>  - 2 references as shell variables in *.m4 files,
>  - more references as shell variables in the modules description,
>  - 1 AC_SUBST, so it becomes usable as @HAVE_OPENDIR@ in *.h files.
> 
> What you propose is
>  - arbitrary, because it selects one out of several assignments, to
>    combine them with AC_SUBST,
>  - counter-maintainable, because it hides the fact that one of these
>    is a variable assignment. When I have a shell variable, I want to
>    have *all* assignments done through the VAR=... syntax and *all*
>    references through $VAR syntax.

If what your pattern of work is grep foo=, then we could have

 gl_SUBSTS(
   GNULIB_ISWBLANK=0
   GNULIB_ISWDIGIT=0
   GNULIB_ISWXDIGIT=0
   GNULIB_WCTYPE=0
   GNULIB_ISWCTYPE=0
   GNULIB_WCTRANS=0
   GNULIB_TOWCTRANS=0
 )

But the current solution where you repeat things is not something I like.  
You're the boss, granted, your taste matters more than mine, granted.  But I 
dislike these repetitive and tedious lines.

> 2) I work with this code on a daily basis. Often I need to locate all
>   assignments. A 'grep -rw HAVE_OPENDIR=' does the job. What you
>   propose, would force me to search for
>     1. the 'HAVE_OPENDIR=' pattern,
>     2. the 'AC_SUBST([HAVE_OPENDIR]' pattern,
>   thus effectively doubling the time I need to make such a search.
> 
> And finally, your proposal with gl_SUBSTS groups things together that
> are semantically unrelated. There is no relation between GNULIB_ISWBLANK
> and GNULIB_WCTYPE. Therefore it is wrong to group them together.

The grouping is already there, just with more boilerplate.


> Most probably, you wanted to reduce the complexity of the *.m4 code by
> making it smaller?
> 
> A laudible goal, but unfortunately you have a totally wrong complexity
> measure.

Let's say that I would not put it this way...  But I agree we disagree.

> The complexity comes from the fact that there are different
> assignment in different files. But you can't reduce that complexity
> (or at least, I see no way of doing that).
> 
> --- --- --- ---
> 
> IF you want to reduce complexity in relation to AC_SUBST, then add
> a variant AC_SUBST_CONST in such a way that
>  - AC_SUBST_CONST(VAR) registers a substitution of VAR, like AC_SUBST
>    does,
>  - AC_SUBST_CONST(VAR) asserts that VAR has already its value at this
>    point,
>  - assignments to VAR that occur later are forbidden (like with 'const'
>    variables in C).
> 
> This would reduce complexity, because it guarantees that the value is
> already known, which - by the way AC_REQUIRE works - means that no other
> file can modify the value.
> 
> For the maintainer, in those cases where AC_SUBST_CONST is applicable
> (e.g. HAVE_GLOB_H), it would greatly reduce the scope in which I have to
> look for assignments to the variable.
> 
> That's how you reduce complexity: Take away irrelevant freedoms, and
> make things more "functional" or "declarative" instead of "procedural".

I'm looking for means to avoid repetitions, and boilerplate.  This does
not.

Checking that the assigned value is "const" is nice though.  Yet I would
still prefer AC_SUBST_CONST([FOO=1]) than all these error prone repetitions.


reply via email to

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