[groff] 01/01: Empower backtracing in error/warning conditions.

[G. Branden Robinson]

gbranden pushed a commit to branch master
in repository groff.

commit 6051dae0ed025521ec5a32f9c5e8ebffa7feaf36
Author: G. Branden Robinson <address@hidden>
AuthorDate: Sat Apr 11 11:08:55 2020 +1000

    Empower backtracing in error/warning conditions.
    
    Enable backtracing across process/file boundaries when errors or
    non-ignored warnings are encountered.
    
    Experimentation reveals that .so, .mso, and .pso requests act as
    barriers to backtracing except when explicitly requested with the
    .backtrace request.  Judging by the git history, this behavior dates
    back to June 1991 or earlier.  This did not appear to be the intention
    according to a comment, which was only to suppress the output of
    backtrace output for the line corresponding to the top level itself.
    Unfortunately, that was not its only effect.
    
    This change does result in one additional line of output when -b is
    given and an error or (non-ignored) warning happens at the top level.
    However, I regard this as unobjectionable because (1) a backtrace was in
    fact explicitly requested; and (2) it seems a poor tradeoff to suppress
    most of the backtrace in all complicated and frustrating cases for the
    sake of one fewer line of backtrace output in a trivial one.
    
    Now, backtracing behaves the same no matter what triggers it.
    
    Fixes Savannah #58153.
    
    * src/roff/troff/input.cpp (file_iterator::backtrace): Call
      get_location() for its side effect of rewriting a filename of "-" to
      "<standard input>", for consistency with other diagnostic messages.
      (In this class, this member function always returns 1, so ignore the
      return value.)
    
      (input_stack::backtrace): Replace member function body with that
      of input_stack::backtrace_all().
    
      (input_stack::backtrace_all): Delete.
    
      (backtrace_request): Update the only call site of the above.
    
    * src/roff/groff/tests/regression_savannah_58153.sh:
    * src/roff/groff/groff.am: Add regression test.
    
    * src/roff/groff/tests/string_case_xform_errors.sh: Update regression
      test to not be confounded by additional line of backtrace output.
      Remove bashism along the way.
    
    * doc/groff.texi:
    * man/groff.7.man:
    * man/groff_diff.7.man: Update documentation.
---
 ChangeLog                                          | 47 ++++++++++++++++++++++
 doc/groff.texi                                     | 11 ++---
 man/groff.7.man                                    |  2 +-
 man/groff_diff.7.man                               |  2 +-
 src/roff/groff/groff.am                            |  1 +
 ...form_errors.sh => regression_savannah_58153.sh} | 14 +++----
 src/roff/groff/tests/string_case_xform_errors.sh   |  4 +-
 src/roff/troff/input.cpp                           | 21 +++-------
 8 files changed, 71 insertions(+), 31 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c6418f4..ced141e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,50 @@
+2020-04-11  G. Branden Robinson <address@hidden>
+
+       Enable backtracing across process/file boundaries when errors or
+       non-ignored warnings are encountered.
+
+       Experimentation reveals that .so, .mso, and .pso requests act as
+       barriers to backtracing except when explicitly requested with
+       the .backtrace request.  Judging by the git history, this
+       behavior dates back to June 1991 or earlier.  This did not
+       appear to be the intention according to a comment, which was
+       only to suppress the output of backtrace output for the line
+       corresponding to the top level itself.  Unfortunately, that was
+       not its only effect.
+
+       This change does result in one additional line of output when -b
+       is given and an error or (non-ignored) warning happens at the
+       top level.  However, I regard this as unobjectionable because
+       {1} a backtrace was in fact explicitly requested; and {2} it
+       seems a poor tradeoff to suppress most of the backtrace in all
+       complicated and frustrating cases for the sake of one fewer line
+       of backtrace output in a trivial one.
+
+       Now, backtracing behaves the same no matter what triggers it.
+
+       Fixes Savannah #58153.
+
+       * src/roff/troff/input.cpp (file_iterator::backtrace): Call
+       get_location() for its side effect of rewriting a filename of
+       "-" to "<standard input>", for consistency with other diagnostic
+       messages.  (In this class, this member function always returns
+       1, so ignore the return value.)
+       (input_stack::backtrace): Replace member function body with that
+       of input_stack::backtrace_all().
+       (input_stack::backtrace_all): Delete.
+       (backtrace_request): Update the only call site of the above.
+
+       * src/roff/groff/tests/regression_savannah_58153.sh:
+       * src/roff/groff/groff.am: Add regression test.
+
+       * src/roff/groff/tests/string_case_xform_errors.sh: Update
+       regression test to not be confounded by additional line of
+       backtrace output.  Remove bashism along the way.
+
+       * doc/groff.texi:
+       * man/groff.7.man:
+       * man/groff_diff.7.man: Update documentation.
+
 2020-04-10  G. Branden Robinson <address@hidden>
 
        Reorganize backtrace output.
diff --git a/doc/groff.texi b/doc/groff.texi
index 0478baf..5717194 100644
--- a/doc/groff.texi
+++ b/doc/groff.texi
@@ -14098,13 +14098,14 @@ Consider the following in file @file{test}:
 On execution, @code{gtroff} prints the following:
 
 @Example
-test:2: backtrace: macro `xxx'
-test:5: backtrace: macro `yyy'
-test:8: backtrace: file `test'
+gtroff: backtrace: 'test':2: macro 'xxx'
+gtroff: backtrace: 'test':5: macro 'yyy'
+gtroff: backtrace: file 'test':8
 @endExample
 
-The option @option{-b} of @code{gtroff} internally calls a variant of
-this request on each error and warning.
+The option @option{-b} of @code{gtroff} causes a backtrace to be
+generated on each error and warning.  Note that warnings have to be
+enabled; @pxref{Warnings}.
 @endDefreq
 
 @Defreg {slimit}
diff --git a/man/groff.7.man b/man/groff.7.man
index 7043020..af03082 100644
--- a/man/groff.7.man
+++ b/man/groff.7.man
@@ -1261,7 +1261,7 @@ Unformat ASCII characters, spaces, and some escape 
sequences in
 .
 .TPx
 .REQ .backtrace
-Print a backtrace of the input on stderr.
+Write a backtrace of the input stack to the standard error stream.
 .
 .TPx
 .REQ .bd "font N"
diff --git a/man/groff_diff.7.man b/man/groff_diff.7.man
index 1881f4b..a32b559 100644
--- a/man/groff_diff.7.man
+++ b/man/groff_diff.7.man
@@ -933,7 +933,7 @@ instead.
 .
 .TP
 .B .backtrace
-Print a backtrace of the input stack on stderr.
+Write a backtrace of the input stack to the standard error stream.
 .
 .TP
 .BI .blm\  xx
diff --git a/src/roff/groff/groff.am b/src/roff/groff/groff.am
index fae28aa..ca1f4ad 100644
--- a/src/roff/groff/groff.am
+++ b/src/roff/groff/groff.am
@@ -44,6 +44,7 @@ groff_TESTS = \
   src/roff/groff/tests/on_latin1_device_oq_is_0x27.sh \
   src/roff/groff/tests/recognize_end_of_sentence.sh \
   src/roff/groff/tests/regression_savannah_56555.sh \
+  src/roff/groff/tests/regression_savannah_58153.sh \
   src/roff/groff/tests/string_case_xform_errors.sh \
   src/roff/groff/tests/string_case_xform_requests.sh \
   src/roff/groff/tests/string_case_xform_unicode_escape.sh \
diff --git a/src/roff/groff/tests/string_case_xform_errors.sh 
b/src/roff/groff/tests/regression_savannah_58153.sh
similarity index 70%
copy from src/roff/groff/tests/string_case_xform_errors.sh
copy to src/roff/groff/tests/regression_savannah_58153.sh
index c1c02bc..a12d86e 100755
--- a/src/roff/groff/tests/string_case_xform_errors.sh
+++ b/src/roff/groff/tests/regression_savannah_58153.sh
@@ -1,6 +1,6 @@
-#!/usr/bin/env bash
+#!/bin/sh
 #
-# Copyright (C) 2019 Free Software Foundation, Inc.
+# Copyright (C) 2020 Free Software Foundation, Inc.
 #
 # This file is part of groff.
 #
@@ -20,11 +20,11 @@
 
 groff="${abs_top_builddir:-.}/test-groff"
 
-expected="troff: <standard input>:1: error: cannot apply string case 
transformation to a request ('br')"
-
-actual=$("$groff" -Tutf8 2>&1 <<EOF
-.stringdown br
+# Ensure that we get backtrace output across file and pipe boundaries.
+OUT=$("$groff" -U 2>&1 >/dev/null <<EOF
+.pso printf '\\\s[-20]\\\n'
 EOF
 )
 
-diff -u <(echo "$expected") <(echo "$actual")
+printf "%s\n" "$OUT" | grep -qw 'backtrace: pipe'
+printf "%s\n" "$OUT" | grep -qw 'backtrace: file'
diff --git a/src/roff/groff/tests/string_case_xform_errors.sh 
b/src/roff/groff/tests/string_case_xform_errors.sh
index c1c02bc..7caaadb 100755
--- a/src/roff/groff/tests/string_case_xform_errors.sh
+++ b/src/roff/groff/tests/string_case_xform_errors.sh
@@ -1,4 +1,4 @@
-#!/usr/bin/env bash
+#!/bin/sh
 #
 # Copyright (C) 2019 Free Software Foundation, Inc.
 #
@@ -27,4 +27,4 @@ actual=$("$groff" -Tutf8 2>&1 <<EOF
 EOF
 )
 
-diff -u <(echo "$expected") <(echo "$actual")
+echo "$actual" | grep -qx "$expected"
diff --git a/src/roff/troff/input.cpp b/src/roff/troff/input.cpp
index 5d2ceb6..f35fcbf 100644
--- a/src/roff/troff/input.cpp
+++ b/src/roff/troff/input.cpp
@@ -401,10 +401,13 @@ int file_iterator::get_location(int /*allow_macro*/,
 
 void file_iterator::backtrace()
 {
+  const char *fn;
+  int l;
+  // Get side effect of filename rewrite if stdin.
+  (void) get_location(0, &fn, &l);
   if (program_name)
     fprintf(stderr, "%s: ", program_name);
-  errprint("backtrace: %3 '%1':%2\n", filename, lineno,
-          popened ? "pipe" : "file");
+  errprint("backtrace: %3 '%1':%2\n", fn, l, popened ? "pipe" : "file");
 }
 
 int file_iterator::set_location(const char *f, int ln)
@@ -435,7 +438,6 @@ public:
   static int get_location(int, const char **, int *);
   static int set_location(const char *, int);
   static void backtrace();
-  static void backtrace_all();
   static void next_file(FILE *, const char *);
   static void end_file();
   static void shift(int n);
@@ -698,17 +700,6 @@ int input_stack::get_location(int allow_macro, const char 
**filenamep, int *line
 
 void input_stack::backtrace()
 {
-  const char *f;
-  int n;
-  // only backtrace down to (not including) the topmost file
-  for (input_iterator *p = top;
-       p && !p->get_location(0, &f, &n);
-       p = p->next)
-    p->backtrace();
-}
-
-void input_stack::backtrace_all()
-{
   for (input_iterator *p = top; p; p = p->next)
     p->backtrace();
 }
@@ -796,7 +787,7 @@ inline int input_stack::get_compatible_flag()
 
 void backtrace_request()
 {
-  input_stack::backtrace_all();
+  input_stack::backtrace();
   fflush(stderr);
   skip_line();
 }