[PATCH] D45657: [Instruction, BasicBlock] Add is_none_of and skipInsts helper functions.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In https://reviews.llvm.org/D45657#1068834, @vsk wrote:

> I like the direction this is going in :).
>
> I think having an API like 'instructions_no_debug' or similar in BasicBlock would make the interface friendlier. For one, it'd signal to readers that skipping debug intrinsics is an established pattern. Also, I think it would be easier to change if, e.g we wanted update all existing users of the API to skip lifetime intrinsics too.


Thanks for the feedback! I'll add a instructionsNoDebug function. I tried to make skipInsts as flexible as possible, but I am not sure if the flexibility is really needed. Should I remove it for now?



================
Comment at: include/llvm/IR/BasicBlock.h:198
+  iterator_range<
+      filter_iterator<BasicBlock::iterator, bool (*)(const Instruction &)>>
+  skipInsts() {
----------------
dblaikie wrote:
> Indirection through a function pointer may not be optimized away - may be worth baking this into a functor type?
Will do


================
Comment at: unittests/IR/BasicBlockTest.cpp:17
 #include "llvm/IR/NoFolder.h"
+#include "llvm/Support/Debug.h"
 #include "gmock/gmock-matchers.h"
----------------
I'll remove this.


https://reviews.llvm.org/D45657