Hi rnk,
This moves the code to Job.cpp, which seems like a more natural fit, and
replaces the "is this a JobList? is this a Command?" logic with a virtual
function call.
It also removes the code duplication between PrintJob and PrintDiagnosticJob
and simplifies the code.
There's no functionality change here, except that the Executable is now always
printed within quotes, whereas it would previously not be quoted in crash
reports, which I think was a bug.
http://llvm-reviews.chandlerc.com/D1653
Files:
examples/clang-interpreter/main.cpp
include/clang/Driver/Compilation.h
include/clang/Driver/Job.h
lib/Driver/Compilation.cpp
lib/Driver/Driver.cpp
lib/Driver/Job.cpp
lib/Frontend/CreateInvocationFromCommandLine.cpp
lib/Tooling/Tooling.cpp
test/Driver/crash-report.c
Index: examples/clang-interpreter/main.cpp
===================================================================
--- examples/clang-interpreter/main.cpp
+++ examples/clang-interpreter/main.cpp
@@ -95,7 +95,7 @@
if (Jobs.size() != 1 || !isa<driver::Command>(*Jobs.begin())) {
SmallString<256> Msg;
llvm::raw_svector_ostream OS(Msg);
- C->PrintJob(OS, C->getJobs(), "; ", true);
+ Jobs.Print(OS, "; ", true);
Diags.Report(diag::err_fe_expected_compiler_job) << OS.str();
return 1;
}
@@ -118,7 +118,7 @@
// Show the invocation, with -v.
if (CI->getHeaderSearchOpts().Verbose) {
llvm::errs() << "clang invocation:\n";
- C->PrintJob(llvm::errs(), C->getJobs(), "\n", true);
+ Jobs.Print(llvm::errs(), "\n", true);
llvm::errs() << "\n";
}
Index: include/clang/Driver/Compilation.h
===================================================================
--- include/clang/Driver/Compilation.h
+++ include/clang/Driver/Compilation.h
@@ -155,23 +155,6 @@
const JobAction *JA,
bool IssueErrors = false) const;
- /// PrintJob - Print one job in -### format.
- ///
- /// \param OS - The stream to print on.
- /// \param J - The job to print.
- /// \param Terminator - A string to print at the end of the line.
- /// \param Quote - Should separate arguments be quoted.
- void PrintJob(raw_ostream &OS, const Job &J,
- const char *Terminator, bool Quote) const;
-
- /// PrintDiagnosticJob - Print one job in -### format, but with the
- /// superfluous options removed, which are not necessary for
- /// reproducing the crash.
- ///
- /// \param OS - The stream to print on.
- /// \param J - The job to print.
- void PrintDiagnosticJob(raw_ostream &OS, const Job &J) const;
-
/// ExecuteCommand - Execute an actual command.
///
/// \param FailingCommand - For non-zero results, this will be set to the
Index: include/clang/Driver/Job.h
===================================================================
--- include/clang/Driver/Job.h
+++ include/clang/Driver/Job.h
@@ -14,6 +14,10 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/Option/Option.h"
+namespace llvm {
+ class raw_ostream;
+}
+
namespace clang {
namespace driver {
class Action;
@@ -38,13 +42,20 @@
virtual ~Job();
JobClass getKind() const { return Kind; }
+
+ /// Print - Print this Job in -### format.
+ ///
+ /// \param OS - The stream to print on.
+ /// \param Terminator - A string to print at the end of the line.
+ /// \param Quote - Should separate arguments be quoted.
+ /// \param CrashReport - Whether to print for inclusion in a crash report.
+ virtual void Print(llvm::raw_ostream &OS, const char *Terminator,
+ bool Quote, bool CrashReport = false) const = 0;
};
/// Command - An executable path/name and argument vector to
/// execute.
class Command : public Job {
- virtual void anchor();
-
/// Source - The action which caused the creation of this job.
const Action &Source;
@@ -62,6 +73,9 @@
Command(const Action &_Source, const Tool &_Creator, const char *_Executable,
const llvm::opt::ArgStringList &_Arguments);
+ virtual void Print(llvm::raw_ostream &OS, const char *Terminator,
+ bool Quote, bool CrashReport = false) const;
+
/// getSource - Return the Action which caused the creation of this job.
const Action &getSource() const { return Source; }
@@ -92,6 +106,9 @@
JobList();
virtual ~JobList();
+ virtual void Print(llvm::raw_ostream &OS, const char *Terminator,
+ bool Quote, bool CrashReport = false) const;
+
/// Add a job to the list (taking ownership).
void addJob(Job *J) { Jobs.push_back(J); }
Index: lib/Driver/Compilation.cpp
===================================================================
--- lib/Driver/Compilation.cpp
+++ lib/Driver/Compilation.cpp
@@ -14,7 +14,6 @@
#include "clang/Driver/Options.h"
#include "clang/Driver/ToolChain.h"
#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringSwitch.h"
#include "llvm/Option/ArgList.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Program.h"
@@ -71,136 +70,6 @@
return *Entry;
}
-void Compilation::PrintJob(raw_ostream &OS, const Job &J,
- const char *Terminator, bool Quote) const {
- if (const Command *C = dyn_cast<Command>(&J)) {
- OS << " \"" << C->getExecutable() << '"';
- for (ArgStringList::const_iterator it = C->getArguments().begin(),
- ie = C->getArguments().end(); it != ie; ++it) {
- OS << ' ';
- if (!Quote && !std::strpbrk(*it, " \"\\$")) {
- OS << *it;
- continue;
- }
-
- // Quote the argument and escape shell special characters; this isn't
- // really complete but is good enough.
- OS << '"';
- for (const char *s = *it; *s; ++s) {
- if (*s == '"' || *s == '\\' || *s == '$')
- OS << '\\';
- OS << *s;
- }
- OS << '"';
- }
- OS << Terminator;
- } else {
- const JobList *Jobs = cast<JobList>(&J);
- for (JobList::const_iterator
- it = Jobs->begin(), ie = Jobs->end(); it != ie; ++it)
- PrintJob(OS, **it, Terminator, Quote);
- }
-}
-
-static bool skipArg(const char *Flag, bool &SkipNextArg) {
- StringRef FlagRef(Flag);
-
- // Assume we're going to see -Flag <Arg>.
- SkipNextArg = true;
-
- // These flags are all of the form -Flag <Arg> and are treated as two
- // arguments. Therefore, we need to skip the flag and the next argument.
- bool Res = llvm::StringSwitch<bool>(Flag)
- .Cases("-I", "-MF", "-MT", "-MQ", true)
- .Cases("-o", "-coverage-file", "-dependency-file", true)
- .Cases("-fdebug-compilation-dir", "-idirafter", true)
- .Cases("-include", "-include-pch", "-internal-isystem", true)
- .Cases("-internal-externc-isystem", "-iprefix", "-iwithprefix", true)
- .Cases("-iwithprefixbefore", "-isysroot", "-isystem", "-iquote", true)
- .Cases("-resource-dir", "-serialize-diagnostic-file", true)
- .Case("-dwarf-debug-flags", true)
- .Default(false);
-
- // Match found.
- if (Res)
- return Res;
-
- // The remaining flags are treated as a single argument.
- SkipNextArg = false;
-
- // These flags are all of the form -Flag and have no second argument.
- Res = llvm::StringSwitch<bool>(Flag)
- .Cases("-M", "-MM", "-MG", "-MP", "-MD", true)
- .Case("-MMD", true)
- .Default(false);
-
- // Match found.
- if (Res)
- return Res;
-
- // These flags are treated as a single argument (e.g., -F<Dir>).
- if (FlagRef.startswith("-F") || FlagRef.startswith("-I"))
- return true;
-
- return false;
-}
-
-static bool quoteNextArg(const char *flag) {
- return llvm::StringSwitch<bool>(flag)
- .Case("-D", true)
- .Default(false);
-}
-
-void Compilation::PrintDiagnosticJob(raw_ostream &OS, const Job &J) const {
- if (const Command *C = dyn_cast<Command>(&J)) {
- OS << C->getExecutable();
- unsigned QuoteNextArg = 0;
- for (ArgStringList::const_iterator it = C->getArguments().begin(),
- ie = C->getArguments().end(); it != ie; ++it) {
-
- bool SkipNext;
- if (skipArg(*it, SkipNext)) {
- if (SkipNext) ++it;
- continue;
- }
-
- if (!QuoteNextArg)
- QuoteNextArg = quoteNextArg(*it) ? 2 : 0;
-
- OS << ' ';
-
- if (QuoteNextArg == 1)
- OS << '"';
-
- if (!std::strpbrk(*it, " \"\\$")) {
- OS << *it;
- } else {
- // Quote the argument and escape shell special characters; this isn't
- // really complete but is good enough.
- OS << '"';
- for (const char *s = *it; *s; ++s) {
- if (*s == '"' || *s == '\\' || *s == '$')
- OS << '\\';
- OS << *s;
- }
- OS << '"';
- }
-
- if (QuoteNextArg) {
- if (QuoteNextArg == 1)
- OS << '"';
- --QuoteNextArg;
- }
- }
- OS << '\n';
- } else {
- const JobList *Jobs = cast<JobList>(&J);
- for (JobList::const_iterator
- it = Jobs->begin(), ie = Jobs->end(); it != ie; ++it)
- PrintDiagnosticJob(OS, **it);
- }
-}
-
bool Compilation::CleanupFile(const char *File, bool IssueErrors) const {
std::string P(File);
@@ -288,7 +157,7 @@
if (getDriver().CCPrintOptions)
*OS << "[Logging clang options]";
- PrintJob(*OS, C, "\n", /*Quote=*/getDriver().CCPrintOptions);
+ C.Print(*OS, "\n", /*Quote=*/getDriver().CCPrintOptions);
if (OS != &llvm::errs())
delete OS;
Index: lib/Driver/Driver.cpp
===================================================================
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -441,11 +441,11 @@
std::string Cmd;
llvm::raw_string_ostream OS(Cmd);
if (FailingCommand)
- C.PrintDiagnosticJob(OS, *FailingCommand);
+ FailingCommand->Print(OS, "\n", /*Quote*/ false, /*CrashReportFlagsOnly*/ true);
else
// Crash triggered by FORCE_CLANG_DIAGNOSTICS_CRASH, which doesn't have an
// associated FailingCommand, so just pass all jobs.
- C.PrintDiagnosticJob(OS, C.getJobs());
+ C.getJobs().Print(OS, "\n", /*Quote*/ false, /*CrashReportFlagsOnly*/ true);
OS.flush();
// Keep track of whether we produce any errors while trying to produce
@@ -578,7 +578,7 @@
SmallVectorImpl< std::pair<int, const Command *> > &FailingCommands) const {
// Just print if -### was present.
if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH)) {
- C.PrintJob(llvm::errs(), C.getJobs(), "\n", true);
+ C.getJobs().Print(llvm::errs(), "\n", true);
return 0;
}
Index: lib/Driver/Job.cpp
===================================================================
--- lib/Driver/Job.cpp
+++ lib/Driver/Job.cpp
@@ -9,26 +9,122 @@
#include "clang/Driver/Job.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/raw_ostream.h"
#include <cassert>
using namespace clang::driver;
+using llvm::raw_ostream;
+using llvm::StringRef;
Job::~Job() {}
-void Command::anchor() {}
-
Command::Command(const Action &_Source, const Tool &_Creator,
const char *_Executable,
const llvm::opt::ArgStringList &_Arguments)
: Job(CommandClass), Source(_Source), Creator(_Creator),
Executable(_Executable), Arguments(_Arguments) {}
+static int skipArgs(const char *Flag) {
+ // These flags are all of the form -Flag <Arg> and are treated as two
+ // arguments. Therefore, we need to skip the flag and the next argument.
+ bool Res = llvm::StringSwitch<bool>(Flag)
+ .Cases("-I", "-MF", "-MT", "-MQ", true)
+ .Cases("-o", "-coverage-file", "-dependency-file", true)
+ .Cases("-fdebug-compilation-dir", "-idirafter", true)
+ .Cases("-include", "-include-pch", "-internal-isystem", true)
+ .Cases("-internal-externc-isystem", "-iprefix", "-iwithprefix", true)
+ .Cases("-iwithprefixbefore", "-isysroot", "-isystem", "-iquote", true)
+ .Cases("-resource-dir", "-serialize-diagnostic-file", true)
+ .Case("-dwarf-debug-flags", true)
+ .Default(false);
+
+ // Match found.
+ if (Res)
+ return 2;
+
+ // The remaining flags are treated as a single argument.
+
+ // These flags are all of the form -Flag and have no second argument.
+ Res = llvm::StringSwitch<bool>(Flag)
+ .Cases("-M", "-MM", "-MG", "-MP", "-MD", true)
+ .Case("-MMD", true)
+ .Default(false);
+
+ // Match found.
+ if (Res)
+ return 1;
+
+ // These flags are treated as a single argument (e.g., -F<Dir>).
+ StringRef FlagRef(Flag);
+ if (FlagRef.startswith("-F") || FlagRef.startswith("-I"))
+ return 1;
+
+ return 0;
+}
+
+static bool quoteNextArg(const char *flag) {
+ return llvm::StringSwitch<bool>(flag)
+ .Case("-D", true)
+ .Default(false);
+}
+
+static void PrintArg(raw_ostream &OS, const char *Arg, bool Quote) {
+ const bool Escape = std::strpbrk(Arg, "\"\\$");
+
+ if (!Quote && !Escape) {
+ OS << Arg;
+ return;
+ }
+
+ // Quote and escape. This isn't really complete, but good enough.
+ OS << '"';
+ while (const char c = *Arg++) {
+ if (c == '"' || c == '\\' || c == '$')
+ OS << '\\';
+ OS << c;
+ }
+ OS << '"';
+}
+
+void Command::Print(raw_ostream &OS, const char *Terminator, bool Quote,
+ bool CrashReport) const {
+ OS << " \"" << Executable << '"';
+
+ for (size_t i = 0, e = Arguments.size(); i < e; ++i) {
+ const char *const Arg = Arguments[i];
+
+ if (CrashReport) {
+ if (int Skip = skipArgs(Arg)) {
+ i += Skip - 1;
+ continue;
+ }
+ }
+
+ OS << ' ';
+ PrintArg(OS, Arg, Quote);
+
+ if (CrashReport && quoteNextArg(Arg) && i + 1 < e) {
+ OS << ' ';
+ PrintArg(OS, Arguments[++i], true);
+ }
+ }
+ OS << Terminator;
+}
+
JobList::JobList() : Job(JobListClass) {}
JobList::~JobList() {
for (iterator it = begin(), ie = end(); it != ie; ++it)
delete *it;
}
+void JobList::Print(raw_ostream &OS, const char *Terminator, bool Quote,
+ bool CrashReport) const {
+ for (const_iterator it = begin(), ie = end(); it != ie; ++it)
+ (*it)->Print(OS, Terminator, Quote, CrashReport);
+}
+
void JobList::clear() {
DeleteContainerPointers(Jobs);
}
Index: lib/Frontend/CreateInvocationFromCommandLine.cpp
===================================================================
--- lib/Frontend/CreateInvocationFromCommandLine.cpp
+++ lib/Frontend/CreateInvocationFromCommandLine.cpp
@@ -56,7 +56,7 @@
// Just print the cc1 options if -### was present.
if (C->getArgs().hasArg(driver::options::OPT__HASH_HASH_HASH)) {
- C->PrintJob(llvm::errs(), C->getJobs(), "\n", true);
+ C->getJobs().Print(llvm::errs(), "\n", true);
return 0;
}
@@ -66,7 +66,7 @@
if (Jobs.size() != 1 || !isa<driver::Command>(*Jobs.begin())) {
SmallString<256> Msg;
llvm::raw_svector_ostream OS(Msg);
- C->PrintJob(OS, C->getJobs(), "; ", true);
+ Jobs.Print(OS, "; ", true);
Diags->Report(diag::err_fe_expected_compiler_job) << OS.str();
return 0;
}
Index: lib/Tooling/Tooling.cpp
===================================================================
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -67,7 +67,7 @@
if (Jobs.size() != 1 || !isa<clang::driver::Command>(*Jobs.begin())) {
SmallString<256> error_msg;
llvm::raw_svector_ostream error_stream(error_msg);
- Compilation->PrintJob(error_stream, Compilation->getJobs(), "; ", true);
+ Jobs.Print(error_stream, "; ", true);
Diagnostics->Report(clang::diag::err_fe_expected_compiler_job)
<< error_stream.str();
return NULL;
@@ -185,7 +185,7 @@
// Show the invocation, with -v.
if (Invocation->getHeaderSearchOpts().Verbose) {
llvm::errs() << "clang Invocation:\n";
- Compilation->PrintJob(llvm::errs(), Compilation->getJobs(), "\n", true);
+ Compilation->getJobs().Print(llvm::errs(), "\n", true);
llvm::errs() << "\n";
}
Index: test/Driver/crash-report.c
===================================================================
--- test/Driver/crash-report.c
+++ test/Driver/crash-report.c
@@ -22,6 +22,7 @@
// CHECK-NEXT: note: diagnostic msg: {{.*}}.c
FOO
// CHECKSRC: FOO
+// CHECKSH: -cc1
// CHECKSH: -D "FOO=BAR"
// CHECKSH-NOT: -F/tmp/
// CHECKSH-NOT: -I /tmp/
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
