[PATCH] D49881: docs: Emphasize ArrayRef over SmallVectorImpl

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith created this revision.
dexonsmith added a reviewer: bogner.

The section on SmallVector has a note about preferring SmallVectorImpl for APIs but doesn't mention ArrayRef.  Although ArrayRef is discussed elsewhere, let's re-emphasize here.


https://reviews.llvm.org/D49881

Files:
  llvm/docs/ProgrammersManual.rst


Index: llvm/docs/ProgrammersManual.rst
===================================================================
--- llvm/docs/ProgrammersManual.rst
+++ llvm/docs/ProgrammersManual.rst
@@ -1467,13 +1467,15 @@
 
 .. note::
 
-   Prefer to use ``SmallVectorImpl<T>`` as a parameter type.
+   Prefer to use ``ArrayRef<T>`` or ``SmallVectorImpl<T>`` as a parameter type.
 
-   In APIs that don't care about the "small size" (most?), prefer to 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 ``SmallVectorImpl<T>`` so the
-   conversion is implicit and costs nothing. E.g.
+   It's rarely appropriate to use ``SmallVector<T, N>`` as a parameter type.
+   If an API only reads from the vector, it should use :ref:`ArrayRef
+   <dss_arrayref>`.  Even if an API updates the vector the "small size" is
+   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
+   ``SmallVectorImpl<T>`` so the conversion is implicit and costs nothing. E.g.
 
    .. code-block:: c++
 
@@ -1488,8 +1490,19 @@
         allowsAnySmallSize(Vec); // Works.
       }
 
-   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.
+      allowsAnyContiguousStorage(ArrayRef<Foo> In);
+
+      void someOtherFunc() {
+        Foo Vec[] = { /* ... */ };
+        hardcodedContiguousStorage(Vec); // Error.
+        allowsAnyContiguousStorage(Vec); // Works.
+      }
+
+   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.
 
 .. _dss_vector:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D49881.157592.patch
Type: text/x-patch
Size: 2158 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180726/0b400a7e/attachment.bin>