Bug 1257395: Update comments for GCHashTable and GCPolicy. DONTBUILD r=terrence

--HG--
extra : rebase_source : 12d7d4f2b7a1178498e1499c1f68b0033449f887

js/public/GCHashTable.h

@@ -25,25 +25,27 @@ struct DefaultMapSweepPolicy {
 // A GCHashMap is a GC-aware HashMap, meaning that it has additional trace and
 // sweep methods that know how to visit all keys and values in the table.
 // HashMaps that contain GC pointers will generally want to use this GCHashMap
-// specialization in lieu of HashMap, either because those pointers must be
-// traced to be kept alive -- in which case, KeyPolicy and/or ValuePolicy
-// should do the appropriate tracing -- or because those pointers are weak and
-// must be swept during a GC -- in which case needsSweep should be set
-// appropriately.
+// specialization instead of HashMap, because this conveniently supports tracing
+// keys and values, and cleaning up weak entries.
 //
-// Most types of GC pointers as keys and values can be traced with no extra
-// infrastructure. For structs, the GCPolicy<T> will call a trace() method on
-// the struct. For other structs and non-gc-pointer members, ensure that there
-// is a specialization of GCPolicy<T> with an appropriate trace() static method
-// available to handle the custom type. Generic helpers can be found in
-// js/public/TracingAPI.h.
+// GCHashMap::trace applies GCPolicy<T>::trace to each entry's key and value.
+// Most types of GC pointers already have appropriate specializations of
+// GCPolicy, so they should just work as keys and values. Any struct type with a
+// default constructor and trace and sweep functions should work as well. If you
+// need to define your own GCPolicy specialization, generic helpers can be found
+// in js/public/TracingAPI.h.
 //
-// Note that this HashMap only knows *how* to trace and sweep (and the tracing
-// can handle keys that move), but it does not itself cause tracing or sweeping
-// to be invoked. For tracing, it must be used with Rooted or PersistentRooted,
-// or barriered and traced manually. For sweeping, currently it requires an
-// explicit call to <map>.sweep().
+// The MapSweepPolicy template parameter controls how the table drops entries
+// when swept. GCHashMap::sweep applies MapSweepPolicy::needsSweep to each table
+// entry; if it returns true, the entry is dropped. The default MapSweepPolicy
+// drops the entry if either the key or value is about to be finalized,
+// according to its GCPolicy<T>::needsSweep method. (This default is almost
+// always fine: it's hard to imagine keeping such an entry around anyway.)
 //
+// Note that this HashMap only knows *how* to trace and sweep, but it does not
+// itself cause tracing or sweeping to be invoked. For tracing, it must be used
+// with Rooted or PersistentRooted, or barriered and traced manually. For
+// sweeping, currently it requires an explicit call to <map>.sweep().
 template <typename Key,
           typename Value,
           typename HashPolicy = DefaultHasher<Key>,
@@ -90,6 +92,10 @@ class GCHashMap : public HashMap<Key, Value, HashPolicy, AllocPolicy>
 };
 
 // HashMap that supports rekeying.
+//
+// If your keys are pointers to something like JSObject that can be tenured or
+// compacted, prefer to use GCHashMap with MovableCellHasher, which takes
+// advantage of the Zone's stable id table to make rekeying unnecessary.
 template <typename Key,
           typename Value,
           typename HashPolicy = DefaultHasher<Key>,

js/public/GCPolicyAPI.h

@@ -8,27 +8,34 @@
 
 // A GCPolicy controls how the GC interacts with both direct pointers to GC
 // things (e.g. JSObject* or JSString*), tagged and/or optional pointers to GC
-// things (e.g.  Value or jsid), and C++ aggregate types (e.g.
+// things (e.g.  Value or jsid), and C++ container types (e.g.
 // JSPropertyDescriptor or GCHashMap).
 //
 // The GCPolicy provides at a minimum:
 //
 //   static T initial()
-//       - Tells the GC how to construct an empty T.
+//       - Construct and return an empty T.
 //
 //   static void trace(JSTracer, T* tp, const char* name)
-//       - Tells the GC how to traverse the edge. In the case of an aggregate,
-//         describe how to trace the children.
+//       - Trace the edge |*tp|, calling the edge |name|. Containers like
+//         GCHashMap and GCHashSet use this method to trace their children.
 //
 //   static bool needsSweep(T* tp)
-//       - Tells the GC how to determine if an edge is about to be finalized,
-//         and potentially updates the edge for moving GC if not. For
-//         aggregates, it determines the weakness semantics of storing the
-//         aggregate inside a weak container of some sort. For example, you
-//         might specialize a weak table's key type GCPolicy to describe
-//         when an entry should be kept during sweeping. This is the primary
-//         reason that GC-supporting weak containers can override the [sweep?]
-//         policy on a per-container basis.
+//       - Return true if |*tp| is about to be finalized. Otherwise, update the
+//         edge for moving GC, and return false. Containers like GCHashMap and
+//         GCHashSet use this method to decide when to remove an entry: if this
+//         function returns true on a key/value/member/etc, its entry is dropped
+//         from the container. Specializing this method is the standard way to
+//         get custom weak behavior from a container type.
+//
+// The default GCPolicy<T> assumes that T has a default constructor and |trace|
+// and |needsSweep| methods, and forwards to them. GCPolicy has appropriate
+// specializations for pointers to GC things and pointer-like types like
+// JS::Heap<T> and mozilla::UniquePtr<T>.
+//
+// There are some stock structs your specializations can inherit from.
+// IgnoreGCPolicy<T> does nothing. StructGCPolicy<T> forwards the methods to the
+// referent type T.
 
 #ifndef GCPolicyAPI_h
 #define GCPolicyAPI_h
@@ -49,8 +56,8 @@ class Symbol;
 
 namespace js {
 
-// Defines a policy for aggregate types with non-GC, i.e. C storage. This
-// policy dispatches to the underlying aggregate for GC interactions.
+// Defines a policy for container types with non-GC, i.e. C storage. This
+// policy dispatches to the underlying struct for GC interactions.
 template <typename T>
 struct StructGCPolicy
 {