From: Hans Wennborg Date: Thu, 12 Sep 2013 18:23:34 +0000 (+0000) Subject: Move Compilation::PrintJob and PrintDiagnosticJob into Job::Print. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=fc3389701ead32709ba84825e4c06651065da2c0;p=clang Move Compilation::PrintJob and PrintDiagnosticJob into Job::Print. 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 a little. 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. Differential Revision: http://llvm-reviews.chandlerc.com/D1653 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@190620 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/examples/clang-interpreter/main.cpp b/examples/clang-interpreter/main.cpp index 4c4ace8665..e00583d1a1 100644 --- a/examples/clang-interpreter/main.cpp +++ b/examples/clang-interpreter/main.cpp @@ -95,7 +95,7 @@ int main(int argc, const char **argv, char * const *envp) { if (Jobs.size() != 1 || !isa(*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 @@ int main(int argc, const char **argv, char * const *envp) { // 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"; } diff --git a/include/clang/Driver/Compilation.h b/include/clang/Driver/Compilation.h index 77a8d88706..3493e4f150 100644 --- a/include/clang/Driver/Compilation.h +++ b/include/clang/Driver/Compilation.h @@ -155,23 +155,6 @@ public: 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 diff --git a/include/clang/Driver/Job.h b/include/clang/Driver/Job.h index 68d6a66baa..17f0784bf3 100644 --- a/include/clang/Driver/Job.h +++ b/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 @@ public: 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 @@ public: 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 @@ public: 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); } diff --git a/lib/Driver/Compilation.cpp b/lib/Driver/Compilation.cpp index fdc6fc25c8..5e7babef0d 100644 --- a/lib/Driver/Compilation.cpp +++ b/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 @@ const DerivedArgList &Compilation::getArgsForToolChain(const ToolChain *TC, return *Entry; } -void Compilation::PrintJob(raw_ostream &OS, const Job &J, - const char *Terminator, bool Quote) const { - if (const Command *C = dyn_cast(&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(&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 . - SkipNextArg = true; - - // These flags are all of the form -Flag and are treated as two - // arguments. Therefore, we need to skip the flag and the next argument. - bool Res = llvm::StringSwitch(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(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). - if (FlagRef.startswith("-F") || FlagRef.startswith("-I")) - return true; - - return false; -} - -static bool quoteNextArg(const char *flag) { - return llvm::StringSwitch(flag) - .Case("-D", true) - .Default(false); -} - -void Compilation::PrintDiagnosticJob(raw_ostream &OS, const Job &J) const { - if (const Command *C = dyn_cast(&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(&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 @@ int Compilation::ExecuteCommand(const Command &C, 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; diff --git a/lib/Driver/Driver.cpp b/lib/Driver/Driver.cpp index 7461e99963..b1d5e61963 100644 --- a/lib/Driver/Driver.cpp +++ b/lib/Driver/Driver.cpp @@ -441,11 +441,11 @@ void Driver::generateCompilationDiagnostics(Compilation &C, std::string Cmd; llvm::raw_string_ostream OS(Cmd); if (FailingCommand) - C.PrintDiagnosticJob(OS, *FailingCommand); + FailingCommand->Print(OS, "\n", /*Quote*/ false, /*CrashReport*/ 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, /*CrashReport*/ true); OS.flush(); // Keep track of whether we produce any errors while trying to produce @@ -578,7 +578,7 @@ int Driver::ExecuteCompilation(const Compilation &C, SmallVectorImpl< std::pair > &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; } diff --git a/lib/Driver/Job.cpp b/lib/Driver/Job.cpp index 2906ef9290..116fbd0541 100644 --- a/lib/Driver/Job.cpp +++ b/lib/Driver/Job.cpp @@ -9,19 +9,109 @@ #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 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 and are treated as two + // arguments. Therefore, we need to skip the flag and the next argument. + bool Res = llvm::StringSwitch(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(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). + StringRef FlagRef(Flag); + if (FlagRef.startswith("-F") || FlagRef.startswith("-I")) + return 1; + + return 0; +} + +static bool quoteNextArg(const char *flag) { + return llvm::StringSwitch(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() { @@ -29,6 +119,12 @@ JobList::~JobList() { 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); } diff --git a/lib/Frontend/CreateInvocationFromCommandLine.cpp b/lib/Frontend/CreateInvocationFromCommandLine.cpp index acb93fa1b0..78f39d4298 100644 --- a/lib/Frontend/CreateInvocationFromCommandLine.cpp +++ b/lib/Frontend/CreateInvocationFromCommandLine.cpp @@ -56,7 +56,7 @@ clang::createInvocationFromCommandLine(ArrayRef ArgList, // 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 @@ clang::createInvocationFromCommandLine(ArrayRef ArgList, if (Jobs.size() != 1 || !isa(*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; } diff --git a/lib/Tooling/Tooling.cpp b/lib/Tooling/Tooling.cpp index b420ba934d..f9876daa6a 100644 --- a/lib/Tooling/Tooling.cpp +++ b/lib/Tooling/Tooling.cpp @@ -67,7 +67,7 @@ static const llvm::opt::ArgStringList *getCC1Arguments( if (Jobs.size() != 1 || !isa(*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 @@ bool ToolInvocation::runInvocation( // 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"; } diff --git a/test/Driver/crash-report.c b/test/Driver/crash-report.c index afac934714..528edfb0d8 100644 --- a/test/Driver/crash-report.c +++ b/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/