[PATCH] D27782: [codegen] Add generic functions to skip debug values.

Matthias Braun via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 14 16:33:34 PST 2016


MatzeB added a comment.

Thanks for doing the refactoring. I have some comments on the API:



================
Comment at: include/llvm/CodeGen/MachineBasicBlock.h:813
+template<typename IterT>
+inline bool skipDebugInstructionsForward(IterT &It, const IterT &End) {
+  while (It != End && It->isDebugValue())
----------------
- Iterators are usually small enough to be better passed by value (so `IterT End` instead of `const IterT &End`).
- APIs modifying their "parameters" through references are often unintuitive in C++. How about changing this to: `IterT skipDebugInstructionsForward(IterT Begin, IterT End);` that returns the new iterator position after skipping?
- Given the iterator type is a template that I guess is only intended to be used with MachineBasicBlock::{const_}?{instr_}?iterator maybe listing those 4 possibiltites would help understanding the function.


https://reviews.llvm.org/D27782





More information about the llvm-commits mailing list