[PATCH] D153175: [Frontend] Don't output skipped includes from predefines

[Shoaib Meenai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG67a11290df64: [Frontend] Dont output skipped includes 
from predefines (authored by smeenai).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153175/new/

https://reviews.llvm.org/D153175

Files:
  clang/lib/Frontend/HeaderIncludeGen.cpp
  clang/test/Frontend/print-header-includes.c

Index: clang/test/Frontend/print-header-includes.c
===
--- clang/test/Frontend/print-header-includes.c
+++ 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 
-#include "Inputs/test.h"
+#include 
Index: clang/lib/Frontend/HeaderIncludeGen.cpp
===
--- clang/lib/Frontend/HeaderIncludeGen.cpp
+++ clang/lib/Frontend/HeaderIncludeGen.cpp
@@ -49,6 +49,18 @@
 
   void FileSkipped(const FileEntryRef , const Token ,
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 @@
 }
 
 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 .
   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 
-  // header.
   // FIXME: Identify headers in a more robust way than comparing their name to
   // "" and "" in a bunch of places.
-  if (ShowHeader && Reason == PPCallbacks::EnterFile &&
+  if (Reason == PPCallbacks::EnterFile &&
   UserLoc.getFilename() != StringRef("")) {
 PrintHeaderInfo(OutputFile, UserLoc.getFilename(), ShowDepth, IncludeDepth,
 MSStyle);
@@ -246,7 +251,7 @@
   if (!DepOpts.ShowSkippedHeaderIncludes)
 return;
 
-  if (!DepOpts.IncludeSystemHeaders && isSystem(FileType))
+  if (!ShouldShowHeader(FileType))
 return;
 
   PrintHeaderInfo(OutputFile, SkippedFile.getName(), ShowDepth,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153175: [Frontend] Don't output skipped includes from predefines

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

In D153175#4431923 , @hans wrote:

>> -H is supposed to skip outputting headers from -include command line
>> arguments, but -fshow-skipped-includes was outputting any skipped
>> includes encountered via -include.
>
> I was thrown off by the "-H is supposed to skip ... but 
> -fshow-skipped-includes was outputting ..."
>
> I suppose the point is that we do want `-fshow-skipped-includes` to show 
> skipped includes, but not the ones from `-include` or system headers (unless 
> `-sys-header-deps`)?
>
> lgtm if I got that right.

Yup, my description was kinda terrible :D Lemme try to clarify it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153175/new/

https://reviews.llvm.org/D153175

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153175: [Frontend] Don't output skipped includes from predefines

[Hans Wennborg via Phabricator via cfe-commits]

hans accepted this revision.
hans added a comment.
This revision is now accepted and ready to land.

> -H is supposed to skip outputting headers from -include command line
> arguments, but -fshow-skipped-includes was outputting any skipped
> includes encountered via -include.

I was thrown off by the "-H is supposed to skip ... but -fshow-skipped-includes 
was outputting ..."

I suppose the point is that we do want `-fshow-skipped-includes` to show 
skipped includes, but not the ones from `-include` or system headers (unless 
`-sys-header-deps`)?

lgtm if I got that right.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153175/new/

https://reviews.llvm.org/D153175

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D153175: [Frontend] Don't output skipped includes from predefines

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai created this revision.
smeenai added reviewers: hans, rnk, thakis.
Herald added a project: All.
smeenai requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

`-H` is supposed to skip outputting headers from `-include` command line
arguments, but `-fshow-skipped-includes` was outputting any skipped
includes encountered via `-include`. Add a check to prevent this, and
add tests for the interaction between `-fshow-skipped-includes` and
`-sys-header-deps` while I'm here.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153175

Files:
  clang/lib/Frontend/HeaderIncludeGen.cpp
  clang/test/Frontend/print-header-includes.c

Index: clang/test/Frontend/print-header-includes.c
===
--- clang/test/Frontend/print-header-includes.c
+++ 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 
-#include "Inputs/test.h"
+#include 
Index: clang/lib/Frontend/HeaderIncludeGen.cpp
===
--- clang/lib/Frontend/HeaderIncludeGen.cpp
+++ clang/lib/Frontend/HeaderIncludeGen.cpp
@@ -49,6 +49,18 @@
 
   void FileSkipped(const FileEntryRef , const Token ,
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 @@
 }
 
 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 .
   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 
-  // header.
   // FIXME: Identify headers in a more robust way than comparing their name to
   // "" and "" in a bunch of places.
-  if (ShowHeader && Reason == PPCallbacks::EnterFile &&
+  if (Reason == PPCallbacks::EnterFile &&
   UserLoc.getFilename() != StringRef("")) {
 PrintHeaderInfo(OutputFile, UserLoc.getFilename(), ShowDepth, IncludeDepth,
 MSStyle);
@@ -246,7 +251,7 @@
   if (!DepOpts.ShowSkippedHeaderIncludes)
 return;
 
-  if (!DepOpts.IncludeSystemHeaders && isSystem(FileType))
+  if (!ShouldShowHeader(FileType))
 return;
 
   PrintHeaderInfo(OutputFile, SkippedFile.getName(), ShowDepth,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits