nikic 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); ---------------- barannikov88 wrote: > 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. > Nope :( Making constructor explicit disables implicit conversion so we cannot > do things like: SmallVector<int, 16> NewMask = Mask; anymore. I think that's fine. Similar to the existing iterator_range constructor, we would require `SmallVector<int, 16> NewMask(Mask)`, which seems like the idiomatic way to write it anyway? > 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. It is not redundant. It ensures that iterator_range and ArrayRef can be freely substituted. Switching iterator_range to ArrayRef currently requires going through a lot of SmallVector constructors and replacing them with less readable code. The alternative to this change is D129988, which looks like a significant regression in code quality. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130268/new/ https://reviews.llvm.org/D130268 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
