r241310 - Driver: Remove the Job class. NFC
Justin Bogner
mail at justinbogner.com
Fri Jul 3 01:05:30 PDT 2015
Sean Silva <chisophugis at gmail.com> writes:
> Do we need updates to http://clang.llvm.org/docs/DriverInternals.html ?
Reworded a little in r241327.
> -- Sean Silva
>
> On Thu, Jul 2, 2015 at 3:52 PM, Justin Bogner <mail at justinbogner.com> wrote:
>
> Author: bogner
> Date: Thu Jul 2 17:52:08 2015
> New Revision: 241310
>
> URL: http://llvm.org/viewvc/llvm-project?rev=241310&view=rev
> Log:
> Driver: Remove the Job class. NFC
>
> We had a strange relationship here where we made a list of Jobs
> inherit from a single Job, but there weren't actually any places where
> this arbitrary nesting was used or needed.
>
> Simplify all of this by removing Job entirely and updating all of the
> users to either work with a JobList or a single Command.
>
> Modified:
> cfe/trunk/include/clang/Driver/Compilation.h
> cfe/trunk/include/clang/Driver/Driver.h
> cfe/trunk/include/clang/Driver/Job.h
> cfe/trunk/lib/Driver/Compilation.cpp
> cfe/trunk/lib/Driver/Driver.cpp
> cfe/trunk/lib/Driver/Job.cpp
> cfe/trunk/lib/Tooling/CompilationDatabase.cpp
>
> Modified: cfe/trunk/include/clang/Driver/Compilation.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/
> Compilation.h?rev=241310&r1=241309&r2=241310&view=diff
> ==========================================================================
> ====
> --- cfe/trunk/include/clang/Driver/Compilation.h (original)
> +++ cfe/trunk/include/clang/Driver/Compilation.h Thu Jul 2 17:52:08 2015
> @@ -169,8 +169,9 @@ public:
> ///
> /// \param FailingCommands - For non-zero results, this will be a
> vector of
> /// failing commands and their associated result code.
> - void ExecuteJob(const Job &J,
> - SmallVectorImpl< std::pair<int, const Command *> > &FailingCommands)
> const;
> + void ExecuteJobs(
> + const JobList &Jobs,
> + SmallVectorImpl<std::pair<int, const Command *>> &FailingCommands)
> const;
>
> /// initCompilationForDiagnostics - Remove stale state and suppress
> output
> /// so compilation can be reexecuted to generate additional diagnostic
>
> Modified: cfe/trunk/include/clang/Driver/Driver.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/
> Driver.h?rev=241310&r1=241309&r2=241310&view=diff
> ==========================================================================
> ====
> --- cfe/trunk/include/clang/Driver/Driver.h (original)
> +++ cfe/trunk/include/clang/Driver/Driver.h Thu Jul 2 17:52:08 2015
> @@ -42,7 +42,7 @@ namespace driver {
> class Command;
> class Compilation;
> class InputInfo;
> - class Job;
> + class JobList;
> class JobAction;
> class SanitizerArgs;
> class ToolChain;
> @@ -195,7 +195,7 @@ private:
> llvm::opt::Arg **FinalPhaseArg = nullptr)
> const;
>
> // Before executing jobs, sets up response files for commands that need
> them.
> - void setUpResponseFiles(Compilation &C, Job &J);
> + void setUpResponseFiles(Compilation &C, Command &Cmd);
>
> void generatePrefixedToolNames(const char *Tool, const ToolChain &TC,
> SmallVectorImpl<std::string> &Names)
> const;
>
> Modified: cfe/trunk/include/clang/Driver/Job.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/
> Job.h?rev=241310&r1=241309&r2=241310&view=diff
> ==========================================================================
> ====
> --- cfe/trunk/include/clang/Driver/Job.h (original)
> +++ cfe/trunk/include/clang/Driver/Job.h Thu Jul 2 17:52:08 2015
> @@ -37,37 +37,9 @@ struct CrashReportInfo {
> : Filename(Filename), VFSPath(VFSPath) {}
> };
>
> -class Job {
> -public:
> - enum JobClass {
> - CommandClass,
> - FallbackCommandClass,
> - JobListClass
> - };
> -
> -private:
> - JobClass Kind;
> -
> -protected:
> - Job(JobClass Kind) : Kind(Kind) {}
> -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 CrashInfo - Details for inclusion in a crash report.
> - virtual void Print(llvm::raw_ostream &OS, const char *Terminator, bool
> Quote,
> - CrashReportInfo *CrashInfo = nullptr) const = 0;
> -};
> -
> /// Command - An executable path/name and argument vector to
> /// execute.
> -class Command : public Job {
> +class Command {
> /// Source - The action which caused the creation of this job.
> const Action &Source;
>
> @@ -108,9 +80,10 @@ class Command : public Job {
> public:
> Command(const Action &Source, const Tool &Creator, const char
> *Executable,
> const llvm::opt::ArgStringList &Arguments);
> + virtual ~Command() {}
>
> - void Print(llvm::raw_ostream &OS, const char *Terminator, bool Quote,
> - CrashReportInfo *CrashInfo = nullptr) const override;
> + virtual void Print(llvm::raw_ostream &OS, const char *Terminator, bool
> Quote,
> + CrashReportInfo *CrashInfo = nullptr) const;
>
> virtual int Execute(const StringRef **Redirects, std::string *ErrMsg,
> bool *ExecutionFailed) const;
> @@ -133,11 +106,6 @@ public:
> const char *getExecutable() const { return Executable; }
>
> const llvm::opt::ArgStringList &getArguments() const { return
> Arguments; }
> -
> - static bool classof(const Job *J) {
> - return J->getKind() == CommandClass ||
> - J->getKind() == FallbackCommandClass;
> - }
> };
>
> /// Like Command, but with a fallback which is executed in case
> @@ -154,18 +122,14 @@ public:
> int Execute(const StringRef **Redirects, std::string *ErrMsg,
> bool *ExecutionFailed) const override;
>
> - static bool classof(const Job *J) {
> - return J->getKind() == FallbackCommandClass;
> - }
> -
> private:
> std::unique_ptr<Command> Fallback;
> };
>
> /// JobList - A sequence of jobs to perform.
> -class JobList : public Job {
> +class JobList {
> public:
> - typedef SmallVector<std::unique_ptr<Job>, 4> list_type;
> + typedef SmallVector<std::unique_ptr<Command>, 4> list_type;
> typedef list_type::size_type size_type;
> typedef llvm::pointee_iterator<list_type::iterator> iterator;
> typedef llvm::pointee_iterator<list_type::const_iterator>
> const_iterator;
> @@ -174,14 +138,11 @@ private:
> list_type Jobs;
>
> public:
> - JobList();
> - ~JobList() override {}
> -
> void Print(llvm::raw_ostream &OS, const char *Terminator,
> - bool Quote, CrashReportInfo *CrashInfo = nullptr) const
> override;
> + bool Quote, CrashReportInfo *CrashInfo = nullptr) const;
>
> /// Add a job to the list (taking ownership).
> - void addJob(std::unique_ptr<Job> J) { Jobs.push_back(std::move(J)); }
> + void addJob(std::unique_ptr<Command> J) { Jobs.push_back(std::move(J));
> }
>
> /// Clear the job list.
> void clear();
> @@ -193,10 +154,6 @@ public:
> const_iterator begin() const { return Jobs.begin(); }
> iterator end() { return Jobs.end(); }
> const_iterator end() const { return Jobs.end(); }
> -
> - static bool classof(const Job *J) {
> - return J->getKind() == JobListClass;
> - }
> };
>
> } // end namespace driver
>
> Modified: cfe/trunk/lib/Driver/Compilation.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/
> Compilation.cpp?rev=241310&r1=241309&r2=241310&view=diff
> ==========================================================================
> ====
> --- cfe/trunk/lib/Driver/Compilation.cpp (original)
> +++ cfe/trunk/lib/Driver/Compilation.cpp Thu Jul 2 17:52:08 2015
> @@ -192,18 +192,14 @@ static bool InputsOk(const Command &C,
> return !ActionFailed(&C.getSource(), FailingCommands);
> }
>
> -void Compilation::ExecuteJob(const Job &J,
> - FailingCommandList &FailingCommands) const {
> - if (const Command *C = dyn_cast<Command>(&J)) {
> - if (!InputsOk(*C, FailingCommands))
> - return;
> +void Compilation::ExecuteJobs(const JobList &Jobs,
> + FailingCommandList &FailingCommands) const
> {
> + for (const auto &Job : Jobs) {
> + if (!InputsOk(Job, FailingCommands))
> + continue;
> const Command *FailingCommand = nullptr;
> - if (int Res = ExecuteCommand(*C, FailingCommand))
> + if (int Res = ExecuteCommand(Job, FailingCommand))
> FailingCommands.push_back(std::make_pair(Res, FailingCommand));
> - } else {
> - const JobList *Jobs = cast<JobList>(&J);
> - for (const auto &Job : *Jobs)
> - ExecuteJob(Job, FailingCommands);
> }
> }
>
> Modified: cfe/trunk/lib/Driver/Driver.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?
> rev=241310&r1=241309&r2=241310&view=diff
> ==========================================================================
> ====
> --- cfe/trunk/lib/Driver/Driver.cpp (original)
> +++ cfe/trunk/lib/Driver/Driver.cpp Thu Jul 2 17:52:08 2015
> @@ -500,7 +500,7 @@ void Driver::generateCompilationDiagnost
>
> // Generate preprocessed output.
> SmallVector<std::pair<int, const Command *>, 4> FailingCommands;
> - C.ExecuteJob(C.getJobs(), FailingCommands);
> + C.ExecuteJobs(C.getJobs(), FailingCommands);
>
> // If any of the preprocessing commands failed, clean up and exit.
> if (!FailingCommands.empty()) {
> @@ -560,26 +560,16 @@ void Driver::generateCompilationDiagnost
> << "\n\n********************";
> }
>
> -void Driver::setUpResponseFiles(Compilation &C, Job &J) {
> - if (JobList *Jobs = dyn_cast<JobList>(&J)) {
> - for (auto &Job : *Jobs)
> - setUpResponseFiles(C, Job);
> - return;
> - }
> -
> - Command *CurCommand = dyn_cast<Command>(&J);
> - if (!CurCommand)
> - return;
> -
> +void Driver::setUpResponseFiles(Compilation &C, Command &Cmd) {
> // Since argumentsFitWithinSystemLimits() may underestimate system's
> capacity
> // if the tool does not support response files, there is a chance/ that
> things
> // will just work without a response file, so we silently just skip it.
> - if (CurCommand->getCreator().getResponseFilesSupport() == Tool::RF_None
> ||
> - llvm::sys::argumentsFitWithinSystemLimits(CurCommand->getArguments
> ()))
> + if (Cmd.getCreator().getResponseFilesSupport() == Tool::RF_None ||
> + llvm::sys::argumentsFitWithinSystemLimits(Cmd.getArguments()))
> return;
>
> std::string TmpName = GetTemporaryPath("response", "txt");
> - CurCommand->setResponseFile(
> + Cmd.setResponseFile(
> C.addTempFile(C.getArgs().MakeArgString(TmpName.c_str())));
> }
>
> @@ -597,9 +587,10 @@ int Driver::ExecuteCompilation(
> return 1;
>
> // Set up response file names for each command, if necessary
> - setUpResponseFiles(C, C.getJobs());
> + for (auto &Job : C.getJobs())
> + setUpResponseFiles(C, Job);
>
> - C.ExecuteJob(C.getJobs(), FailingCommands);
> + C.ExecuteJobs(C.getJobs(), FailingCommands);
>
> // Remove temp files.
> C.CleanupFileList(C.getTempFiles());
>
> Modified: cfe/trunk/lib/Driver/Job.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Job.cpp?rev=
> 241310&r1=241309&r2=241310&view=diff
> ==========================================================================
> ====
> --- cfe/trunk/lib/Driver/Job.cpp (original)
> +++ cfe/trunk/lib/Driver/Job.cpp Thu Jul 2 17:52:08 2015
> @@ -25,12 +25,10 @@ using llvm::raw_ostream;
> using llvm::StringRef;
> using llvm::ArrayRef;
>
> -Job::~Job() {}
> -
> Command::Command(const Action &Source, const Tool &Creator,
> const char *Executable, const ArgStringList &Arguments)
> - : Job(CommandClass), Source(Source), Creator(Creator),
> - Executable(Executable), Arguments(Arguments), ResponseFile(nullptr)
> {}
> + : Source(Source), Creator(Creator), Executable(Executable),
> + Arguments(Arguments), ResponseFile(nullptr) {}
>
> static int skipArgs(const char *Flag, bool HaveCrashVFS) {
> // These flags are all of the form -Flag <Arg> and are treated as two
> @@ -293,8 +291,6 @@ int FallbackCommand::Execute(const Strin
> return SecondaryStatus;
> }
>
> -JobList::JobList() : Job(JobListClass) {}
> -
> void JobList::Print(raw_ostream &OS, const char *Terminator, bool Quote,
> CrashReportInfo *CrashInfo) const {
> for (const auto &Job : *this)
>
> Modified: cfe/trunk/lib/Tooling/CompilationDatabase.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/
> CompilationDatabase.cpp?rev=241310&r1=241309&r2=241310&view=diff
> ==========================================================================
> ====
> --- cfe/trunk/lib/Tooling/CompilationDatabase.cpp (original)
> +++ cfe/trunk/lib/Tooling/CompilationDatabase.cpp Thu Jul 2 17:52:08 2015
> @@ -250,14 +250,11 @@ static bool stripPositionalArgs(std::vec
>
> CompileJobAnalyzer CompileAnalyzer;
>
> - for (const auto &Job : Jobs) {
> - if (Job.getKind() == driver::Job::CommandClass) {
> - const driver::Command &Cmd = cast<driver::Command>(Job);
> - // Collect only for Assemble jobs. If we do all jobs we get
> duplicates
> - // since Link jobs point to Assemble jobs as inputs.
> - if (Cmd.getSource().getKind() == driver::Action::AssembleJobClass)
> - CompileAnalyzer.run(&Cmd.getSource());
> - }
> + for (const auto &Cmd : Jobs) {
> + // Collect only for Assemble jobs. If we do all jobs we get
> duplicates
> + // since Link jobs point to Assemble jobs as inputs.
> + if (Cmd.getSource().getKind() == driver::Action::AssembleJobClass)
> + CompileAnalyzer.run(&Cmd.getSource());
> }
>
> if (CompileAnalyzer.Inputs.empty()) {
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list