On Tue, Feb 23, 2016 at 2:44 PM, David Blaikie via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> > > On Tue, Feb 23, 2016 at 11:30 AM, Nico Weber via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: nico >> Date: Tue Feb 23 13:30:43 2016 >> New Revision: 261674 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=261674&view=rev >> Log: >> Rename Action::begin() to Action::input_begin(). >> >> Also introduce inputs() that reutnrs an llvm::iterator_range. >> Iterating over A->inputs() is much less mysterious than >> iterating over *A. No intended behavior change. >> > > Seems like a strange change - is there ambiguity of what an Action is a > collection of? > Action isn't primarily a collection, but an action (a list of inputs, but also a kind, an output type, etc) :-) I found this code pretty confusing, hence I renamed it. (I do have a local change currently that gives it a second iterable thing, but this seemed like a good change independent of my local change.) > (does it expose other collections (perhaps its outputs?) - or only its > inputs? ) > > Lots of things in LLVM/Clang are containers (functions are containers of > basic blocks, basic blocks are containers of instructions, etc) so this > doesn't seem too strange to me... perhaps there's something different about > this situation/API? > > >> >> Modified: >> cfe/trunk/include/clang/Driver/Action.h >> cfe/trunk/lib/Driver/Compilation.cpp >> cfe/trunk/lib/Driver/Driver.cpp >> cfe/trunk/lib/Driver/Tools.cpp >> cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp >> cfe/trunk/lib/Tooling/CompilationDatabase.cpp >> >> Modified: cfe/trunk/include/clang/Driver/Action.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Action.h?rev=261674&r1=261673&r2=261674&view=diff >> >> ============================================================================== >> --- cfe/trunk/include/clang/Driver/Action.h (original) >> +++ cfe/trunk/include/clang/Driver/Action.h Tue Feb 23 13:30:43 2016 >> @@ -41,8 +41,10 @@ namespace driver { >> class Action { >> public: >> typedef ActionList::size_type size_type; >> - typedef ActionList::iterator iterator; >> - typedef ActionList::const_iterator const_iterator; >> + typedef ActionList::iterator input_iterator; >> + typedef ActionList::const_iterator input_const_iterator; >> + typedef llvm::iterator_range<input_iterator> input_range; >> + typedef llvm::iterator_range<input_const_iterator> input_const_range; >> >> enum ActionClass { >> InputClass = 0, >> @@ -98,10 +100,14 @@ public: >> >> size_type size() const { return Inputs.size(); } >> >> - iterator begin() { return Inputs.begin(); } >> - iterator end() { return Inputs.end(); } >> - const_iterator begin() const { return Inputs.begin(); } >> - const_iterator end() const { return Inputs.end(); } >> + input_iterator input_begin() { return Inputs.begin(); } >> + input_iterator input_end() { return Inputs.end(); } >> + input_range inputs() { return input_range(input_begin(), input_end()); >> } >> + input_const_iterator input_begin() const { return Inputs.begin(); } >> + input_const_iterator input_end() const { return Inputs.end(); } >> + input_const_range inputs() const { >> + return input_const_range(input_begin(), input_end()); >> + } >> }; >> >> class InputAction : public Action { >> >> Modified: cfe/trunk/lib/Driver/Compilation.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Compilation.cpp?rev=261674&r1=261673&r2=261674&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Driver/Compilation.cpp (original) >> +++ cfe/trunk/lib/Driver/Compilation.cpp Tue Feb 23 13:30:43 2016 >> @@ -176,8 +176,8 @@ static bool ActionFailed(const Action *A >> if (A == &(CI->second->getSource())) >> return true; >> >> - for (Action::const_iterator AI = A->begin(), AE = A->end(); AI != AE; >> ++AI) >> - if (ActionFailed(*AI, FailingCommands)) >> + for (const Action *AI : A->inputs()) >> + if (ActionFailed(AI, FailingCommands)) >> return true; >> >> return false; >> >> Modified: cfe/trunk/lib/Driver/Driver.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=261674&r1=261673&r2=261674&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Driver/Driver.cpp (original) >> +++ cfe/trunk/lib/Driver/Driver.cpp Tue Feb 23 13:30:43 2016 >> @@ -948,15 +948,15 @@ static unsigned PrintActions1(const Comp >> os << "\"" << IA->getInputArg().getValue() << "\""; >> } else if (BindArchAction *BIA = dyn_cast<BindArchAction>(A)) { >> os << '"' << BIA->getArchName() << '"' << ", {" >> - << PrintActions1(C, *BIA->begin(), Ids) << "}"; >> + << PrintActions1(C, *BIA->input_begin(), Ids) << "}"; >> } else if (CudaDeviceAction *CDA = dyn_cast<CudaDeviceAction>(A)) { >> os << '"' >> << (CDA->getGpuArchName() ? CDA->getGpuArchName() : "(multiple >> archs)") >> - << '"' << ", {" << PrintActions1(C, *CDA->begin(), Ids) << "}"; >> + << '"' << ", {" << PrintActions1(C, *CDA->input_begin(), Ids) << >> "}"; >> } else { >> const ActionList *AL; >> if (CudaHostAction *CHA = dyn_cast<CudaHostAction>(A)) { >> - os << "{" << PrintActions1(C, *CHA->begin(), Ids) << "}" >> + os << "{" << PrintActions1(C, *CHA->input_begin(), Ids) << "}" >> << ", gpu binaries "; >> AL = &CHA->getDeviceActions(); >> } else >> @@ -996,7 +996,7 @@ static bool ContainsCompileOrAssembleAct >> isa<AssembleJobAction>(A)) >> return true; >> >> - for (const Action *Input : *A) >> + for (const Action *Input : A->inputs()) >> if (ContainsCompileOrAssembleAction(Input)) >> return true; >> >> @@ -1747,8 +1747,8 @@ static const Tool *selectToolForJob(Comp >> // Compile job may be wrapped in CudaHostAction, extract it if >> // that's the case and update CollapsedCHA if we combine phases. >> CudaHostAction *CHA = >> dyn_cast<CudaHostAction>(*BackendInputs->begin()); >> - JobAction *CompileJA = >> - cast<CompileJobAction>(CHA ? *CHA->begin() : >> *BackendInputs->begin()); >> + JobAction *CompileJA = cast<CompileJobAction>( >> + CHA ? *CHA->input_begin() : *BackendInputs->begin()); >> assert(CompileJA && "Backend job is not preceeded by compile job."); >> const Tool *Compiler = TC->SelectTool(*CompileJA); >> if (!Compiler) >> @@ -1770,7 +1770,7 @@ static const Tool *selectToolForJob(Comp >> // that's the case and update CollapsedCHA if we combine phases. >> CudaHostAction *CHA = dyn_cast<CudaHostAction>(*Inputs->begin()); >> JobAction *CompileJA = >> - cast<CompileJobAction>(CHA ? *CHA->begin() : *Inputs->begin()); >> + cast<CompileJobAction>(CHA ? *CHA->input_begin() : >> *Inputs->begin()); >> assert(CompileJA && "Backend job is not preceeded by compile job."); >> const Tool *Compiler = TC->SelectTool(*CompileJA); >> if (!Compiler) >> @@ -1841,7 +1841,7 @@ InputInfo Driver::BuildJobsForActionNoCa >> } >> // Override current action with a real host compile action and >> continue >> // processing it. >> - A = *CHA->begin(); >> + A = *CHA->input_begin(); >> } >> >> if (const InputAction *IA = dyn_cast<InputAction>(A)) { >> @@ -1867,7 +1867,7 @@ InputInfo Driver::BuildJobsForActionNoCa >> else >> TC = &C.getDefaultToolChain(); >> >> - return BuildJobsForAction(C, *BAA->begin(), TC, ArchName, AtTopLevel, >> + return BuildJobsForAction(C, *BAA->input_begin(), TC, ArchName, >> AtTopLevel, >> MultipleArchs, LinkingOutput, >> CachedResults); >> } >> >> @@ -1875,11 +1875,11 @@ InputInfo Driver::BuildJobsForActionNoCa >> // Initial processing of CudaDeviceAction carries host params. >> // Call BuildJobsForAction() again, now with correct device >> parameters. >> InputInfo II = BuildJobsForAction( >> - C, *CDA->begin(), C.getCudaDeviceToolChain(), >> CDA->getGpuArchName(), >> - CDA->isAtTopLevel(), /*MultipleArchs*/ true, LinkingOutput, >> - CachedResults); >> - // Currently II's Action is *CDA->begin(). Set it to CDA instead, >> so that >> - // one can retrieve II's GPU arch. >> + C, *CDA->input_begin(), C.getCudaDeviceToolChain(), >> + CDA->getGpuArchName(), CDA->isAtTopLevel(), >> /*MultipleArchs=*/true, >> + LinkingOutput, CachedResults); >> + // Currently II's Action is *CDA->input_begin(). Set it to CDA >> instead, so >> + // that one can retrieve II's GPU arch. >> II.setAction(A); >> return II; >> } >> @@ -2364,7 +2364,8 @@ const ToolChain &Driver::getToolChain(co >> >> bool Driver::ShouldUseClangCompiler(const JobAction &JA) const { >> // Say "no" if there is not exactly one input of a type clang >> understands. >> - if (JA.size() != 1 || >> !types::isAcceptedByClang((*JA.begin())->getType())) >> + if (JA.size() != 1 || >> + !types::isAcceptedByClang((*JA.input_begin())->getType())) >> return false; >> >> // And say "no" if this is not a kind of action clang understands. >> >> Modified: cfe/trunk/lib/Driver/Tools.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=261674&r1=261673&r2=261674&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Driver/Tools.cpp (original) >> +++ cfe/trunk/lib/Driver/Tools.cpp Tue Feb 23 13:30:43 2016 >> @@ -2471,8 +2471,8 @@ static bool ContainsCompileAction(const >> if (isa<CompileJobAction>(A) || isa<BackendJobAction>(A)) >> return true; >> >> - for (const auto &Act : *A) >> - if (ContainsCompileAction(Act)) >> + for (const auto &AI : A->inputs()) >> + if (ContainsCompileAction(AI)) >> return true; >> >> return false; >> >> Modified: cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp?rev=261674&r1=261673&r2=261674&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp (original) >> +++ cfe/trunk/lib/Frontend/CreateInvocationFromCommandLine.cpp Tue Feb 23 >> 13:30:43 2016 >> @@ -70,7 +70,7 @@ clang::createInvocationFromCommandLine(A >> for (auto &A : C->getActions()){ >> // On MacOSX real actions may end up being wrapped in >> BindArchAction >> if (isa<driver::BindArchAction>(A)) >> - A = *A->begin(); >> + A = *A->input_begin(); >> if (isa<driver::CudaDeviceAction>(A)) { >> CudaCompilation = true; >> break; >> >> Modified: cfe/trunk/lib/Tooling/CompilationDatabase.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/CompilationDatabase.cpp?rev=261674&r1=261673&r2=261674&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Tooling/CompilationDatabase.cpp (original) >> +++ cfe/trunk/lib/Tooling/CompilationDatabase.cpp Tue Feb 23 13:30:43 2016 >> @@ -139,9 +139,8 @@ private: >> ; >> } >> >> - for (driver::ActionList::const_iterator I = A->begin(), E = A->end(); >> - I != E; ++I) >> - runImpl(*I, CollectChildren); >> + for (const driver::Action *AI : A->inputs()) >> + runImpl(AI, CollectChildren); >> } >> }; >> >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
