[PATCH] D129565: [NFC][Metadata] Change MDNode::operands()'s return type from op_range to ArrayRef<MDOperand>

[Dawid Jurczak via Phabricator via llvm-commits]

yurai007 added inline comments.


================
Comment at: llvm/include/llvm/IR/Metadata.h:1348
   TempMDTuple cloneImpl() const {
-    return getTemporary(getContext(), SmallVector<Metadata *, 4>(operands()));
   }
----------------
dexonsmith wrote:
> I’m surprised SmallVector doesn’t have a generic constructor from ArrayRef. I wonder if it should?
> 
> (why did the iterator_range work before, but ArrayRef doesn’t?)
That's because SmallVector has appropriate constructor:
  
  template <typename RangeTy>
  explicit SmallVector(const iterator_range<RangeTy> &R);
  
but is lacking one for ArrayRef. However from what I can see by browsing containers definitions in llvm/ADT, convenience constructors taking ArrayRefs are not very popular. There is only TinyPtrVector which has one. But the same can be said for iterator_range - only SmallVector constructor takes it.
Even if constructing SmallVector from ArrayRef's iterators seem to not be extremely popular, still, I can find some places in code base (except ones from this change ofc. ) that would benefit from having convenience constructor - files where ArrayRef mask is used like X86ISelLowering.cpp, HexagonISelDAGToDAGHVX.cpp and DAGCombiner.cpp.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129565/new/

https://reviews.llvm.org/D129565