fix for diagnostic/84034

Message ID DB6PR0701MB2664A0AAD17DD5BC8E69B34BE4E10@DB6PR0701MB2664.eurprd07.prod.outlook.com
State New
Headers show
Series
  • fix for diagnostic/84034
Related show

Commit Message

Bernd Edlinger Jan. 25, 2018, 6:53 p.m.
Hi,

as PR diagnostic/84034 shows, source files with
dos style line endings can cause a glitch in the
terminal emulation that erases the source line that
is supposed to be shown.

That happens when the colorizing escape sequences are
printed between the CR and the LF.  Apparently the LF is
being ignored and thus the following line overwrites
everything from the last source line.


The following patch fixes the visual glitch by handling
a CR '\r' like a TAB '\t' character.


Bootstrapped and reg-rested in x86_64-pc-linux-gnu.
OK for trunk?


Thanks
Bernd.

Comments

David Malcolm Jan. 26, 2018, 10:48 p.m. | #1
On Thu, 2018-01-25 at 18:53 +0000, Bernd Edlinger wrote:
> Hi,

> 

> as PR diagnostic/84034 shows, source files with

> dos style line endings can cause a glitch in the

> terminal emulation that erases the source line that

> is supposed to be shown.

> 

> That happens when the colorizing escape sequences are

> printed between the CR and the LF.  Apparently the LF is

> being ignored and thus the following line overwrites

> everything from the last source line.

> 

> 

> The following patch fixes the visual glitch by handling

> a CR '\r' like a TAB '\t' character.

> 

> 

> Bootstrapped and reg-rested in x86_64-pc-linux-gnu.

> OK for trunk?


Thanks for working on this.

[BEGIN BRAIN-DUMP:

Before gcc 6, we handled this case by printing (e.g. gcc 5):
/tmp/t.c: In function ‘test’:
/tmp/t.c:5:20: warning: suggest parentheses around ‘&&’ within ‘||’ [-
Wparentheses]
       (d && b > e) &&
                    ^
where all lines would end with LF, apart from the quoted source line,
which would end with CR+LF.

From gcc 6 onwards, the lines all end with LF, apart from quoted source
lines, which end with:
  CR + (end-colorization) + LF.

What's happening is that layout::print_source_line etc treat the CR as
non-whitespace, and prints it.  Then layout::print_newline () prints
the end-colorization and LF.

Presumably we *don't* want to preserve that mixed-ending behavior in
our output from gcc 5 and earlier descibed above; that seems like an
accident of the old implementation, and doesn't seem useful.

Your patch effectively turns the CR before the LF into trailing
whitespace, stopping the CR from being printed, turning it into just:
  (end-colorization) + LF
and thus fixing the glitch.

END BRAIN-DUMP]

The patch is OK.

Thanks
Dave

Patch

2018-01-25  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR diagnostic/84034
	* diagnostic-show-locus.c (get_line_width_without_trailing_whitespace):
	Handle CR like TAB.
	(layout::print_source_line): Likewise.
	(test_get_line_width_without_trailing_whitespace): Add test cases.

Index: gcc/diagnostic-show-locus.c
===================================================================
--- gcc/diagnostic-show-locus.c	(revision 257048)
+++ gcc/diagnostic-show-locus.c	(working copy)
@@ -639,7 +639,7 @@  get_line_width_without_trailing_whitespace (const
   while (result > 0)
     {
       char ch = line[result - 1];
-      if (ch == ' ' || ch == '\t')
+      if (ch == ' ' || ch == '\t' || ch == '\r')
 	result--;
       else
 	break;
@@ -648,7 +648,8 @@  get_line_width_without_trailing_whitespace (const
   gcc_assert (result <= line_width);
   gcc_assert (result == 0 ||
 	      (line[result - 1] != ' '
-	       && line[result -1] != '\t'));
+	       && line[result -1] != '\t'
+	       && line[result -1] != '\r'));
   return result;
 }
 
@@ -673,9 +674,11 @@  test_get_line_width_without_trailing_whitespace ()
   assert_eq ("", 0);
   assert_eq (" ", 0);
   assert_eq ("\t", 0);
+  assert_eq ("\r", 0);
   assert_eq ("hello world", 11);
   assert_eq ("hello world     ", 11);
   assert_eq ("hello world     \t\t  ", 11);
+  assert_eq ("hello world\r", 11);
 }
 
 #endif /* #if CHECKING_P */
@@ -1176,8 +1179,8 @@  layout::print_source_line (int row, const char *li
 	  else
 	    m_colorizer.set_normal_text ();
 	}
-      char c = *line == '\t' ? ' ' : *line;
-      if (c == '\0')
+      char c = *line;
+      if (c == '\0' || c == '\t' || c == '\r')
 	c = ' ';
       if (c != ' ')
 	{