[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 (LineNo - CurLine <= 8) {
       const char *NewLines = "\n\n\n\n\n\n\n\n";
       OS.write(NewLines, LineNo - CurLine);
     } 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.
+    // 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/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 lines
+// 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 #

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=LINEMARKES
+// 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=MINWS
-// RUN: %clang_cc1 -E -P -fminimize-whitespace %s 2>&1 | FileCheck %s --strict-whitespace --check-prefix=MINCOL
+// 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 variying number of lines without emitted tokens.
+// Check behavior after varying number of lines without emitted tokens.
 
-// LINEMARKES:       # 1 "{{.*}}skip-empty-lines.c" 2
-// LINEMARKERS-NEXT: int a ;
-// LINEMARKERS-NEXT: int b ;
+// LINEMARKERS:       {{^}}# 1 "{{.*}}skip-empty-lines.c" 2
+// LINEMARKERS-NEXT: {{^}}  int a ;
+// LINEMARKERS-NEXT: {{^}}  int b ;
 // LINEMARKERS-EMPTY:
-// LINEMARKERS-NEXT: int c ;
-// LINEMARKERS-NEXT: # 14 "/c/Users/meinersbur/src/llvm-project/clang/test/Preprocessor/skip-empty-lines.c"
-// LINEMARKERS-NEXT: int d ;
-
-// COLSONLY:      int a ;
-// COLSONLY-NEXT: int b ;
-// COLSONLY-NEXT: int c ;
-// COLSONLY-NEXT: int d ;
-
-// MINWS:      # 1 "{{.*}}skip-empty-lines.c" 2
-// MINWS-NEXT: int a;
-// MINWS-NEXT: int b;
-// MINWS-EMPTY:
-// MINWS-NEXT: int c;
-// MINWS-NEXT: # 14 "{{.*}}skip-empty-lines.c"
-// MINWS-NEXT: int d;
+// 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;
 
-// MINCOL: int a;int b;int c;int d;