[PATCH] D14363: [FunctionAttrs] Remove a loop, NFC refactor

[Chandler Carruth via llvm-commits]

chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.

Generally much cleaner, minor comments below, submit whenever.


================
Comment at: lib/Transforms/IPO/FunctionAttrs.cpp:324-325
@@ -323,15 +323,4 @@
 
-    bool Found = false;
-    Function::arg_iterator AI = F->arg_begin(), AE = F->arg_end();
-    for (CallSite::arg_iterator PI = CS.arg_begin(), PE = CS.arg_end();
-         PI != PE; ++PI, ++AI) {
-      if (AI == AE) {
-        assert(F->isVarArg() && "More params than args in non-varargs call");
-        Captured = true;
-        return true;
-      }
-      if (PI == U) {
-        Uses.push_back(&*AI);
-        Found = true;
-        break;
-      }
+    unsigned UseIndex =
+      std::distance(const_cast<const Use *>(CS->op_begin()), U);
+
----------------
Why op_begin here rather than arg_begin?

Also, its a bit odd that the use index here is the correct increment amount for the function argument iterator -- where is the function operand to the callsite?

I'd at least leave some comments to remind the reader of the surprising indexing contract of argument operands vs. function operands and the function argument list.

================
Comment at: lib/Transforms/IPO/FunctionAttrs.cpp:334-336
@@ -341,1 +333,5 @@
+
+    auto FormalArgIt = F->arg_begin();
+    std::advance(FormalArgIt, UseIndex);
+    Uses.push_back(FormalArgIt);
     return false;
----------------
This can be:

  Uses.push_back(std::next(F->arg_begin(), UseIndex));


http://reviews.llvm.org/D14363