r261674 - Rename Action::begin() to Action::input_begin().
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 23 12:55:42 PST 2016
On Tue, Feb 23, 2016 at 12:46 PM, Nico Weber <thakis at chromium.org> wrote:
> On Tue, Feb 23, 2016 at 2:44 PM, David Blaikie via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>>
>>
>> On Tue, Feb 23, 2016 at 11:30 AM, Nico Weber via cfe-commits <
>> cfe-commits at 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) :-)
>
The analogy to llvm::Function, llvm::BasicBlock, etc, still seem somewhat
apt (a Function isn't, in some sense, primarily a collection of basic
blocks - it's a global value with a name and parameters, 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.)
>
Fair enough - just figured I'd point out there's a fair bit of precedent
for thing-is-a-collection across LLVM, in case that provides a diferent
perspective.
- Dave
>
>
>> (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 at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160223/aa3dfed7/attachment-0001.html>
More information about the cfe-commits
mailing list