Re: AC_C_BIGENDIAN answers "universal" on powerpc-aix

[Ralf Wildenhues]

Hi Stepan,

Thanks for your review of the patch.

* Stepan Kasal wrote on Tue, Aug 26, 2008 at 04:25:36PM CEST:
> 
> 1)
> It should be documented that the default action-if-universal works only
> with autoheader.  So, instead of
[...]
> I propose:
> 
> ``And finally, the default for @var{action-if-universal} is to ensure
> that `config header' defines @samp{WORDS_BIGENDIAN), if and only if
> the code being generated is destined for big-endian architecture.
> This feature works only if autoheader is used.''

Fine with me.

> 2)
> The code contains check m4_ifdef([AH_HEADER], ...)
> I'm afraid this check is not correct, because the config header may
> be declared after AC_C_BIGENDIAN.  (For example, all config foos may
> be declared just before AC_OUTPUT.)
> 
> So instead of
> 
> if AH_HEADER defined || ACTION_IF_UNIVERSAL given
> then
>    set ac_cv_c_bigendian=universal if universal detected
> else
>    AC_DIAGNOSE([obsolete], [AC_C_BIGENDIAN suggests AC_CONFIG_HEADERS])
> fi
> 
> I propose to perform to do (unconditionally):
>     set ac_cv_c_bigendian=universal if universal detected

OK with me.  You could still emit the AC_DIAGNOSE, but wrapped in a
AC_CONFIG_COMMANDS_PRE (Libtool's ltdl.m4 does similar, while checking
AH_HEADER).  What do you think?

> 3)
> Moreover, I would move the universal-detection code to a separate
> macro, perhaps _AC_C_BIGENDIAN_UNIVERSAL.

I'm rather indifferent about this.  So, my gut reaction would be to
not change it now if it doesn't fix a bug.

> The comment above its
> definition should mention prominently that the failure wegths are
> very asymmetrical: false positives brek "normal" builds and are
> much bigger shame that "incomplete universal binaries support."

You assume that we will mess this macro up again?

> 4)
> (The warning could be implmented in a macro called at AC_OUTPUT time,
> but I do not think it is _needed_ for 2.63.  The warning cannot catch
> all situations, anyway: AC_CONFIG_HEADERS does not necessarily mean
> autoheader is used in this project.)

See above.

> 5)
> It is important that "AC_APPLE_UNIVERSAL_BUILD" < "WORDS_BIGENDIAN"
> so that the macros fall into the config header in the right order.
> Perhaps this should be mentioned in a comment above 
> AC_DEFINE([AC_APPLE_UNIVERSAL_BUILD],...)

Good point.

> 6)
> The template contains (in the branch when AC_APPLE_UNIVERSAL_BUILD is
> not defined):
> 
> # ifndef WORDS_BIGENDIAN
> #  undef WORDS_BIGENDIAN
> # endif
> 
> But I see no need for this "#ifndef"; the #undef line is equivalent
> to all the other #undef lines in the header template.
> WORDS_BIGENDIAN is not a builtin macro and config header is not
> allowed to be included twice.
> So I propose to replace the above three lined by a mere #undef.

If that 'undef' gets mangled to be a 'define', then the difference
matters, though.  So please do not change this.

> 7)
> A cheeky proposal:

> Yes, this does not preserve backward compatibility, because it
> defines WORDS_BIGENDIAN even when action-if-bigendian is specified.
> But does that really matter?

No breaking of backward compatibility.

WORDS_BIGENDIAN is in the user's namespace, and with your proposal,
there is no way not to invade that.

Users that need WORDS_BIGENDIAN plus another action to take place,
can use #ifdef WORDS_BIGENDIAN later, or can use the ac_cv_ variable.

Cheers,
Ralf