[PATCH] D49881: docs: Emphasize ArrayRef over SmallVectorImpl

[Dean Michael Berris via Phabricator via llvm-commits]

dberris added a comment.

Drive-by-review here.



================
Comment at: llvm/docs/ProgrammersManual.rst:1476
+   unlikely to be relevant; such an API should use the ``SmallVectorImpl<T>``
+   class, which is basically just the "vector header" (and methods) without the
+   elements allocated after it. Note that ``SmallVector<T, N>`` inherits from
----------------
s/basically just//


================
Comment at: llvm/docs/ProgrammersManual.rst:1493-1494
 
-   Even though it has "``Impl``" in the name, this is so widely used that
-   it really isn't "private to the implementation" anymore. A name like
+      // BAD: Clients cannot pass e.g. raw arrays.
+      hardcodedContiguousStorage(const SmallVectorImpl<Foo> &In);
+      // GOOD: Clients can pass any contiguous storage of Foo.
----------------
I suggest "discouraged" instead of "bad".


================
Comment at: llvm/docs/ProgrammersManual.rst:1504-1505
+
+   Even though it has "``Impl``" in the name, SmallVectorImpl is so widely used
+   that it really isn't "private to the implementation" anymore. A name like
    ``SmallVectorHeader`` would be more appropriate.
----------------
s/is so widely/is widely/
s/really isn't/is no longer/
s/"private to the implementation" anymore/an implementation detail/


https://reviews.llvm.org/D49881