Re: regression in autoconf-2.62 vs. 2.61

[Eric Blake]

Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de> writes:

> 
> Hello Karsten,
> 
> Thanks for the bug report.

Yes, thanks for bringing it to our attention.

> 
> > https://bugzilla.redhat.com/show_bug.cgi?id=449245 

It looks like the redhat bug was closed as not-a-bug, but I agree with Ralf's 
analysis that a simple patch on top of 2.62 would allow using this idiom again, 
and that the RedHat bug should probably be reopened to cover whatever 
resolution we get from this thread.

> > - autoconf shouldn't temper with symbols it hasn't been told to
> > substitute.
> 
> Well, that, however, is done intentionally; see this comment in
> lib/autoconf/status.m4:
>     # Replace #undef with comments.  This is necessary, for example,
>     # in the case of _POSIX_SOURCE, which is predefined and required
>     # on some systems where configure will not decide to define it.
> 
> (maybe we should document that?)

We already mention the aspect of commenting out mentioned but unused #undef 
lines, earlier in the same node:

| 
| Then you could have code like the following in `conf.h.in'.  On systems
| that have `unistd.h', `configure' defines `HAVE_UNISTD_H' to 1.  On
| other systems, the whole line is commented out (in case the system
| predefines that symbol).


> 
> Let's have a test case to see what happens:
> 
> cat >configure.in <<\EOF
> AC_INIT
> AC_CONFIG_HEADER(config.h)
> AC_DEFINE([DEFINED_SYMBOL], [1])
> if false; then
>   AC_DEFINE([UNDEFINED_SYMBOL], [1])
> fi
> AC_OUTPUT
> EOF
> cat >config.h.in <<\EOF
> #undef DEFINED_SYMBOL /* some comment */
> #undef UNDEFINED_SYMBOL /* some other comment */
> #undef DEFINED_SYMBOL
> #undef UNDEFINED_SYMBOL
> EOF

This test case would have been nicer with three cases instead of two:
DEFINED_SYMBOL encountered in AC_DEFINE and substituted during configure
UNDEFINED_SYMBOL mentioned in AC_DEFINE, but not encountered during configure
OTHER_SYMBOL not mentioned at all in configure.ac

If I understand the bug report correctly, maybe we should try to tweak things 
such that if any AC_DEFINE mentions the hook, we comment out that hook when not 
substituted, but if the symbol is not mentioned at all, we don't treat it as an 
undefined hook.  On the other hand, I don't know how complex this would be to 
implement, now that we rewrote the config.status algorithm to use awk instead 
of sed.

Meanwhile, you can work around this by using AH_VERBATIM or AT_BOTTOM to 
#include a secondary file, and place your #undef in that second file without 
worrying whether autoheader/config.status will munge it.  So blindly commenting 
ALL #undef in the template file, whether or not they are mentioned as hooks in 
configure.ac, is not an unsurmountable limitation.

So I think the best thing to do now would be a wording tweak to make it obvious 
that we intentionally comment out ALL remaining #undef in the template file, 
rather than just the ones that were hooked by AC_SUBST/AC_DEFINE but not 
substituted.

> --- 2.13 ---
> /* config.h.  Generated automatically by configure.  */
> #define DEFINED_SYMBOL 1 /* some comment */
> /* #undef UNDEFINED_SYMBOL */ /* some other comment */
> #define DEFINED_SYMBOL 1

Is this a bug?  If I remember correctly, the preprocessor is supposed to warn 
if the sequence of tokens following the redefinition of an existing macro name 
is not identical to its prior definition, and I'm not sure whether the removal 
of a comment qualifies as a different sequence of tokens.

> /* #undef UNDEFINED_SYMBOL */
> 
> --- 2.59 ---
> /* config.h.  Generated by configure.  */
> /* #undef DEFINED_SYMBOL */ /* some comment */
> /* #undef UNDEFINED_SYMBOL */ /* some other comment */
> #define DEFINED_SYMBOL 1
> /* #undef UNDEFINED_SYMBOL */

Thus I like this output slightly better - no risk of redefining an already-
defined symbol with a different sequence of tokens.

> 
> --- 2.61 ---
> /* config.h.  Generated from config.h.in by configure.  */
> #undef DEFINED_SYMBOL /* some comment */

This approach of leaving all but the last occurence of a defined hook as an 
#undef would also avoid redefining a symbol...

> #undef UNDEFINED_SYMBOL /* some other comment */

...but leaving the first instance of an undefined hook uncommented is indeed a 
bug.  On the other hand, template files generally don't mention the same hook 
multiple times.

> #define DEFINED_SYMBOL 1
> /* #undef UNDEFINED_SYMBOL */
> 
> --- 2.62 ---
> /* config.h.  Generated from config.h.in by configure.  */
> #define DEFINED_SYMBOL 1
> /* #undef UNDEFINED_SYMBOL /* some other comment */ */
> #define DEFINED_SYMBOL 1
> /* #undef UNDEFINED_SYMBOL */
> 
> The commenting out of line 2 with 2.59 looks like a bug to me.
> Likewise, the first two #undef line with 2.61 look buggy to me.
> 
> The fact that 2.62 removes the comment after the defined symbol
> looks like a QoI issue (i.e., would be nice to fix, but no error
> if not).  The nested comment, however, is clearly undesirable.

Yes - (not) stripping the comment is only QoI, since it should not affect the 
generated code.

> 
> So here's a patch for a simple fix, to generates this instead:
> --- 
> /* config.h.  Generated from config.h.in by configure.  */
> #define DEFINED_SYMBOL 1
> /* undef UNDEFINED_SYMBOL */
> #define DEFINED_SYMBOL 1
> /* undef UNDEFINED_SYMBOL */
> ---
> 
> Question is if that's good enough.  Judging from autoconf history as
> seen above, comments have been problematic ever since 2.59.  I haven't
> tried incomplete (multi-line) comments, or C++ style comments.

With C++ comments, it should not matter whether we follow the 2.13 approach of 
inserting */ after the macro name and before the rest of the line, or your 
proposed patch of truncation, since all of these are correctly commented:
/* #undef foo */ // comment
/* #undef foo // comment */
/* #undef foo */

But if I understand things correctly, multi-line comments suffer the same fate 
in 2.62 (except that the first */ comes from the config.status substitution, 
and the second from the template, rather than in the one-line case where the 
first */ is from the template).

#undef foo /* one
two */

=>

#undef foo /* one */
two */

For this case, then, the 2.13/2.59 approach of putting the closing */ after the 
macro name but prior to all other contents of the line is better than 
truncating the line after the macro name and first */ (truncating the line will 
leave the incomplete comment conclusion on the next line):

/* #undef foo */
two */

vs.

/* #undef foo */ /* one
two */


On the other hand, only the 2.62 behavior and your proposed truncation handles 
the case of the user doing invalid preprocessor constructs in the template, 
such as this:

#undef foo not valid

With 2.13 and 2.59, this would result in the syntax error after preprocessing:

/* #undef foo */ not valid

and with 2.61, the first instance (if listed multiple times) would result in a 
preprocessor syntax error:

#undef foo not valid


> 
> I'm willing to add a test case after we've decided what to be desirable.
> 
> @@ -842,7 +842,7 @@ cat >>$CONFIG_STATUS <<_ACEOF || ac_write_fail=1
>      # in the case of _POSIX_SOURCE, which is predefined and required
>      # on some systems where configure will not decide to define it.
>      if (defundef == "undef") {
> -      print "/*", line, "*/"
> +      print "/*", prefix defundef, macro, "*/"

How hard would it be to split the original line into the macro name and the 
rest of the line, such that we output

/* #undef foo */ /* comment */

similar to 2.13?

-- 
Eric Blake