[PATCH] D145390: [ADT] Introduce `map_to_vector` helper

[Laszlo Kindrat via Phabricator via llvm-commits]

laszlokindrat added inline comments.


================
Comment at: llvm/include/llvm/ADT/SmallVectorExtras.h:23-24
+/// Map a range to a SmallVector with element types deduced from the mapping.
+template <class ContainerTy, class FuncTy>
+auto map_to_vector(ContainerTy &&C, FuncTy &&F) {
+  return to_vector(
----------------
kuhar wrote:
> laszlokindrat wrote:
> > kuhar wrote:
> > > I'd expect this to optionally accept small size, just like `to_vector<N>(...)` does. Is there a specific reason why only the default size is allowed?
> > > 
> > > Also, do you plan on adding `map_to_vector_of`? IIRC `to_vector_of` was required for some common mlir functions like `DenseElementAttribute::getValues<T>()`
> > > I'd expect this to optionally accept small size, just like `to_vector<N>(...)` does. Is there a specific reason why only the default size is allowed?
> > 
> > No reason other than I prefer to add complexity incrementally. But I agree that this would be nice, so I will add it in a subsequent patch.
> > 
> > > Also, do you plan on adding `map_to_vector_of`?
> > 
> > I can't really find any use cases for this in the llvm monorepo. Do you have a downstream project that could benefit from it? I usually like to add this kind of sugar if/when there are lots of places that can benefit from it.
> > I can't really find any use cases for this in the llvm monorepo. Do you have a downstream project that could benefit from it? I usually like to add this kind of sugar if/when there are lots of places that can benefit from it.
> 
> I think I wanted to use `to_vector(map(...))` here https://github.com/llvm/llvm-project/blob/890aa28f1c9a502ad012d83ad4d6ad60f75efccb/mlir/lib/Dialect/Vector/IR/VectorOps.cpp#L2671C10-L2687 and https://github.com/llvm/llvm-project/blob/890aa28f1c9a502ad012d83ad4d6ad60f75efccb/mlir/lib/Dialect/Arith/Transforms/IntNarrowing.cpp#L201-L204 (in `reduce` or `accumulate`)  but couldn't at the time. I think this can be a bit of a chicken and egg problem -- you may not be able to find many places that would immediately benefit form it because folks have already worked around it.
> 
See https://reviews.llvm.org/D150601 for a patch that adds the ability to specify the size of the resulting vector.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145390