r261674 - Rename Action::begin() to Action::input_begin().

Nico Weber via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 23 12:46:02 PST 2016


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) :-) 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 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/c2d7921c/attachment-0001.html>


More information about the cfe-commits mailing list