[Frontend] Don't output skipped includes from predefines

`-H` displays a tree of included header files, but that tree is supposed
to omit two categories of header files:
1. Any header files pulled in via `-include`, which the code refers to
   as the "predefines".
2. Any header files whose inclusion was skipped because they'd already
   been included (assuming header guards or `#pragma once`).

`-fshow-skipped-includes` was intended to make `-H` display the second
category of files. It wasn't checking for the first category, however,
so you could end up with only the middle of the `-include` hierarchy
displayed, e.g. the added test would previously output:

```
... /data/users/smeenai/llvm-project/clang/test/Frontend/Inputs/test2.h
. /data/users/smeenai/llvm-project/clang/test/Frontend/Inputs/test.h
```

This diff adds a check to prevent that and correctly omit headers from
`-include` even when `-fshow-skipped-includes` is passed. While I'm
here, add tests for the interaction between `-fshow-skipped-includes`
and `-sys-header-deps` as well.

Reviewed By: hans

Differential Revision: https://reviews.llvm.org/D153175

clang/lib/Frontend/HeaderIncludeGen.cpp

@@ -49,6 +49,18 @@ class HeaderIncludesCallback : public PPCallbacks {
 
   void FileSkipped(const FileEntryRef &SkippedFile, const Token &FilenameTok,
                    SrcMgr::CharacteristicKind FileType) override;
+
+private:
+  bool ShouldShowHeader(SrcMgr::CharacteristicKind HeaderType) {
+    if (!DepOpts.IncludeSystemHeaders && isSystem(HeaderType))
+      return false;
+
+    // Show the current header if we are (a) past the predefines, or (b) showing
+    // all headers and in the predefines at a depth past the initial file and
+    // command line buffers.
+    return (HasProcessedPredefines ||
+            (ShowAllHeaders && CurrentIncludeDepth > 2));
+  }
 };
 
 /// A callback for emitting header usage information to a file in JSON. Each
@@ -211,29 +223,22 @@ void HeaderIncludesCallback::FileChanged(SourceLocation Loc,
     }
 
     return;
-  } else
+  } else {
+    return;
+  }
+
+  if (!ShouldShowHeader(NewFileType))
     return;
 
-  // Show the header if we are (a) past the predefines, or (b) showing all
-  // headers and in the predefines at a depth past the initial file and command
-  // line buffers.
-  bool ShowHeader = (HasProcessedPredefines ||
-                     (ShowAllHeaders && CurrentIncludeDepth > 2));
   unsigned IncludeDepth = CurrentIncludeDepth;
   if (!HasProcessedPredefines)
     --IncludeDepth; // Ignore indent from <built-in>.
   else if (!DepOpts.ShowIncludesPretendHeader.empty())
     ++IncludeDepth; // Pretend inclusion by ShowIncludesPretendHeader.
 
-  if (!DepOpts.IncludeSystemHeaders && isSystem(NewFileType))
-    ShowHeader = false;
-
-  // Dump the header include information we are past the predefines buffer or
-  // are showing all headers and this isn't the magic implicit <command line>
-  // header.
   // FIXME: Identify headers in a more robust way than comparing their name to
   // "<command line>" and "<built-in>" in a bunch of places.
-  if (ShowHeader && Reason == PPCallbacks::EnterFile &&
+  if (Reason == PPCallbacks::EnterFile &&
       UserLoc.getFilename() != StringRef("<command line>")) {
     PrintHeaderInfo(OutputFile, UserLoc.getFilename(), ShowDepth, IncludeDepth,
                     MSStyle);
@@ -246,7 +251,7 @@ void HeaderIncludesCallback::FileSkipped(const FileEntryRef &SkippedFile, const
   if (!DepOpts.ShowSkippedHeaderIncludes)
     return;
 
-  if (!DepOpts.IncludeSystemHeaders && isSystem(FileType))
+  if (!ShouldShowHeader(FileType))
     return;
 
   PrintHeaderInfo(OutputFile, SkippedFile.getName(), ShowDepth,

clang/test/Frontend/print-header-includes.c

@@ -17,6 +17,29 @@
 // SKIPPED: .. {{.*test2.h}}
 // SKIPPED: .. {{.*test2.h}}
 
+// RUN: %clang_cc1 -isystem %S -isystem %S/Inputs/SystemHeaderPrefix \
+// RUN:     -E -H -fshow-skipped-includes -sys-header-deps -o /dev/null %s 2> %t.stderr
+// RUN: FileCheck --check-prefix=SKIPPED-SYS < %t.stderr %s
+
+// SKIPPED-SYS: . {{.*noline.h}}
+// SKIPPED-SYS: . {{.*test.h}}
+// SKIPPED-SYS: .. {{.*test2.h}}
+// SKIPPED-SYS: .. {{.*test2.h}}
+
+// RUN: %clang_cc1 -I%S -isystem %S/Inputs/SystemHeaderPrefix -include Inputs/test.h \
+// RUN:     -E -H -fshow-skipped-includes -o /dev/null %s 2> %t.stderr
+// RUN: FileCheck --check-prefix=SKIPPED-PREDEFINES < %t.stderr %s
+
+// The skipped include of test2.h from the -include test.h shouldn't be printed.
+// SKIPPED-PREDEFINES-NOT: {{.*test2.h}}
+// SKIPPED-PREDEFINES:     . {{.*test.h}}
+
+// RUN: %clang_cc1 -isystem %S -isystem %S/Inputs/SystemHeaderPrefix \
+// RUN:     -E -H -fshow-skipped-includes -o /dev/null %s 2> %t.stderr
+// RUN: FileCheck --check-prefix=SKIPPED-NO-SYS --allow-empty < %t.stderr %s
+
+// SKIPPED-NO-SYS-NOT: .
+
 // RUN: %clang_cc1 -I%S -include Inputs/test3.h -isystem %S/Inputs/SystemHeaderPrefix \
 // RUN:     -E -H -sys-header-deps -o /dev/null %s 2> %t.stderr
 // RUN: FileCheck --check-prefix SYSHEADERS < %t.stderr %s
@@ -58,4 +81,4 @@
 // MS-IGNORELIST-NOT: Note
 
 #include <noline.h>
-#include "Inputs/test.h"
+#include <Inputs/test.h>