[api] Improve documentation for API handle types.

include/v8.h

Index: include/v8.h
diff --git a/include/v8.h b/include/v8.h
index 4e445d2144f8fe48ccb8945f1d45501f244372de..47ac72e68bafce669345ca5c9988fbe0c9c0e0c4 100644
--- a/include/v8.h
+++ b/include/v8.h
@@ -159,7 +159,6 @@ class GlobalHandles;
     *(static_cast<T* volatile*>(0)) = static_cast<S*>(0);      \
   }
 
-
 /**
  * An object reference managed by the v8 garbage collector.
  *
@@ -173,10 +172,16 @@ class GlobalHandles;
  * allocated on the heap.
  *
  * There are two types of handles: local and persistent handles.
+ *
  * Local handles are light-weight and transient and typically used in
- * local operations.  They are managed by HandleScopes.  Persistent
- * handles can be used when storing objects across several independent
- * operations and have to be explicitly deallocated when they're no
+ * local operations.  They are managed by HandleScopes, which means that a
+ * HandleScope must exist on the stack when they are created, and that they are

[Franzi, 2017/03/29 08:40:45]

I would change the wording a little bit: 
They are managed by HandleScopes. That means that a
HandleScope must exist on the stack when the local handles are created and that
...

I think no comma between created and and. 

[Anna Henningsen, 2017/03/29 18:02:41]

Done.

+ * only valid inside of the HandleScope active during their creation.
+ * For passing a local handle to an outer HandleScope, a EscapableHandleScope

[Franzi, 2017/03/29 08:40:45]

an EscapableH....

[Anna Henningsen, 2017/03/29 18:02:41]

Done.

+ * and its Escape() method need to be used.

[Franzi, 2017/03/29 08:40:45]

Must instead of need

[Anna Henningsen, 2017/03/29 18:02:41]

Done.

+ *
+ * Persistent handles can be used when storing objects across several
+ * independent operations and have to be explicitly deallocated when they're no
  * longer used.
  *
  * It is safe to extract the object stored in the handle by
@@ -254,6 +259,11 @@ class Local {
     return !operator==(that);
   }
 
+  /**
+   * Casts a handle to a subclass, e.g. Local<Value> to Local<Object>.

[Franzi, 2017/03/29 08:40:45]

We are probably not consistent in this doc, but I'd prefer 
Cast a ... (without the s).

[Anna Henningsen, 2017/03/29 18:02:41]

Done.

+   * This is only valid if the handle actually refers to a value of the

[Franzi, 2017/03/29 08:40:45]

valid? The method throws otherwise, right?

[Anna Henningsen, 2017/03/29 18:02:41]

With V8_ENABLE_CHECKS it crashes, otherwise it fails silently.

+   * target type.
+   */
   template <class S> V8_INLINE static Local<T> Cast(Local<S> that) {
 #ifdef V8_ENABLE_CHECKS
     // If we're going to perform the type check then we have to check
@@ -263,6 +273,11 @@ class Local {
     return Local<T>(T::Cast(*that));
   }
 
+  /**
+   * Calling this is equivalent to Local<S>::Cast().

[Franzi, 2017/03/29 08:40:45]

How about 
Alias for ...?

[Anna Henningsen, 2017/03/29 18:02:41]

It seems a bit odd to me to call a bound method and a static constructor aliases
for each other... if you feel strongly about it, sure.

+   * In particular, this is only valid if the handle actually refers to a value
+   * of the target type.
+   */
   template <class S>
   V8_INLINE Local<S> As() const {
     return Local<S>::Cast(*this);
@@ -339,15 +354,26 @@ class MaybeLocal {
 
   V8_INLINE bool IsEmpty() const { return val_ == nullptr; }
 
+  /**
+   * Converts this MaybeLocal<> to a Local<>. If this MaybeLocal<> is empty,
+   * |false| is returned and |out| is left untouched.
+   */
   template <class S>
   V8_WARN_UNUSED_RESULT V8_INLINE bool ToLocal(Local<S>* out) const {
     out->val_ = IsEmpty() ? nullptr : this->val_;
     return !IsEmpty();
   }
 
-  // Will crash if the MaybeLocal<> is empty.
+  /**
+   * Converts this MaybeLocal<> to a Local<>. If this MaybeLocal<> is empty,
+   * V8 will crash the process.
+   */
   V8_INLINE Local<T> ToLocalChecked();
 
+  /**
+   * Converts this MaybeLocal<> to a Local<>, using a default value if this
+   * MaybeLocal<> is empty.
+   */
   template <class S>
   V8_INLINE Local<S> FromMaybe(Local<S> default_value) const {
     return IsEmpty() ? default_value : Local<S>(val_);
@@ -357,8 +383,10 @@ class MaybeLocal {
   T* val_;
 };
 
-
-// Eternal handles are set-once handles that live for the life of the isolate.
+/**
+ * Eternal handles are set-once handles that live for the lifetime of the
+ * isolate.
+ */
 template <class T> class Eternal {
  public:
   V8_INLINE Eternal() : val_(nullptr) {}
@@ -438,10 +466,10 @@ enum class WeakCallbackType { kParameter, kInternalFields, kFinalizer };
  * An object reference that is independent of any handle scope.  Where
  * a Local handle only lives as long as the HandleScope in which it was
  * allocated, a PersistentBase handle remains valid until it is explicitly
- * disposed.
+ * disposed using Reset().
  *
  * A persistent handle contains a reference to a storage cell within
- * the v8 engine which holds an object value and which is updated by
+ * the V8 engine which holds an object value and which is updated by
  * the garbage collector whenever the object is moved.  A new storage
  * cell can be created using the constructor or PersistentBase::Reset and
  * existing handles can be disposed using PersistentBase::Reset.
@@ -899,6 +927,11 @@ class V8_EXPORT EscapableHandleScope : public HandleScope {
   internal::Object** escape_slot_;
 };
 
+/**
+ * A SealHandleScope acts like a handle scope in which no handle allocations
+ * are allowed, which can be useful for debugging handle leaks.

[Franzi, 2017/03/29 08:40:45]

allowed. It can be useful...

[Anna Henningsen, 2017/03/29 18:02:41]

Done.

+ * Handles can be allocated within inner normal HandleScopes.
+ */
 class V8_EXPORT SealHandleScope {
  public:
   SealHandleScope(Isolate* isolate);
@@ -1363,7 +1396,7 @@ class V8_EXPORT ScriptCompiler {
    * CachedData instance is still valid; the tag has no other meaing.
    *
    * Background: The data carried by CachedData may depend on the exact
-   *   V8 version number or currently compiler flags. This means when
+   *   V8 version number or current compiler flags. This means that when
    *   persisting CachedData, the embedder must take care to not pass in
    *   data from another V8 version, or the same version with different
    *   features enabled.
@@ -1683,21 +1716,21 @@ class V8_EXPORT ValueSerializer {
    public:
     virtual ~Delegate() {}
 
-    /*
+    /**
      * Handles the case where a DataCloneError would be thrown in the structured
      * clone spec. Other V8 embedders may throw some other appropriate exception
      * type.
      */
     virtual void ThrowDataCloneError(Local<String> message) = 0;
 
-    /*
+    /**
      * The embedder overrides this method to write some kind of host object, if
      * possible. If not, a suitable exception should be thrown and
      * Nothing<bool>() returned.
      */
     virtual Maybe<bool> WriteHostObject(Isolate* isolate, Local<Object> object);
 
-    /*
+    /**
      * Called when the ValueSerializer is going to serialize a
      * SharedArrayBuffer object. The embedder must return an ID for the
      * object, using the same ID if this SharedArrayBuffer has already been
@@ -1712,7 +1745,7 @@ class V8_EXPORT ValueSerializer {
 
     virtual Maybe<uint32_t> GetWasmModuleTransferId(
         Isolate* isolate, Local<WasmCompiledModule> module);
-    /*
+    /**
      * Allocates memory for the buffer of at least the size provided. The actual
      * size (which may be greater or equal) is written to |actual_size|. If no
      * buffer has been allocated yet, nullptr will be provided.
@@ -1724,7 +1757,7 @@ class V8_EXPORT ValueSerializer {
     virtual void* ReallocateBufferMemory(void* old_buffer, size_t size,
                                          size_t* actual_size);
 
-    /*
+    /**
      * Frees a buffer allocated with |ReallocateBufferMemory|.
      */
     virtual void FreeBufferMemory(void* buffer);
@@ -1734,24 +1767,24 @@ class V8_EXPORT ValueSerializer {
   ValueSerializer(Isolate* isolate, Delegate* delegate);
   ~ValueSerializer();
 
-  /*
+  /**
    * Writes out a header, which includes the format version.
    */
   void WriteHeader();
 
-  /*
+  /**
    * Serializes a JavaScript value into the buffer.
    */
   V8_WARN_UNUSED_RESULT Maybe<bool> WriteValue(Local<Context> context,
                                                Local<Value> value);
 
-  /*
+  /**
    * Returns the stored data. This serializer should not be used once the buffer
    * is released. The contents are undefined if a previous write has failed.
    */
   V8_DEPRECATE_SOON("Use Release()", std::vector<uint8_t> ReleaseBuffer());
 
-  /*
+  /**
    * Returns the stored data (allocated using the delegate's
    * AllocateBufferMemory) and its size. This serializer should not be used once
    * the buffer is released. The contents are undefined if a previous write has
@@ -1759,7 +1792,7 @@ class V8_EXPORT ValueSerializer {
    */
   V8_WARN_UNUSED_RESULT std::pair<uint8_t*, size_t> Release();
 
-  /*
+  /**
    * Marks an ArrayBuffer as havings its contents transferred out of band.
    * Pass the corresponding ArrayBuffer in the deserializing context to
    * ValueDeserializer::TransferArrayBuffer.
@@ -1767,7 +1800,7 @@ class V8_EXPORT ValueSerializer {
   void TransferArrayBuffer(uint32_t transfer_id,
                            Local<ArrayBuffer> array_buffer);
 
-  /*
+  /**
    * Similar to TransferArrayBuffer, but for SharedArrayBuffer.
    */
   V8_DEPRECATE_SOON("Use Delegate::GetSharedArrayBufferId",
@@ -1775,7 +1808,7 @@ class V8_EXPORT ValueSerializer {
                         uint32_t transfer_id,
                         Local<SharedArrayBuffer> shared_array_buffer));
 
-  /*
+  /**
    * Indicate whether to treat ArrayBufferView objects as host objects,
    * i.e. pass them to Delegate::WriteHostObject. This should not be
    * called when no Delegate was passed.
@@ -1784,7 +1817,7 @@ class V8_EXPORT ValueSerializer {
    */
   void SetTreatArrayBufferViewsAsHostObjects(bool mode);
 
-  /*
+  /**
    * Write raw data in various common formats to the buffer.
    * Note that integer types are written in base-128 varint format, not with a
    * binary copy. For use during an override of Delegate::WriteHostObject.
@@ -1816,14 +1849,14 @@ class V8_EXPORT ValueDeserializer {
    public:
     virtual ~Delegate() {}
 
-    /*
+    /**
      * The embedder overrides this method to read some kind of host object, if
      * possible. If not, a suitable exception should be thrown and
      * MaybeLocal<Object>() returned.
      */
     virtual MaybeLocal<Object> ReadHostObject(Isolate* isolate);
 
-    /*
+    /**
      * Get a WasmCompiledModule given a transfer_id previously provided
      * by ValueSerializer::GetWasmModuleTransferId
      */
@@ -1836,25 +1869,25 @@ class V8_EXPORT ValueDeserializer {
                     Delegate* delegate);
   ~ValueDeserializer();
 
-  /*
+  /**
    * Reads and validates a header (including the format version).
    * May, for example, reject an invalid or unsupported wire format.
    */
   V8_WARN_UNUSED_RESULT Maybe<bool> ReadHeader(Local<Context> context);
 
-  /*
+  /**
    * Deserializes a JavaScript value from the buffer.
    */
   V8_WARN_UNUSED_RESULT MaybeLocal<Value> ReadValue(Local<Context> context);
 
-  /*
+  /**
    * Accepts the array buffer corresponding to the one passed previously to
    * ValueSerializer::TransferArrayBuffer.
    */
   void TransferArrayBuffer(uint32_t transfer_id,
                            Local<ArrayBuffer> array_buffer);
 
-  /*
+  /**
    * Similar to TransferArrayBuffer, but for SharedArrayBuffer.
    * The id is not necessarily in the same namespace as unshared ArrayBuffer
    * objects.
@@ -1862,7 +1895,7 @@ class V8_EXPORT ValueDeserializer {
   void TransferSharedArrayBuffer(uint32_t id,
                                  Local<SharedArrayBuffer> shared_array_buffer);
 
-  /*
+  /**
    * Must be called before ReadHeader to enable support for reading the legacy
    * wire format (i.e., which predates this being shipped).
    *
@@ -1871,19 +1904,19 @@ class V8_EXPORT ValueDeserializer {
    */
   void SetSupportsLegacyWireFormat(bool supports_legacy_wire_format);
 
-  /*
+  /**
    * Expect inline wasm in the data stream (rather than in-memory transfer)
    */
   void SetExpectInlineWasm(bool allow_inline_wasm);
 
-  /*
+  /**
    * Reads the underlying wire format version. Likely mostly to be useful to
    * legacy code reading old wire format versions. Must be called after
    * ReadHeader.
    */
   uint32_t GetWireFormatVersion() const;
 
-  /*
+  /**
    * Reads raw data in various common formats to the buffer.
    * Note that integer types are read in base-128 varint format, not with a
    * binary copy. For use during an override of Delegate::ReadHostObject.
@@ -2308,7 +2341,15 @@ class V8_EXPORT Name : public Primitive {
   static void CheckCast(Value* obj);
 };
 
-
+/**
+ * A flag describing different modes of string creation.
+ * The |kInternalized| flag acts as a hint that the string should be created

[Franzi, 2017/03/29 08:40:45]

There's Doxygen syntax for enums. Can we use that?

[Anna Henningsen, 2017/03/29 18:02:41]

Done, assuming you meant adding doc comments to the enum values themselves?

+ * in the old generation heap space and be deduplicated if an identical string
+ * already exists.
+ *
+ * Aside from performance implications there are no differences between the two
+ * creation modes.
+ */
 enum class NewStringType { kNormal, kInternalized };
 
 
@@ -2325,7 +2366,7 @@ class V8_EXPORT String : public Name {
     ONE_BYTE_ENCODING = 0x8
   };
   /**
-   * Returns the number of characters in this string.
+   * Returns the number of characters (UTF-16 code units) in this string.
    */
   int Length() const;
 
@@ -2336,14 +2377,16 @@ class V8_EXPORT String : public Name {
   int Utf8Length() const;
 
   /**
-   * Returns whether this string is known to contain only one byte data.
+   * Returns whether this string is known to contain only one byte data,
+   * i.e. ISO-8859-1 code points.
    * Does not read the string.
    * False negatives are possible.
    */
   bool IsOneByte() const;
 
   /**
-   * Returns whether this string contain only one byte data.
+   * Returns whether this string contain only one byte data,
+   * i.e. ISO-8859-1 code points.
    * Will read the entire string in some cases.
    */
   bool ContainsOnlyOneByte() const;
@@ -2650,7 +2693,7 @@ class V8_EXPORT String : public Name {
   };
 
   /**
-   * Converts an object to a two-byte string.
+   * Converts an object to a two-byte (UTF-16-encoded) string.
    * If conversion to a string fails (eg. due to an exception in the toString()
    * method of the object) then the length() method returns 0 and the * operator
    * returns NULL.
@@ -2685,22 +2728,30 @@ class V8_EXPORT String : public Name {
  */
 class V8_EXPORT Symbol : public Name {
  public:
-  // Returns the print name string of the symbol, or undefined if none.
+  /**
+   * Returns the print name string of the symbol, or undefined if none.
+   */
   Local<Value> Name() const;
 
-  // Create a symbol. If name is not empty, it will be used as the description.
+  /**
+   * Create a symbol. If name is not empty, it will be used as the description.
+   */
   static Local<Symbol> New(Isolate* isolate,
                            Local<String> name = Local<String>());
 
-  // Access global symbol registry.
-  // Note that symbols created this way are never collected, so
-  // they should only be used for statically fixed properties.
-  // Also, there is only one global name space for the names used as keys.
-  // To minimize the potential for clashes, use qualified names as keys.
+  /**
+   * Access global symbol registry.
+   * Note that symbols created this way are never collected, so
+   * they should only be used for statically fixed properties.
+   * Also, there is only one global name space for the names used as keys.
+   * To minimize the potential for clashes, use qualified names as keys.
+   */
   static Local<Symbol> For(Isolate *isolate, Local<String> name);
 
-  // Retrieve a global symbol. Similar to |For|, but using a separate
-  // registry that is not accessible by (and cannot clash with) JavaScript code.
+  /**
+   * Retrieve a global symbol. Similar to |For|, but using a separate
+   * registry that is not accessible by (and cannot clash with) JavaScript code.
+   */
   static Local<Symbol> ForApi(Isolate *isolate, Local<String> name);
 
   // Well-known symbols
@@ -2725,20 +2776,26 @@ class V8_EXPORT Symbol : public Name {
  */
 class V8_EXPORT Private : public Data {
  public:
-  // Returns the print name string of the private symbol, or undefined if none.
+  /**
+   * Returns the print name string of the private symbol, or undefined if none.
+   */
   Local<Value> Name() const;
 
-  // Create a private symbol. If name is not empty, it will be the description.
+  /**
+   * Create a private symbol. If name is not empty, it will be the description.
+   */
   static Local<Private> New(Isolate* isolate,
                             Local<String> name = Local<String>());
 
-  // Retrieve a global private symbol. If a symbol with this name has not
-  // been retrieved in the same isolate before, it is created.
-  // Note that private symbols created this way are never collected, so
-  // they should only be used for statically fixed properties.
-  // Also, there is only one global name space for the names used as keys.
-  // To minimize the potential for clashes, use qualified names as keys,
-  // e.g., "Class#property".
+  /**
+   * Retrieve a global private symbol. If a symbol with this name has not
+   * been retrieved in the same isolate before, it is created.
+   * Note that private symbols created this way are never collected, so
+   * they should only be used for statically fixed properties.
+   * Also, there is only one global name space for the names used as keys.
+   * To minimize the potential for clashes, use qualified names as keys,
+   * e.g., "Class#property".
+   */
   static Local<Private> ForApi(Isolate* isolate, Local<String> name);
 
  private:
@@ -4063,7 +4120,8 @@ class V8_EXPORT ArrayBuffer : public Object {
     /**
      * malloc/free based convenience allocator.
      *
-     * Caller takes ownership.
+     * Caller takes ownership, i.e. the returned object needs to be freed using
+     * |delete allocator| once it is no longer in use.
      */
     static Allocator* NewDefaultAllocator();
   };
@@ -4107,8 +4165,11 @@ class V8_EXPORT ArrayBuffer : public Object {
   /**
    * Create a new ArrayBuffer over an existing memory block.
    * The created array buffer is by default immediately in externalized state.
-   * The memory block will not be reclaimed when a created ArrayBuffer
-   * is garbage-collected.
+   * In externalized state, the memory block will not be reclaimed when a
+   * created ArrayBuffer is garbage-collected.
+   * In internalized state, the memory block will be released using
+   * |Allocator::Free| once all ArrayBuffers referencing it are collected by
+   * the garbage collector.
    */
   static Local<ArrayBuffer> New(
       Isolate* isolate, void* data, size_t byte_length,
@@ -6057,6 +6118,12 @@ class V8_EXPORT HeapStatistics {
   size_t heap_size_limit() { return heap_size_limit_; }
   size_t malloced_memory() { return malloced_memory_; }
   size_t peak_malloced_memory() { return peak_malloced_memory_; }
+
+  /**
+   * Returns a 0/1 boolean, which signifies whether the |--zap_code_space|
+   * option is enabled or not, which makes V8 overwrite heap garbage with a bit
+   * pattern.
+   */
   size_t does_zap_garbage() { return does_zap_garbage_; }
 
  private:
@@ -7847,20 +7914,33 @@ class Maybe {
   V8_INLINE bool IsNothing() const { return !has_value_; }
   V8_INLINE bool IsJust() const { return has_value_; }
 
-  // Will crash if the Maybe<> is nothing.
+  /**
+   * An alias for |FromJust|. Will crash if the Maybe<> is nothing.
+   */
   V8_INLINE T ToChecked() const { return FromJust(); }
 
+  /**
+   * Converts this Maybe<> to a value of type T. If this Maybe<> is
+   * nothing (empty), |false| is returned and |out| is left untouched.
+   */
   V8_WARN_UNUSED_RESULT V8_INLINE bool To(T* out) const {
     if (V8_LIKELY(IsJust())) *out = value_;
     return IsJust();
   }
 
-  // Will crash if the Maybe<> is nothing.
+  /**
+   * Converts this Maybe<> to a value of type T. If this Maybe<> is
+   * nothing (empty), V8 will crash the process.
+   */
   V8_INLINE T FromJust() const {
     if (V8_UNLIKELY(!IsJust())) V8::FromJustIsNothing();
     return value_;
   }
 
+  /**
+   * Converts this Maybe<> to a value of type T, using a default value if this
+   * Maybe<> is nothing (empty).
+   */
   V8_INLINE T FromMaybe(const T& default_value) const {
     return has_value_ ? value_ : default_value;
   }