Re: regression in autoconf-2.62 vs. 2.61

[Ralf Wildenhues]

Hello, and sorry for the delay,

* Eric Blake wrote on Thu, Jun 05, 2008 at 11:37:01PM CEST:
> Stepan Kasal <skasal <at> redhat.com> writes:
> 
> > First, let me state that, strictly speaking, this is not a
> > regression.  The Autoconf manual says that the #undef line cannot
> > contain anything after the symbol.   And in that case, all versions
> > tested by Ralf do the same.
> 
> I agree that whether or not we change code, a doc patch is necessary.  But 
> I'm 
> still interested to see if Ralf's patch to restore saner behavior proves 
> worthwhile, before deciding whether your proposed doc patch is right.

I'm punting.  The '#define header templates' test already requires that
  #undef macro additional-stuff

removes "additional-stuff" from the input.  It may be possible to
distinguish between '/* comments */', '// C++ comments', and other
junk, but I don't think it's worth the effort, nor worth slowing
everyone else down.

> >  I thought this were the reason why these
> > comments were generally frown upon.
> 
> Comments in the config.h template file are different than normal C comments, 
> in 
> the sense that we are passing them through yet another set of tools (formerly 
> sed, now awk) to create a valid C89 header file.  So I think the autoconf 
> restriction of no comments, above and beyond the C89 requirements, is more 
> for 
> making our template conversion easy, than for avoiding invalid code.

Agreed.

> > About the case of #undef OTHER_SYMBOL being commented out, I suggest
> > to preserve that behaviour, or someone will get trapped by that
> > change and report a regression.  
> 
> I could go either way on this one.  But whichever we decide (leaving 
> OTHER_SYMBOL commented out, as currently done, or leaving it unmolested, as 
> the 
> OP requested) should be documented and tested prior to the 2.63 release.

I vote for keeping the current behavior.  Changing it introduces an
arbitrary backwards incompatibility; yet there hasn't even been a bug
report which would benefit by this.  (OTOH, there have been real,
actual, problematic bug reports which have so far generated much fewer
traffic...)

OK to commit this conservative fix?

Thanks,
Ralf

2008-06-16  Ralf Wildenhues  <address@hidden>

        Fix '#undef variable /* comment */' transform in config headers.
        * lib/autoconf/status.m4 (_AC_OUTPUT_HEADERS_PREPARE): For
        undefined preprocessor macros that are followed by a comment
        in the header template, do not create nested comments in the
        output.
        * tests/torture.at (@%:@define header templates): Extend test.
        * NEWS: Update.
        Report by Karsten Hopp <address@hidden>.

diff --git a/NEWS b/NEWS
index b2ee046..632fbaa 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,9 @@ GNU Autoconf NEWS - User visible changes.
 ** Autotest testsuites accept an option --jobs[=N] for parallel testing.
    Alternatively, testsuites can act as GNU make jobserver client.
 
+** Config header templates `#undef UNDEFINED /* comment */' do not lead to
+   nested comments any more; regression introduced in 2.62.
+
 
 * Major changes in Autoconf 2.62 (2008-04-05) [stable]
   Released by Eric Blake, based on git versions 2.61a.*.
diff --git a/lib/autoconf/status.m4 b/lib/autoconf/status.m4
index 50be77b..d5ed323 100644
--- a/lib/autoconf/status.m4
+++ b/lib/autoconf/status.m4
@@ -832,9 +832,9 @@ cat >>$CONFIG_STATUS <<_ACEOF || ac_write_fail=1
   }
   split(mac1, mac2, "(") #)
   macro = mac2[1]
+  prefix = substr(line, 1, index(line, defundef) - 1)
   if (D_is_set[macro]) {
     # Preserve the white space surrounding the "#".
-    prefix = substr(line, 1, index(line, defundef) - 1)
     print prefix "define", macro P[macro] D[macro]
     next
   } else {
@@ -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, "*/"
       next
     }
   }
diff --git a/tests/torture.at b/tests/torture.at
index c37daba..92f8d5b 100644
--- a/tests/torture.at
+++ b/tests/torture.at
@@ -497,6 +497,8 @@ AT_DATA([config.hin],
 #define str(define) \
 #define
 #define stringify(arg) str(arg)
+#undef aaa /* with comments */
+#undef not_substed /* with comments */
 ]])
 
 AT_CHECK_AUTOCONF
@@ -527,6 +529,8 @@ ARG1
 #define str(define) \
 #define
 #define stringify(arg) str(arg)
+#define aaa AAA
+/* #undef not_substed */
 ]])
 AT_CHECK([cat config.h], 0, expout)