[Preprocessor] -E -P: Ensure newline after 8 skipped lines.

[Meinersbur]

Summary

The implementation of -fminimize-whitespace (D104601) revised the logic when to emit newlines. There was no case to handle when more than 8 lines were skippped in -P (DisableLineMarkers) mode and instead fell through the case intended for -fminimize-whitespace, i.e. emit nothing. This patch will emit one newline in this case.

The newline logic is slightly reorganized. The -P -fminimize-whitespace case is handled explicitly and emitting at least one newline is the new fallback case. The choice between emitting a line marker or up to 8 empty lines is now a choice only with enabled line markers. The up to 8 newlines likely are fewer characters than a line directive, but in -P mode this had the paradoxic effect that it would print up to 7 empty lines, but none at all if more than 8 lines had to be skipped. Now with DisableLineMarkers, we don't consider printing empty lines (just start a new line) which matches gcc's behavior.

The line-directive-output-mincol.c test is replaced with a more comprehensive test skip-empty-lines.c also testing the more than 8 skipped lines behaviour with all flag combinations.

[mstorsjo]

Thanks! I can confirm that this fixes the issue I ran into.

[aaron.ballman]

LGTM!

[mstorsjo]

@Meinersbur After landing this, can you coordinate with @tstellar to get an ack for backporting this to the 13.x release branch, which also suffers from the regression?

[Meinersbur]

• Fix embrassing mistakes in skip-empty-lines test (misspelled "LINEMARKES", MINWS<->MINCOL, absolute path)
• Skip-empty-lines now also checks leading whitespace behavior
• Fix typos in comments

[Meinersbur]

• Use correct base commit

[Meinersbur]

In D106924#2911145, @mstorsjo wrote:

@Meinersbur After landing this, can you coordinate with @tstellar to get an ack for backporting this to the 13.x release branch, which also suffers from the regression?

Release blocker has been created: http://llvm.org/PR51261

[sberg]

Bisecting shows that this commit causes

$ echo foo | clang -E -P -

foo

to emit a leading newline now.

That breaks e.g. LibreOffice's configure, which uses echo __clang_version__ | clang -E -P - to obtain the value of that macro, and expects a single line of output.

[sberg]

In D106924#2919406, @sberg wrote:

Bisecting shows that this commit causes

$ echo foo | clang -E -P -

foo

to emit a leading newline now.

That breaks e.g. LibreOffice's configure, which uses echo __clang_version__ | clang -E -P - to obtain the value of that macro, and expects a single line of output.

I now filed https://bugs.llvm.org/show_bug.cgi?id=51616 "Bad newlines in -E -P output after https://reviews.llvm.org/D106924 '[Preprocessor] -E -P: Ensure newline after 8 skipped lines.'"

clang/lib/Frontend/PrintPreprocessedOutput.cpp

if ((RequireStartOfLine && EmittedTokensOnThisLine) ||
     EmittedTokensOnThisLine = false;
     EmittedDirectiveOnThisLine = false;
   }
 
   // If this line is "close enough" to the original line, just print newlines,
   // otherwise print a #line directive.
   if (CurLine == LineNo) {
     // Nothing to do if we are already on the correct line.
-  } else if (!StartedNewLine && (!MinimizeWhitespace || !DisableLineMarkers) &&
-             LineNo - CurLine == 1) {
+  } else if (MinimizeWhitespace && DisableLineMarkers) {
+    // With -E -P -fminimize-whitespace, don't emit anything if not necessary.
+  } else if (!StartedNewLine && LineNo - CurLine == 1) {
     // Printing a single line has priority over printing a #line directive, even
     // when minimizing whitespace which otherwise would print #line directives
     // for every single line.
     OS << '\n';
     StartedNewLine = true;
-  } else if (!MinimizeWhitespace && LineNo - CurLine <= 8) {
+  } else if (!DisableLineMarkers) {
+    if (LineNo - CurLine <= 8) {
       const char *NewLines = "\n\n\n\n\n\n\n\n";
       OS.write(NewLines, LineNo - CurLine);
-    StartedNewLine = true;
-  } else if (!DisableLineMarkers) {
+    } else {
       // Emit a #line or line marker.
       WriteLineInfo(LineNo, nullptr, 0);
+    }
+    StartedNewLine = true;
+  } else if (!StartedNewLine) {
+    // If we are not on the correct line and don't need to be line-correct,
+    // at least ensure we start on a new line.
+    OS << '\n';
     StartedNewLine = true;
   }
 
   if (StartedNewLine) {
     EmittedTokensOnThisLine = false;
     EmittedDirectiveOnThisLine = false;
   }
 

clang/test/Preprocessor/line-directive-output-mincol.c

-// RUN: %clang_cc1 -E -fminimize-whitespace %s 2>&1 | FileCheck %s -strict-whitespace
-
-// CHECK:      # 6 "{{.*}}line-directive-output-mincol.c"
-// CHECK-NEXT: int x;
-// CHECK-NEXT: int y;
-int x;
-int y;
-// CHECK-NEXT: # 10 "{{.*}}line-directive-output-mincol.c"
-// CHECK-NEXT: int z;
-int z;
-

clang/test/Preprocessor/minimize-whitespace.c

 // RUN: %clang_cc1 -fminimize-whitespace -E %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=MINCOL
 // RUN: %clang_cc1 -fminimize-whitespace -E -C %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=MINCCOL
 // RUN: %clang_cc1 -fminimize-whitespace -E -P %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=MINWS
 // RUN: %clang_cc1 -fminimize-whitespace -E -C -P %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=MINCWS
+// The follow empty lines ensure that a #line directive is emitted instead of newline padding after the RUN comments.
+
+
+
+
+
 
 #define NOT_OMP  omp  something
 #define HASH #
 
   int  a;     /*  span-comment  */
   int  b  ;   //  line-comment
   _Pragma  (  "omp  barrier"  ) x //  more line-comments
   #pragma  omp  nothing  //  another comment
 HASH  pragma  NOT_OMP
   int  e;    // again a line
   int  \
 f  ;
 
 
-// MINCOL:      {{^}}# 9 "{{.*}}minimize-whitespace.c"{{$}}
+// MINCOL:      {{^}}# 15 "{{.*}}minimize-whitespace.c"{{$}}
 // MINCOL:      {{^}}int a;{{$}}
 // MINCOL-NEXT: {{^}}int b;{{$}}
 // MINCOL-NEXT: {{^}}#pragma omp barrier{{$}}
-// MINCOL-NEXT: # 11 "{{.*}}minimize-whitespace.c"
+// MINCOL-NEXT: # 17 "{{.*}}minimize-whitespace.c"
 // MINCOL-NEXT: {{^}}x{{$}}
 // MINCOL-NEXT: {{^}}#pragma omp nothing{{$}}
 // MINCOL-NEXT: {{^ }}#pragma omp something{{$}}
 // MINCOL-NEXT: {{^}}int e;{{$}}
 // MINCOL-NEXT: {{^}}int f;{{$}}
 
 // FIXME: Comments after pragmas disappear, even without -fminimize-whitespace
-// MINCCOL:      {{^}}# 9 "{{.*}}minimize-whitespace.c"{{$}}
+// MINCCOL:      {{^}}# 15 "{{.*}}minimize-whitespace.c"{{$}}
 // MINCCOL:      {{^}}int a;/*  span-comment  */{{$}}
 // MINCCOL-NEXT: {{^}}int b;//  line-comment{{$}}
 // MINCCOL-NEXT: {{^}}#pragma omp barrier{{$}}
-// MINCCOL-NEXT: # 11 "{{.*}}minimize-whitespace.c"
+// MINCCOL-NEXT: # 17 "{{.*}}minimize-whitespace.c"
 // MINCCOL-NEXT: {{^}}x//  more line-comments{{$}}
 // MINCCOL-NEXT: {{^}}#pragma omp nothing{{$}}
 // MINCCOL-NEXT: {{^ }}#pragma omp something{{$}}
 // MINCCOL-NEXT: {{^}}int e;// again a line{{$}}
 // MINCCOL-NEXT: {{^}}int f;{{$}}
 
 // MINWS:      {{^}}int a;int b;{{$}}
 // MINWS-NEXT: {{^}}#pragma omp barrier{{$}}

clang/test/Preprocessor/skip-empty-lines.c

+  int  a ;
+  int  b ;
+// A single empty line
+  int  c ;
+/*
+
+more than 8 empty lines
+(forces a line marker instead of newline padding)
+
+
+
+
+*/
+  int  d ;
+
+// RUN: %clang_cc1 -E %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=LINEMARKERS
+// RUN: %clang_cc1 -E -P %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=COLSONLY
+// RUN: %clang_cc1 -E -fminimize-whitespace %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=MINCOL
+// RUN: %clang_cc1 -E -P -fminimize-whitespace %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=MINWS
+
+// Check behavior after varying number of lines without emitted tokens.
+
+// LINEMARKERS:       {{^}}# 1 "{{.*}}skip-empty-lines.c" 2
+// LINEMARKERS-NEXT: {{^}}  int a ;
+// LINEMARKERS-NEXT: {{^}}  int b ;
+// LINEMARKERS-EMPTY:
+// LINEMARKERS-NEXT: {{^}}  int c ;
+// LINEMARKERS-NEXT: {{^}}# 14 "{{.*}}skip-empty-lines.c"
+// LINEMARKERS-NEXT: {{^}}  int d ;
+
+// COLSONLY:      {{^}}  int a ;
+// COLSONLY-NEXT: {{^}}  int b ;
+// COLSONLY-NEXT: {{^}}  int c ;
+// COLSONLY-NEXT: {{^}}  int d ;
+
+// MINCOL:      {{^}}# 1 "{{.*}}skip-empty-lines.c" 2
+// MINCOL-NEXT: {{^}}int a;
+// MINCOL-NEXT: {{^}}int b;
+// MINCOL-EMPTY:
+// MINCOL-NEXT: {{^}}int c;
+// MINCOL-NEXT: {{^}}# 14 "{{.*}}skip-empty-lines.c"
+// MINCOL-NEXT: {{^}}int d;
+
+// MINWS: {{^}}int a;int b;int c;int d;
+