[PATCH] D130268: [WIP] Add SmallVector constructor to allow creation of SmallVector<T> from ArrayRef of items convertible to type T

[Sergei Barannikov via Phabricator via llvm-commits]

barannikov88 added inline comments.


================
Comment at: clang/lib/CodeGen/CGExprConstant.cpp:504
   llvm::StructType *STy = llvm::ConstantStruct::getTypeForElements(
-      CGM.getLLVMContext(), Packed ? PackedElems : UnpackedElems, Packed);
+      CGM.getLLVMContext(), Packed ? PackedElems : to_vector(UnpackedElems),
+      Packed);
----------------
yurai007 wrote:
> nikic wrote:
> > yurai007 wrote:
> > > That's because of "error: conditional expression is ambiguous; 'llvm::SmallVector<llvm::Constant *, 32>' can be converted to 'ArrayRef<llvm::Constant *>' and vice versa". Need to check if there is easier workaround.
> > Would making the ctor explicit help?
> Nope :( Making constructor explicit disables implicit conversion so we cannot do things like: SmallVector<int, 16> NewMask = Mask; anymore.
And leaving it implicit hides the fact of possible memory allocation, which is not cheap. I think absense of such constructor is by-design.

Making it explicit is making it redundant, because there is already a constructor which accepts begin / end, and one that accepts range (note that it is explicit!). It won't save on typing, either.

I'm not in favor of this patch, but my word does not count much, so I won't block it. I'd suggest you, however, to request review of core maintainers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130268