This is an archive of the discontinued LLVM Phabricator instance.

[llvm] remove Sequence::asSmallVector()
ClosedPublic

Authored by gchatelet on Jun 10 2021, 5:35 AM.

Details

Summary

There's no need for toSmallVector() as SmallVector.h already provides a to_vector free function that takes a range.

Diff Detail

Event Timeline

gchatelet created this revision.Jun 10 2021, 5:35 AM
gchatelet requested review of this revision.Jun 10 2021, 5:35 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
Quuxplusone accepted this revision.Jun 10 2021, 8:27 AM
Quuxplusone added inline comments.
mlir/lib/Conversion/PDLToPDLInterp/PDLToPDLInterp.cpp
385–387

The <16> here confuses me. I think to_vector should just figure out the appropriate small-buffer capacity, the same way SmallVector itself does. I think the way to do that would be to add this diff in SmallVector.h:

--- a/llvm/include/llvm/ADT/SmallVector.h
+++ b/llvm/include/llvm/ADT/SmallVector.h
@@ -1249,6 +1249,13 @@ to_vector(R &&Range) {
   return {std::begin(Range), std::end(Range)};
 }
 
+template <typename R>
+SmallVector<typename std::remove_const<typename std::remove_reference<
+                decltype(*std::begin(std::declval<R &>()))>::type>::type>
+to_vector(R &&Range) {
+  return {std::begin(Range), std::end(Range)};
+}
+
 } // end namespace llvm

It would be reasonable, but not strictly necessary, to then audit the codebase for places where llvm::to_vector<N>(r) is called with a template argument, and update them to call just llvm::to_vector(r). If you were to do that, presumably it'd be in a separate (and unfortunately massive) PR.

This revision is now accepted and ready to land.Jun 10 2021, 8:27 AM
This revision was automatically updated to reflect the committed changes.