even better docs/comments

src/lib.rs

@@ -43,10 +43,13 @@ use impl_details::*;
 /// transferred across the FFI boundary (**IF YOU ARE CAREFUL, SEE BELOW!!**).
 ///
 /// While this feature is handy, it is also inherently dangerous to use because Rust and C++ do not
-/// know about eachother. Please be aware of the risks!
+/// know about eachother. Specifically, this can be an issue with non-POD types (types which
+/// have destructors, move constructors, or are `!Copy`).
 ///
-/// The biggest thing to keep in mind is that **FFI functions cannot pass ThinVec/nsTArray by-value 
-/// because of constructors**. That is, these are busted APIs:
+/// ## Do Not Pass By Value
+///
+/// The biggest thing to keep in mind is that **FFI functions cannot pass ThinVec/nsTArray 
+/// by-value**. That is, these are busted APIs:
 ///
 /// ```rust,ignore
 /// // BAD WRONG
@@ -105,26 +108,49 @@ use impl_details::*;
 /// > generate FFI glue so we can pass things "by value" and have it generate by-reference code
 /// > behind our back (like the cxx crate does). This would muddy up debugging/searchfox though.
 ///
-/// ThinVec also has *some* support for handling nsAutoArray, but this isn't well tested. You can
-/// definitely read and pass around auto arrays. More testing should be done on the mutating case,
-/// though. Also Rust isn't aware that the buffer has a limited lifetime, so standard auto array
-/// safety caveats apply about returning/storing them.
+/// ## Types Should Be Trivially Relocatable
+///
+/// Types in Rust are always trivially relocatable (unless suitably borrowed/[pinned][]/hidden).
+/// This means all Rust types are legal to relocate with a bitwise copy, you cannot provide
+/// copy or move constructors to execute when this happens, and the old location won't have its
+/// destructor run. This will cause problems for types which have a significant location
+/// (types that intrusively point into themselves or have their location registered with a service).
+///
+/// While relocations are generally predictable if you're very careful, **you should avoid using
+/// types with significant locations with Rust FFI**.
+///
+/// Specifically, ThinVec will trivially relocate its contents whenever it needs to reallocate its
+/// buffer to change its capacity. This is the default reallocation strategy for nsTArray, and is
+/// suitable for the vast majority of types. Just be aware of this limitation!
+///
+/// ## nsAutoArray Is Dangerous
+///
+/// ThinVec has *some* support for handling nsAutoArray, but this isn't well tested.s
+///
+/// Regardless of how much support we provide, Rust won't be aware of the buffer's limited lifetime,
+/// so standard auto array safety caveats apply about returning/storing them! ThinVec won't ever
+/// produce an auto array on its own, so this is only an issue for transferring an nsTArray into
+/// Rust.
+///
+/// ## Other Issues
 ///
 /// Standard FFI caveats also apply: 
 ///
 ///  * Rust is more strict about POD types being initialized (use MaybeUninit if you must)
 ///  * `ThinVec<T>` has no idea if the C++ version of `T` has move/copy/assign/delete overloads
-///  * `nsTArray<T>` has no idea if the Rust version of `T` has a Drop impl
+///  * `nsTArray<T>` has no idea if the Rust version of `T` has a Drop/Clone impl
 ///  * C++ can do all sorts of unsound things that Rust can't catch
 ///  * C++ and Rust don't agree on how zero-sized/empty types should be handled
 ///
-/// NOTE: the gecko-ffi feature will not work if you aren't linking with code that has nsTArray
+/// The gecko-ffi feature will not work if you aren't linking with code that has nsTArray
 /// defined. Specifically, we must share the symbol for nsTArray's empty singleton. You will get
 /// linking errors if that isn't defined.
 ///
-/// The gecko-ffi feature also limits ThinVec to the legacy behaviours of nsTArray. Most notably,
-/// nsTArray has a maximum capacity of i32::MAX (about 2.1 billion items). Probably not an issue.
+/// The gecko-ffi feature also limits ThinVec to the legacy behaviors of nsTArray. Most notably,
+/// nsTArray has a maximum capacity of i32::MAX (~2.1 billion items). Probably not an issue.
 /// Probably.
+///
+/// [pinned]: https://doc.rust-lang.org/std/pin/index.html
 
 // modules: a simple way to cfg a whole bunch of impl details at once
 
@@ -862,13 +888,13 @@ impl<T> ThinVec<T> {
             // a gecko auto array, and we have items in a stack buffer. We shouldn't
             // free it, but we should memcopy the contents out of it and mark it as empty.
             //
-            // T is assumed to have a trivial move constructor, as this is required
+            // T is assumed to be trivially relocatable, as this is ~required
             // for Rust compatibility anyway. Furthermore, we assume C++ won't try
             // to unconditionally destroy the contents of the stack allocated buffer
             // (i.e. it's obfuscated behind a union).
             //
-            // In effect, we are reimplementing the auto array move constructor by leaving
-            // behind a valid empty instance.
+            // In effect, we are partially reimplementing the auto array move constructor
+            // by leaving behind a valid empty instance.
             let len = self.len();
             if cfg!(feature = "gecko-ffi") && len > 0 {
                 new_header.as_mut().data::<T>().copy_from_nonoverlapping(self.data_raw(), len);