[PATCH] D46394: [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP or /P
fxb created this revision.
fxb added reviewers: hans, erichkeane, thakis.
Herald added a subscriber: cfe-commits.
This replicates 'cl.exe' behavior and allows for both preprocessor output and
dependency information to be extraced with a single compiler invocation.
This is especially useful for compiler caching with tools like Mozilla's
sccache.
See: https://github.com/mozilla/sccache/issues/246
Repository:
rC Clang
https://reviews.llvm.org/D46394
Files:
include/clang/Frontend/DependencyOutputOptions.h
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Frontend/HeaderIncludeGen.cpp
test/Driver/cl-options.c
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -198,10 +198,8 @@
// RUN: %clang_cl /E /showIncludes -### -- %s 2>&1 | FileCheck
-check-prefix=showIncludes_E %s
// RUN: %clang_cl /EP /showIncludes -### -- %s 2>&1 | FileCheck
-check-prefix=showIncludes_E %s
// RUN: %clang_cl /E /EP /showIncludes -### -- %s 2>&1 | FileCheck
-check-prefix=showIncludes_E %s
-// showIncludes_E: warning: argument unused during compilation:
'--show-includes'
-
-// RUN: %clang_cl /EP /P /showIncludes -### -- %s 2>&1 | FileCheck
-check-prefix=showIncludes_E_And_P %s
-// showIncludes_E_And_P-NOT: warning: argument unused during compilation:
'--show-includes'
+// RUN: %clang_cl /EP /P /showIncludes -### -- %s 2>&1 | FileCheck
-check-prefix=showIncludes_E %s
+// showIncludes_E-NOT: warning: argument unused during compilation:
'--show-includes'
// /source-charset: should warn on everything except UTF-8.
// RUN: %clang_cl /source-charset:utf-16 -### -- %s 2>&1 | FileCheck
-check-prefix=source-charset-utf-16 %s
Index: lib/Frontend/HeaderIncludeGen.cpp
===
--- lib/Frontend/HeaderIncludeGen.cpp
+++ lib/Frontend/HeaderIncludeGen.cpp
@@ -80,7 +80,9 @@
const DependencyOutputOptions &DepOpts,
bool ShowAllHeaders, StringRef OutputPath,
bool ShowDepth, bool MSStyle) {
- raw_ostream *OutputFile = MSStyle ? &llvm::outs() : &llvm::errs();
+ raw_ostream *OutputFile = &llvm::errs();
+ if (MSStyle && !DepOpts.PrintShowIncludesToStderr)
+OutputFile = &llvm::outs();
bool OwnsOutputFile = false;
// Open the output file, if used.
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1131,6 +1131,12 @@
Opts.HeaderIncludeOutputFile = Args.getLastArgValue(OPT_header_include_file);
Opts.AddMissingHeaderDeps = Args.hasArg(OPT_MG);
Opts.PrintShowIncludes = Args.hasArg(OPT_show_includes);
+ // Writing both /showIncludes and preprocessor output to stdout
+ // would produce interleaved output, so use stderr for /showIncludes.
+ // This behaves the same as cl.exe, when /E, /EP or /P are passed.
+ if (Opts.PrintShowIncludes &&
+ (Args.hasArg(options::OPT_E) || Args.hasArg(options::OPT_P)))
+Opts.PrintShowIncludesToStderr = true;
Opts.DOTOutputFile = Args.getLastArgValue(OPT_dependency_dot);
Opts.ModuleDependencyOutputDir =
Args.getLastArgValue(OPT_module_dependency_dir);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -5077,13 +5077,8 @@
CmdArgs.push_back("--dependent-lib=oldnames");
}
- // Both /showIncludes and /E (and /EP) write to stdout. Allowing both
- // would produce interleaved output, so ignore /showIncludes in such cases.
- if ((!Args.hasArg(options::OPT_E) && !Args.hasArg(options::OPT__SLASH_EP)) ||
- (Args.hasArg(options::OPT__SLASH_P) &&
- Args.hasArg(options::OPT__SLASH_EP) && !Args.hasArg(options::OPT_E)))
-if (Arg *A = Args.getLastArg(options::OPT_show_includes))
- A->render(Args, CmdArgs);
+ if (Arg *A = Args.getLastArg(options::OPT_show_includes))
+A->render(Args, CmdArgs);
// This controls whether or not we emit RTTI data for polymorphic types.
if (Args.hasFlag(options::OPT__SLASH_GR_, options::OPT__SLASH_GR,
Index: include/clang/Frontend/DependencyOutputOptions.h
===
--- include/clang/Frontend/DependencyOutputOptions.h
+++ include/clang/Frontend/DependencyOutputOptions.h
@@ -29,6 +29,8 @@
/// problems.
unsigned AddMissingHeaderDeps : 1; ///< Add missing headers to dependency
list
unsigned PrintShowIncludes : 1; ///< Print cl.exe style /showIncludes info.
+ unsigned
+ PrintShowIncludesToStderr : 1; ///< Print /showIncludes output to stderr.
unsigned IncludeModuleFiles : 1; ///< Include module file dependencies.
/// The form
[PATCH] D46394: [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP or /P
fxb added a comment. This is my first patch to clang, so any feedback regarding implementation appreciated! Also, let me know if you have any suggestions on how to add more extensive tests for this. Repository: rC Clang https://reviews.llvm.org/D46394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46394: [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP or /P
fxb added inline comments. Comment at: include/clang/Frontend/DependencyOutputOptions.h:31 unsigned AddMissingHeaderDeps : 1; ///< Add missing headers to dependency list unsigned PrintShowIncludes : 1; ///< Print cl.exe style /showIncludes info. + unsigned erichkeane wrote: > Doing these two as separate options is a touch annoying. First, there is an > extra state that ends up being possible but ignored. I'd prefer making a > "PrintShowIncludeDestination : 2" (name open for proper bikeshed) that > contains an enum so that the states are explicit. Something like: > None, > StdOut, > StdErr > > That way, the 4th state is explicitly unused. Yes, that definitely makes sense and is a lot nicer than having these two options, which I found quite ugly as well. I will fix that and upload a new patch set. Comment at: test/Driver/cl-options.c:203 - -// RUN: %clang_cl /EP /P /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E_And_P %s -// showIncludes_E_And_P-NOT: warning: argument unused during compilation: '--show-includes' erichkeane wrote: > I'm perhaps missing something here... why did "/EP /P /showIncludes" > previously NOT warn? /P will preprocess to a file, so it was compatible with /showIncludes, which was printed to stdout. With this change all preprocessor options are now possible to use in combination with /showIncludes, since its output will be written to stderr, like cl.exe does. I was actually not sure if I shall keep these tests at all after my change or just invert them, like I ended up doing here. There is probably a better way to actually test that /showIncludes output will end up on stderr, but with my limited experience I didn't figure that out yet. Let me know if you have any ideas how to improve these tests. Repository: rC Clang https://reviews.llvm.org/D46394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46394: [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP or /P
fxb updated this revision to Diff 145030.
fxb added a comment.
Updated the code to use an enum called `ShowIncludesDestination`
https://reviews.llvm.org/D46394
Files:
include/clang/Frontend/DependencyOutputOptions.h
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInstance.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Frontend/HeaderIncludeGen.cpp
test/Driver/cl-options.c
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -198,10 +198,8 @@
// RUN: %clang_cl /E /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
// RUN: %clang_cl /EP /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
// RUN: %clang_cl /E /EP /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
-// showIncludes_E: warning: argument unused during compilation: '--show-includes'
-
-// RUN: %clang_cl /EP /P /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E_And_P %s
-// showIncludes_E_And_P-NOT: warning: argument unused during compilation: '--show-includes'
+// RUN: %clang_cl /EP /P /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
+// showIncludes_E-NOT: warning: argument unused during compilation: '--show-includes'
// /source-charset: should warn on everything except UTF-8.
// RUN: %clang_cl /source-charset:utf-16 -### -- %s 2>&1 | FileCheck -check-prefix=source-charset-utf-16 %s
Index: lib/Frontend/HeaderIncludeGen.cpp
===
--- lib/Frontend/HeaderIncludeGen.cpp
+++ lib/Frontend/HeaderIncludeGen.cpp
@@ -80,9 +80,23 @@
const DependencyOutputOptions &DepOpts,
bool ShowAllHeaders, StringRef OutputPath,
bool ShowDepth, bool MSStyle) {
- raw_ostream *OutputFile = MSStyle ? &llvm::outs() : &llvm::errs();
+ raw_ostream *OutputFile = &llvm::errs();
bool OwnsOutputFile = false;
+ // Choose output stream, when printing in cl.exe /showIncludes style.
+ if (MSStyle) {
+switch (DepOpts.ShowIncludesDestination) {
+default:
+ llvm_unreachable("Invalid destination for /showIncludes output!");
+case ShowIncludesDestination::Stderr:
+ OutputFile = &llvm::errs();
+ break;
+case ShowIncludesDestination::Stdout:
+ OutputFile = &llvm::outs();
+ break;
+}
+ }
+
// Open the output file, if used.
if (!OutputPath.empty()) {
std::error_code EC;
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1130,7 +1130,17 @@
Opts.ShowHeaderIncludes = Args.hasArg(OPT_H);
Opts.HeaderIncludeOutputFile = Args.getLastArgValue(OPT_header_include_file);
Opts.AddMissingHeaderDeps = Args.hasArg(OPT_MG);
- Opts.PrintShowIncludes = Args.hasArg(OPT_show_includes);
+ if (Args.hasArg(OPT_show_includes)) {
+// Writing both /showIncludes and preprocessor output to stdout
+// would produce interleaved output, so use stderr for /showIncludes.
+// This behaves the same as cl.exe, when /E, /EP or /P are passed.
+if (Args.hasArg(options::OPT_E) || Args.hasArg(options::OPT_P))
+ Opts.ShowIncludesDestination = ShowIncludesDestination::Stderr;
+else
+ Opts.ShowIncludesDestination = ShowIncludesDestination::Stdout;
+ } else {
+Opts.ShowIncludesDestination = ShowIncludesDestination::None;
+ }
Opts.DOTOutputFile = Args.getLastArgValue(OPT_dependency_dot);
Opts.ModuleDependencyOutputDir =
Args.getLastArgValue(OPT_module_dependency_dir);
Index: lib/Frontend/CompilerInstance.cpp
===
--- lib/Frontend/CompilerInstance.cpp
+++ lib/Frontend/CompilerInstance.cpp
@@ -462,7 +462,7 @@
/*ShowDepth=*/false);
}
- if (DepOpts.PrintShowIncludes) {
+ if (DepOpts.ShowIncludesDestination != ShowIncludesDestination::None) {
AttachHeaderIncludeGen(*PP, DepOpts,
/*ShowAllHeaders=*/true, /*OutputPath=*/"",
/*ShowDepth=*/true, /*MSStyle=*/true);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -5077,13 +5077,8 @@
CmdArgs.push_back("--dependent-lib=oldnames");
}
- // Both /showIncludes and /E (and /EP) write to stdout. Allowing both
- // would produce interleaved output, so ignore /showIncludes in such cases.
- if ((!Args.hasArg(options::OPT_E) && !Args.hasArg(options::OPT__SLASH_EP)) ||
- (Args.hasArg(options::OPT__SLASH_P) &&
- Args.hasArg(options::OPT__SLASH_EP) && !Args.hasArg(options::OPT_E)))
-if (Arg *A = Args.getLastArg(options::OPT_sho
[PATCH] D46394: [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP or /P
fxb added a comment. @erichkeane That would be great! Thanks! https://reviews.llvm.org/D46394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46394: [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP or /P
fxb added a comment. I'll have a look at these issues tomorrow and submit a new patch then. Thanks for all your help so far! https://reviews.llvm.org/D46394 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46394: [clang-cl] Print /showIncludes to stderr, if used in combination with /E, /EP or /P
fxb updated this revision to Diff 145196.
fxb added a comment.
1. Fixed the compile error caused by re-using the name
`ShowIncludesDestination`. The member variable is now named `ShowIncludesDest`.
2. Fixed `test/Frontend/print-header-includes.c` to test both cases:
- If only `--show-includes` is passed, includes are printed on stdout.
- If both `--show-includes` and `-E` are passed, includes are printed on
stderr.
https://reviews.llvm.org/D46394
Files:
include/clang/Frontend/DependencyOutputOptions.h
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInstance.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Frontend/HeaderIncludeGen.cpp
test/Driver/cl-options.c
test/Frontend/print-header-includes.c
Index: test/Frontend/print-header-includes.c
===
--- test/Frontend/print-header-includes.c
+++ test/Frontend/print-header-includes.c
@@ -5,16 +5,24 @@
// CHECK: . {{.*test.h}}
// CHECK: .. {{.*test2.h}}
-// RUN: %clang_cc1 -I%S -include Inputs/test3.h -E --show-includes -o /dev/null %s | \
-// RUN: FileCheck --strict-whitespace --check-prefix=MS %s
-// MS-NOT:
-// MS: Note: including file: {{[^ ]*test3.h}}
-// MS: Note: including file: {{[^ ]*test.h}}
-// MS: Note: including file: {{[^ ]*test2.h}}
-// MS-NOT: Note
+// RUN: %clang_cc1 -I%S -include Inputs/test3.h --show-includes -o /dev/null %s | \
+// RUN: FileCheck --strict-whitespace --check-prefix=MS-STDOUT %s
+// MS-STDOUT-NOT:
+// MS-STDOUT: Note: including file: {{[^ ]*test3.h}}
+// MS-STDOUT: Note: including file: {{[^ ]*test.h}}
+// MS-STDOUT: Note: including file: {{[^ ]*test2.h}}
+// MS-STDOUT-NOT: Note
+
+// RUN: %clang_cc1 -I%S -include Inputs/test3.h -E --show-includes -o /dev/null %s 2> %t.stderr
+// RUN: FileCheck --strict-whitespace --check-prefix=MS-STDERR < %t.stderr %s
+// MS-STDERR-NOT:
+// MS-STDERR: Note: including file: {{[^ ]*test3.h}}
+// MS-STDERR: Note: including file: {{[^ ]*test.h}}
+// MS-STDERR: Note: including file: {{[^ ]*test2.h}}
+// MS-STDERR-NOT: Note
// RUN: echo "fun:foo" > %t.blacklist
-// RUN: %clang_cc1 -I%S -fsanitize=address -fdepfile-entry=%t.blacklist -E --show-includes -o /dev/null %s | \
+// RUN: %clang_cc1 -I%S -fsanitize=address -fdepfile-entry=%t.blacklist --show-includes -o /dev/null %s | \
// RUN: FileCheck --strict-whitespace --check-prefix=MS-BLACKLIST %s
// MS-BLACKLIST: Note: including file: {{[^ ]*\.blacklist}}
// MS-BLACKLIST: Note: including file: {{[^ ]*test.h}}
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -198,10 +198,8 @@
// RUN: %clang_cl /E /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
// RUN: %clang_cl /EP /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
// RUN: %clang_cl /E /EP /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
-// showIncludes_E: warning: argument unused during compilation: '--show-includes'
-
-// RUN: %clang_cl /EP /P /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E_And_P %s
-// showIncludes_E_And_P-NOT: warning: argument unused during compilation: '--show-includes'
+// RUN: %clang_cl /EP /P /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
+// showIncludes_E-NOT: warning: argument unused during compilation: '--show-includes'
// /source-charset: should warn on everything except UTF-8.
// RUN: %clang_cl /source-charset:utf-16 -### -- %s 2>&1 | FileCheck -check-prefix=source-charset-utf-16 %s
Index: lib/Frontend/HeaderIncludeGen.cpp
===
--- lib/Frontend/HeaderIncludeGen.cpp
+++ lib/Frontend/HeaderIncludeGen.cpp
@@ -80,9 +80,23 @@
const DependencyOutputOptions &DepOpts,
bool ShowAllHeaders, StringRef OutputPath,
bool ShowDepth, bool MSStyle) {
- raw_ostream *OutputFile = MSStyle ? &llvm::outs() : &llvm::errs();
+ raw_ostream *OutputFile = &llvm::errs();
bool OwnsOutputFile = false;
+ // Choose output stream, when printing in cl.exe /showIncludes style.
+ if (MSStyle) {
+switch (DepOpts.ShowIncludesDest) {
+default:
+ llvm_unreachable("Invalid destination for /showIncludes output!");
+case ShowIncludesDestination::Stderr:
+ OutputFile = &llvm::errs();
+ break;
+case ShowIncludesDestination::Stdout:
+ OutputFile = &llvm::outs();
+ break;
+}
+ }
+
// Open the output file, if used.
if (!OutputPath.empty()) {
std::error_code EC;
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1130,7 +1130,17 @@
Opts.ShowHeaderIncludes = Args.hasArg(OPT_H);
Opts.H
