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