[PATCH] D49881: docs: Emphasize ArrayRef over SmallVectorImpl
Duncan P. N. Exon Smith via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 26 15:29:06 PDT 2018
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>
More information about the llvm-commits
mailing list