autom4te partly swallows m4_pattern_forbid multi-line comments

IF YOU HAVE A COMMAND LIKE

M4_PATTERN_FORBID([SOME_PATTERN], [SOME
MULTI-LINE
COMMENT])

IN YOUR CONFIGURE.AC, THE ERROR MESSAGE WILL ONLY
CONTAIN THE COMMENT'S FIRST LINE.

MY ORIGINAL TEST CASE WAS

DNL CONFIGURE.AC - PROCESS WITH AUTORECONF
AC_INIT([AUTOCONF M4_PATTERN_FORBID ERROR MESSAGE BUG], [0],
[], [])
M4_PATTERN_FORBID([FOOBAR], [LINE 1 OF ERROR MESSAGE
LINE 2 OF ERROR MESSAGE
LINE 3 OF ERROR MESSAGE])
FOOBAR
AC_OUTPUT

BUT THIS CAN BE REDUCED TO

DNL CONFIGURE.AC - PROCESS WITH AUTOCONF (WITHOUT
"RE")
AS_INIT
M4_PATTERN_FORBID([FOOBAR], [LINE 1 OF ERROR MESSAGE
LINE 2 OF ERROR MESSAGE
LINE 3 OF ERROR MESSAGE])
FOOBAR

IN BOTH TEST CASES, THE OUTPUT WILL LOOK LIKE

CONFIGURE.AC:2: ERROR: LINE 1 OF COMMENT                    
                                                            
             
      IF THIS TOKEN AND OTHERS ARE LEGITIMATE, PLEASE USE
M4_PATTERN_ALLOW.
      SEE THE AUTOCONF DOCUMENTATION.

IT APPEARS AUTOM4TE CALLS M4 AND HAS M4 STORE ALL
M4_PATTERN_ALLOW AND M4_PATTERN_FORBID INVOCATIONS FROM
CONFIGURE.AC INTO A FILE $TMP/PATTERNS WHICH CONSISTS OF
LINES LIKE:

FORBID:^AS_BLAH$:
FORBID:FOOBAR:SPECIAL COMMENT
ALLOW:AS_MEH

IN THE CASE OF A MULTI-LINE COMMENT, THIS WILL END UP
LOOKING LIKE

FORBID:^AS_BLAH$:
FORBID:FOOBAR:LINE 1 OF ERROR MESSAGE
LINE 2 OF ERROR MESSAGE
LINE 3 OF ERROR MESSAGE
ALLOW:AS_MEH

AND AUTOM4TE'S SUB HANDLE_OUTPUT() WILL IGNORE ALL LINES
NOT STARTING WITH EITHER 'ALLOW:' OR 'FORBID:'.

I CAN SEE THESE POTENTIAL SOLUTIONS:

  0. NOTABUG. UNDOCUMENTED BEHAVIOUR.
  1. LET M4 CREATE THE $TMP/PATTERNS FILE CONVERTING
     MULTI-LINE COMMENTS INTO AN ESCAPED ONE-LINE FORM
     AND LET AUTOM4TE LATER UNESCAPE THEM.
  2. LET M4 CREATE THE $TMP/PATTERNS FILE AS-IS, BUT HAVE
     AUTOM4TE RE-ASSEMBLE MULTI-LINE COMMENTS.

I WILL REPLY TO THIS MAIL WITH A QUICK-AND-DIRTY PATCH
IMPLEMENTING 2.

Let m4 create $tmp/patterns with comments spread over
multiple lines as before, but have autom4te reassemble
multi-line comments.

Additionally, add tests to check functionality.

An alternative solution would be to let autom4te have
m4 convert multi-line comments to an escaped one-line
form and let autom4te later unescape them.

---
 bin/autom4te.in |   52
+++++++++++++++++++++++++++++++++++++++++-
 tests/tools.at  |   67
+++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 117 insertions(+), 2 deletions(-)

diff --git a/bin/autom4te.in b/bin/autom4te.in
index 685df41..eb7136a 100644
--- a/bin/autom4te.in
+++ b/bin/autom4te.in
 -524,6
+524,55  EOF
 }
 
 
+# forbid_message_filter (PATTERNS)
+# ---------------------------------
+# Like
+#   map { /^forbid:([^:]+):.+$/ => /^forbid:[^:]+:(.+)$/
} PATTERNS;
+# but joining multi-line messages.
+sub forbid_message_filter ()
+{
+  my patterns = _;
+  my %forbidden;
+  my $pattern;
+  my $lastpat;
+  my $accu;
+  foreach $pattern (patterns) {
+    verb "parsing pattern: $pattern";
+    if ($pattern =~ /^forbid:([^:]+):(.+)$/) {
+      my ($pat, $msg) = ($1, $2);
+      verb "parsing forbid pattern: $pat, $msg";
+      if (defined $lastpat) { # flush output buffer
+	verb "flushing: $lastpat => $accu";
+	$forbidden{$lastpat} = $accu;
+	undef $lastpat;
+      }
+      $lastpat = $pat;
+      $accu = $msg;
+    } elsif ($pattern =~ /^allow:([^:]+)/) {
+      my $pat = $1;
+      verb "parsing allow pattern: $pat";
+      if (defined $lastpat) { # flush output buffer
+	verb "flushing: $lastpat => $accu";
+	$forbidden{$lastpat} = $accu;
+	undef $lastpat;
+      }
+    } else {
+      verb "parsing "other" pattern:
$pattern";
+      if (defined $lastpat) {
+        $accu = $accu . "n" . $pattern;
+	verb "appending, now accu: $accu";
+      }
+    }
+  }
+  if (defined $lastpat) { # flush output buffer
+    verb "flushing: $lastpat => $accu";
+    $forbidden{$lastpat} = $accu;
+    undef $lastpat;
+  }
+  return %forbidden;
+}
+
+
 # handle_output ($REQ, $OUTPUT)
 # -----------------------------
 # Run m4 on the input files, perform quadrigraphs
substitution, check for
 -540,8
+589,7  sub handle_output ($$)
 		  'm4_pattern_allow'  => 'allow:$1'));
   my patterns = new Autom4te::XFile ("< " .
open_quote ("$tmp/patterns"))->getlines;
   chomp patterns;
-  my %forbidden =
-    map { /^forbid:([^:]+):.+$/ => /^forbid:[^:]+:(.+)$/
} patterns;
+  my %forbidden = forbid_message_filter(patterns);
   my $forbidden = join ('|', map { /^forbid:([^:]+)/ } patterns)
|| "^$";
   my $allowed   = join ('|', map { /^allow:([^:]+)/  } patterns)
|| "^$";
 
diff --git a/tests/tools.at b/tests/tools.at
index 9e13689..e727a25 100644
--- a/tests/tools.at
+++ b/tests/tools.at
 -362,6
+362,73  AT_CHECK([autoconf])
 AT_CLEANUP
 
 
+# autoconf: forbidden tokens, no comment
+# -------------------------------------------------
+AT_SETUP([autoconf: forbidden tokens,[] no comment])
+
+AT_DATA_M4SH([configure.ac],
+[[AS_INIT
+m4_pattern_forbid([FOOBAR])
+FOOBAR
+]])
+
+AT_CHECK_AUTOCONF([], 1, [],
+[[configure.ac:2: error: possibly undefined macro: FOOBAR
+      If this token and others are legitimate, please use
m4&t_pattern_allow.
+      See the Autoconf documentation.
+]])
+# Second run should succeed and yield no output.
+AT_CHECK([autoconf])
+
+AT_CLEANUP
+
+
+# autoconf: forbidden tokens, with one-line comment
+# -------------------------------------------------
+AT_SETUP([autoconf: forbidden tokens,[] one-line comment])
+
+AT_DATA_M4SH([configure.ac],
+[[AS_INIT
+m4_pattern_forbid([FOOBAR], [foobar macro not found])
+FOOBAR
+]])
+
+AT_CHECK_AUTOCONF([], 1, [],
+[[configure.ac:2: error: foobar macro not found
+      If this token and others are legitimate, please use
m4&t_pattern_allow.
+      See the Autoconf documentation.
+]])
+# Second run should succeed and yield no output.
+AT_CHECK([autoconf])
+
+AT_CLEANUP
+
+
+# autoconf: forbidden tokens, with multi-line comment
+# -------------------------------------------------
+AT_SETUP([autoconf: forbidden tokens,[] multi-line
comment])
+
+AT_DATA_M4SH([configure.ac],
+[[AS_INIT
+m4_pattern_forbid([FOOBAR], [line 1 of comment
+line 2 of comment
+line 3 of comment])
+FOOBAR
+]])
+
+AT_CHECK_AUTOCONF([], 1, [],
+[[configure.ac:2: error: line 1 of comment
+line 2 of comment
+line 3 of comment
+      If this token and others are legitimate, please use
m4&t_pattern_allow.
+      See the Autoconf documentation.
+]])
+# Second run should succeed and yield no output.
+AT_CHECK([autoconf])
+
+AT_CLEANUP
+
+
 # autoconf: forbidden tokens, exceptions
 # --------------------------------------
 AT_SETUP([autoconf: forbidden tokens,[] exceptions])
-- 
1.5.5.1

Hans Ulrich Niedermann <hun <at>
n-dimensional.de> writes:

> 
> Let m4 create $tmp/patterns with comments spread over
> multiple lines as before, but have autom4te reassemble
> multi-line comments.

Thanks for the report, and even more for the patch idea. 
However, you don't 
appear to have copyright on file, so we will need to get
that taken care of 
before your patch can be incorporated.

For that matter, the manual doesn't even document that
m4_pattern_forbid takes 
a second argument - this is a more fundamental bug that
needs fixing, in part 
because we are free to change the semantics of the second
parameter as long as 
is not documented (for that matter, it is technically
possible, although not 
very nice, to simply document that the second argument
exists but must be a 
single line).

> An alternative solution would be to let autom4te have
> m4 convert multi-line comments to an escaped one-line
> form and let autom4te later unescape them.

autom4te is capturing the m4_pattern_forbid via m4's trace
mechanism.  m4 1.4.x 
outputs trace lines with embedded newlines, with no way
around it (trace lines 
are sent to the trace file without any further processing by
m4).  I'm working 
on making the eventual m4 1.6 output escape sequences
instead, but don't know 
how it will interact with autom4te (at any rate, trace style
would have to be 
run-time configurable in m4, and autom4te will still have to
support the older 
1.4.x output if the newer 1.6 trace output is not present).

I haven't looked closely at your patch (intentionally, so
that I still have the 
freedom to implement something independently, if copyright
assignment becomes 
an issue), but did spot some immediate nits:

>  
> +# autoconf: forbidden tokens, no comment
> +# -------------------------------------------------
> +AT_SETUP([autoconf: forbidden tokens,[] no comment])

The [] after , should be extraneous, as of autoconf 2.62. 
For that matter, it 
seems like this would not be adding anything (except
testsuite execution time) 
to what the existing tests for forbidden tokens already
did.

> +
> +# autoconf: forbidden tokens, with one-line comment
> +# -------------------------------------------------
> +AT_SETUP([autoconf: forbidden tokens,[] one-line
comment])

Again, the ,[] seems extra; plain , should work.  But you
did the right thing 
in adding test cases!

>  # autoconf: forbidden tokens, exceptions
>  # --------------------------------------
>  AT_SETUP([autoconf: forbidden tokens,[] exceptions])

Ah, I see.  You were copying existing usage, and that usage
pre-dated my fix in 
2.62 that made the [] unnecessary.

-- 
Eric Blake