Le 30 mars 2012 à 11:18, Bruno Haible a écrit :
> Hi Akim,
>
>>> This patch appears broken to me:
>>> - On one hand it augments CPPFLAGS without ever setting it back.
>>> - On the other hand it saves and restores CFLAGS or CXXFLAGS but
>>> without temporarily modifying its value.
>>>
>>> Can you please provide a fix? Gnulib macros should in general *not*
>>> have lasting effects on $CPPFLAGS, $CFLAGS, $CXXFLAGS.
>>
>> Of course, I'm sorry for this error :/
>
> Thanks, I have applied the correction 0001-warnings.m4-fix-errors.patch,
> but without the unnecessary removal of double-quotes.
Should I do that in a latter patch? Useless double quotes,
for instance in assignments and "case" construct, are a
bad habit, imho, since they lead to useless problems when
one wants to use backquotes and needs double quotes inside.
>> 0002-warnings.m4-exhibit-an-if-else-interface.patch
>
> What is the point of this patch? Those who want to assign to a different
> variable than WARN_CFLAGS can already do so through gl_WARN_ADD. Your
> patch looks like unneeded complexity to me.
On the other hand it provides a more classical interface,
if one wants to issue a warning or a error message if some
option is not supported. Besides, it prepares for more
changes if we want to make more complex check on the way
the compiler supports the option (as I suggested earlier).
It allows to clear separate the test part from the action
part. More lines, but I would argue that it is not
added complexity, but are less complexity.
It would allow to remove some useless duplication in gnulib.
Say manywarnings:
dnl First, check -W -Werror -Wno-missing-field-initializers is supported
dnl with the current $CC $CFLAGS $CPPFLAGS.
AC_MSG_CHECKING([whether -Wno-missing-field-initializers is supported])
AC_CACHE_VAL([gl_cv_cc_nomfi_supported], [
gl_save_CFLAGS="$CFLAGS"
CFLAGS="$CFLAGS -W -Werror -Wno-missing-field-initializers"
AC_COMPILE_IFELSE(
[AC_LANG_PROGRAM([[]], [[]])],
[gl_cv_cc_nomfi_supported=yes],
[gl_cv_cc_nomfi_supported=no])
CFLAGS="$gl_save_CFLAGS"])
AC_MSG_RESULT([$gl_cv_cc_nomfi_supported])
Actually, it would be even helpful to be able to provide
the code to compile, instead of just AC_LANG_PROGRAM.
> Also, what is the point of removing the third argument of the AS_LITERAL_IF
> invocation? IMO this makes the code harder to understand.
I'm not used to leave an
else
{}
to my ifs :) But if that's what you want, sure, I can
restore it.
It's not customary in Autoconf to leave useless empty arguments.
