[PATCH] D90884: [SmallVector] Add `SVec<T>` / `Vec<T>` convenience wrappers.

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: llvm/docs/ProgrammersManual.rst:1517
+* ``Vec<T>`` should be used for vectors on the heap. This includes inside other
+  data structures, such as ``DenseMap<Value *, Vec<Value *>>``.
+
----------------
I don't necessarily subscribe to the stack/heap difference.

If the SVec/Vec is part of a heap allocation, they retain their properties: `Vect` would always an **extra** heap allocation for the data while SVec keeps inlined storage in the original heap allocation (with all the benefits of SmallVector).

I see it similarly to trailing allocation in heap allocated object somehow.
In general, heap or stack, SmallVector and Vec behaves differently in "move" vs "copy".


================
Comment at: llvm/docs/ProgrammersManual.rst:1521
+:ref:`SmallVector <dss_smallvector>` section, this has advantages over
+``std::vector<T>``, even with 0 inline elements.
+
----------------
Should we say that we always recommend `Vec<T>` instead of `std::vector`? Are there use-cases (other than invoking external API expressed with std::vector) to use it?

(actually line 1617 may make this redundant)


================
Comment at: llvm/docs/ProgrammersManual.rst:1525
+inline elements (which can be 0 if ``sizeof(T)`` is very big!). Historically,
+``SmallVector<T, N>`` has suffered from various misuses due to the ``N``
+parameter being chosen arbitrarily or cargo-culted, and this heuristic
----------------
"reference needed" ;)

(or reformulate)


================
Comment at: llvm/docs/ProgrammersManual.rst:1526
+``SmallVector<T, N>`` has suffered from various misuses due to the ``N``
+parameter being chosen arbitrarily or cargo-culted, and this heuristic
+avoids them. If measurements show a need for more or fewer inline elements than
----------------
I'd remove this.


================
Comment at: llvm/docs/ProgrammersManual.rst:1617
+``std::vector<T>`` is well loved and respected.  However, ``Vec<T>``
 is often a better option due to the advantages listed above.  std::vector is
 still useful when you need to store more than ``UINT32_MAX`` elements or when
----------------
`s/often/in general/` ?


================
Comment at: llvm/docs/ProgrammersManual.rst:1618
 is often a better option due to the advantages listed above.  std::vector is
 still useful when you need to store more than ``UINT32_MAX`` elements or when
 interfacing with code that expects vectors :).
----------------
Is this comment still relevant? The header for SmallVector is adjusting its max size right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90884