Re: regression in autoconf-2.62 vs. 2.61

[Ralf Wildenhues]

Hi Eric,

thanks for the quick review.

* Eric Blake wrote on Wed, Jun 04, 2008 at 10:19:40PM CEST:
> Ralf Wildenhues <Ralf.Wildenhues <at> gmx.de> writes:
> >     # 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).

Oh, thanks, I overlooked that.

> > 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

I tried OTHER_SYMBOL; it behaves exactly the same as UNDEFINED_SYMBOL.

> 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.

FWIW, we never did that before.  But the move to awk certainly made this
a lot easier, with sed it would've been almost impossible.

> 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.

Agreed.

> 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.

Or fix it.

> > --- 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.

I'd have to look that up; but anyway, I don't think this is a big
problem, as users are unlikely to define things twice.

> > --- 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.

Well, no.  The use case is that only the first line exists; in that
case, autoconf output is buggy.

Maybe my test case was not good; there is never any state saved across
lines; so, the second substitution does not remember any previous ones.

> > --- 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...

It's not the last one; it's that presence of the comment prevents a
successful substitution.

> > #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.

Exactly.

> > #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):

Agreed.

> 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

We should not strive to support that.  That's a user bug.
If we need to make that clearer, then we should do so.

> > -      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?

I'll look into it.

Cheers,
Ralf