Bug 1124973 (part 1) - Always use inheritance rather than composition for PLDHashTable entries. r=froydnj.
authorNicholas Nethercote <nnethercote@mozilla.com>
Thu, 22 Jan 2015 21:05:52 -0800
changeset 239563 1c86241afc93376e4068c10a07bd0f82d78e5095
parent 239562 a70ae54bd3186d1fd0cad6b5167c27b404001419
child 239564 9e2dbe7f144bd4ebcacf6c8ef05912cf9efbcf37
push id500
push userjoshua.m.grant@gmail.com
push dateThu, 29 Jan 2015 01:48:36 +0000
reviewersfroydnj
bugs1124973
milestone38.0a1
Bug 1124973 (part 1) - Always use inheritance rather than composition for PLDHashTable entries. r=froydnj. Because (a) this is how it's usually done, (b) it allows static_cast<> instead of reinterpret_cast<>, and (c) it will make subsequent patches easier.
dom/xul/templates/nsContentSupportMap.h
dom/xul/templates/nsTemplateMap.h
rdf/base/nsInMemoryDataSource.cpp
xpcom/glue/nsTHashtable.h
xpcom/glue/pldhash.h
--- a/dom/xul/templates/nsContentSupportMap.h
+++ b/dom/xul/templates/nsContentSupportMap.h
@@ -27,44 +27,45 @@ public:
     nsresult Put(nsIContent* aElement, nsTemplateMatch* aMatch) {
         if (!mMap.IsInitialized())
             return NS_ERROR_NOT_INITIALIZED;
 
         PLDHashEntryHdr* hdr = PL_DHashTableAdd(&mMap, aElement);
         if (!hdr)
             return NS_ERROR_OUT_OF_MEMORY;
 
-        Entry* entry = reinterpret_cast<Entry*>(hdr);
+        Entry* entry = static_cast<Entry*>(hdr);
         NS_ASSERTION(entry->mMatch == nullptr, "over-writing entry");
         entry->mContent = aElement;
         entry->mMatch   = aMatch;
-        return NS_OK; }
+        return NS_OK;
+    }
 
     bool Get(nsIContent* aElement, nsTemplateMatch** aMatch) {
         if (!mMap.IsInitialized())
             return false;
 
         PLDHashEntryHdr* hdr = PL_DHashTableLookup(&mMap, aElement);
         if (PL_DHASH_ENTRY_IS_FREE(hdr))
             return false;
 
-        Entry* entry = reinterpret_cast<Entry*>(hdr);
+        Entry* entry = static_cast<Entry*>(hdr);
         *aMatch = entry->mMatch;
-        return true; }
+        return true;
+    }
 
     nsresult Remove(nsIContent* aElement);
 
     void Clear() { Finish(); Init(); }
 
 protected:
     PLDHashTable mMap;
 
     void Init();
     void Finish();
 
-    struct Entry {
-        PLDHashEntryHdr  mHdr;
+    struct Entry : public PLDHashEntryHdr {
         nsIContent*      mContent;
         nsTemplateMatch* mMatch;
     };
 };
 
 #endif
--- a/dom/xul/templates/nsTemplateMap.h
+++ b/dom/xul/templates/nsTemplateMap.h
@@ -6,18 +6,17 @@
 #ifndef nsTemplateMap_h__
 #define nsTemplateMap_h__
 
 #include "pldhash.h"
 #include "nsXULElement.h"
 
 class nsTemplateMap {
 protected:
-    struct Entry {
-        PLDHashEntryHdr mHdr;
+    struct Entry : public PLDHashEntryHdr {
         nsIContent*     mContent;
         nsIContent*     mTemplate;
     };
 
     PLDHashTable mTable;
 
     void
     Init()
@@ -33,18 +32,17 @@ public:
 
     ~nsTemplateMap() { Finish(); }
 
     void
     Put(nsIContent* aContent, nsIContent* aTemplate) {
         NS_ASSERTION(PL_DHASH_ENTRY_IS_FREE(PL_DHashTableLookup(&mTable, aContent)),
                      "aContent already in map");
 
-        Entry* entry =
-            reinterpret_cast<Entry*>(PL_DHashTableAdd(&mTable, aContent));
+        Entry* entry = static_cast<Entry*>(PL_DHashTableAdd(&mTable, aContent));
 
         if (entry) {
             entry->mContent = aContent;
             entry->mTemplate = aTemplate;
         }
     }
 
     void
@@ -57,19 +55,19 @@ public:
             Remove(child);
         }
     }
 
 
     void
     GetTemplateFor(nsIContent* aContent, nsIContent** aResult) {
         Entry* entry =
-            reinterpret_cast<Entry*>(PL_DHashTableLookup(&mTable, aContent));
+            static_cast<Entry*>(PL_DHashTableLookup(&mTable, aContent));
 
-        if (PL_DHASH_ENTRY_IS_BUSY(&entry->mHdr))
+        if (PL_DHASH_ENTRY_IS_BUSY(entry))
             NS_IF_ADDREF(*aResult = entry->mTemplate);
         else
             *aResult = nullptr;
     }
 
     void
     Clear() { Finish(); Init(); }
 };
--- a/rdf/base/nsInMemoryDataSource.cpp
+++ b/rdf/base/nsInMemoryDataSource.cpp
@@ -141,18 +141,17 @@ public:
     // also shared between hash/as (see the union above)
     // but placed after union definition to ensure that
     // all 32-bit entries are long aligned
     uint16_t                    mRefCnt;
     bool                        mHashEntry;
 };
 
 
-struct Entry {
-    PLDHashEntryHdr mHdr;
+struct Entry : PLDHashEntryHdr {
     nsIRDFNode*     mNode;
     Assertion*      mAssertions;
 };
 
 
 Assertion::Assertion(nsIRDFResource* aSource)
     : mSource(aSource),
       mNext(nullptr),
@@ -212,17 +211,17 @@ Assertion::~Assertion()
         NS_RELEASE(u.as.mTarget);
     }
 }
 
 PLDHashOperator
 Assertion::DeletePropertyHashEntry(PLDHashTable* aTable, PLDHashEntryHdr* aHdr,
                                            uint32_t aNumber, void* aArg)
 {
-    Entry* entry = reinterpret_cast<Entry*>(aHdr);
+    Entry* entry = static_cast<Entry*>(aHdr);
 
     Assertion* as = entry->mAssertions;
     while (as) {
         Assertion* doomed = as;
         as = as->mNext;
 
         // Unlink, and release the datasource's reference.
         doomed->mNext = doomed->u.as.mInvNext = nullptr;
@@ -317,44 +316,46 @@ protected:
                             uint32_t aNumber, void* aArg);
 
 public:
     // Implementation methods
     Assertion*
     GetForwardArcs(nsIRDFResource* u) {
         PLDHashEntryHdr* hdr = PL_DHashTableLookup(&mForwardArcs, u);
         return PL_DHASH_ENTRY_IS_BUSY(hdr)
-            ? reinterpret_cast<Entry*>(hdr)->mAssertions
-            : nullptr; }
+            ? static_cast<Entry*>(hdr)->mAssertions
+            : nullptr;
+    }
 
     Assertion*
     GetReverseArcs(nsIRDFNode* v) {
         PLDHashEntryHdr* hdr = PL_DHashTableLookup(&mReverseArcs, v);
         return PL_DHASH_ENTRY_IS_BUSY(hdr)
-            ? reinterpret_cast<Entry*>(hdr)->mAssertions
-            : nullptr; }
+            ? static_cast<Entry*>(hdr)->mAssertions
+            : nullptr;
+    }
 
     void
     SetForwardArcs(nsIRDFResource* u, Assertion* as) {
         if (as) {
-            Entry* entry = reinterpret_cast<Entry*>(PL_DHashTableAdd(&mForwardArcs, u));
+            Entry* entry = static_cast<Entry*>(PL_DHashTableAdd(&mForwardArcs, u));
             if (entry) {
                 entry->mNode = u;
                 entry->mAssertions = as;
             }
         }
         else {
             PL_DHashTableRemove(&mForwardArcs, u);
         }
     }
 
     void
     SetReverseArcs(nsIRDFNode* v, Assertion* as) {
         if (as) {
-            Entry* entry = reinterpret_cast<Entry*>(PL_DHashTableAdd(&mReverseArcs, v));
+            Entry* entry = static_cast<Entry*>(PL_DHashTableAdd(&mReverseArcs, v));
             if (entry) {
                 entry->mNode = v;
                 entry->mAssertions = as;
             }
         }
         else {
             PL_DHashTableRemove(&mReverseArcs, v);
         }
@@ -442,17 +443,17 @@ InMemoryAssertionEnumeratorImpl::InMemor
     if (mSource) {
         mNextAssertion = mDataSource->GetForwardArcs(mSource);
 
         if (mNextAssertion && mNextAssertion->mHashEntry) {
             // its our magical HASH_ENTRY forward hash for assertions
             PLDHashEntryHdr* hdr = PL_DHashTableLookup(mNextAssertion->u.hash.mPropertyHash,
                 aProperty);
             mNextAssertion = PL_DHASH_ENTRY_IS_BUSY(hdr)
-                ? reinterpret_cast<Entry*>(hdr)->mAssertions
+                ? static_cast<Entry*>(hdr)->mAssertions
                 : nullptr;
         }
     }
     else {
         mNextAssertion = mDataSource->GetReverseArcs(mTarget);
     }
 
     // Add an owning reference from the enumerator
@@ -588,17 +589,17 @@ public:
 };
 
 
 PLDHashOperator
 InMemoryArcsEnumeratorImpl::ArcEnumerator(PLDHashTable* aTable,
                                        PLDHashEntryHdr* aHdr,
                                        uint32_t aNumber, void* aArg)
 {
-    Entry* entry = reinterpret_cast<Entry*>(aHdr);
+    Entry* entry = static_cast<Entry*>(aHdr);
     nsISupportsArray* resources = static_cast<nsISupportsArray*>(aArg);
 
     resources->AppendElement(entry->mNode);
     return PL_DHASH_NEXT;
 }
 
 
 InMemoryArcsEnumeratorImpl::InMemoryArcsEnumeratorImpl(InMemoryDataSource* aDataSource,
@@ -823,17 +824,17 @@ InMemoryDataSource::~InMemoryDataSource(
 
     MOZ_COUNT_DTOR(InMemoryDataSource);
 }
 
 PLDHashOperator
 InMemoryDataSource::DeleteForwardArcsEntry(PLDHashTable* aTable, PLDHashEntryHdr* aHdr,
                                            uint32_t aNumber, void* aArg)
 {
-    Entry* entry = reinterpret_cast<Entry*>(aHdr);
+    Entry* entry = static_cast<Entry*>(aHdr);
 
     Assertion* as = entry->mAssertions;
     while (as) {
         Assertion* doomed = as;
         as = as->mNext;
 
         // Unlink, and release the datasource's reference.
         doomed->mNext = doomed->u.as.mInvNext = nullptr;
@@ -976,17 +977,17 @@ InMemoryDataSource::GetTarget(nsIRDFReso
     NS_PRECONDITION(target != nullptr, "null ptr");
     if (! target)
         return NS_ERROR_NULL_POINTER;
 
     Assertion *as = GetForwardArcs(source);
     if (as && as->mHashEntry) {
         PLDHashEntryHdr* hdr = PL_DHashTableLookup(as->u.hash.mPropertyHash, property);
         Assertion* val = PL_DHASH_ENTRY_IS_BUSY(hdr)
-            ? reinterpret_cast<Entry*>(hdr)->mAssertions
+            ? static_cast<Entry*>(hdr)->mAssertions
             : nullptr;
         while (val) {
             if (tv == val->u.as.mTruthValue) {
                 *target = val->u.as.mTarget;
                 NS_IF_ADDREF(*target);
                 return NS_OK;
             }
             val = val->mNext;
@@ -1022,17 +1023,17 @@ InMemoryDataSource::HasAssertion(nsIRDFR
 
     if (! target)
         return NS_ERROR_NULL_POINTER;
 
     Assertion *as = GetForwardArcs(source);
     if (as && as->mHashEntry) {
         PLDHashEntryHdr* hdr = PL_DHashTableLookup(as->u.hash.mPropertyHash, property);
         Assertion* val = PL_DHASH_ENTRY_IS_BUSY(hdr)
-            ? reinterpret_cast<Entry*>(hdr)->mAssertions
+            ? static_cast<Entry*>(hdr)->mAssertions
             : nullptr;
         while (val) {
             if ((val->u.as.mTarget == target) && (tv == (val->u.as.mTruthValue))) {
                 *hasAssertion = true;
                 return NS_OK;
             }
             val = val->mNext;
         }
@@ -1135,17 +1136,17 @@ InMemoryDataSource::LockedAssert(nsIRDFR
     Assertion* next = GetForwardArcs(aSource);
     Assertion* prev = next;
     Assertion* as = nullptr;
 
     bool    haveHash = (next) ? next->mHashEntry : false;
     if (haveHash) {
         PLDHashEntryHdr* hdr = PL_DHashTableLookup(next->u.hash.mPropertyHash, aProperty);
         Assertion* val = PL_DHASH_ENTRY_IS_BUSY(hdr)
-            ? reinterpret_cast<Entry*>(hdr)->mAssertions
+            ? static_cast<Entry*>(hdr)->mAssertions
             : nullptr;
         while (val) {
             if (val->u.as.mTarget == aTarget) {
                 // Wow, we already had the assertion. Make sure that the
                 // truth values are correct and bail.
                 val->u.as.mTruthValue = aTruthValue;
                 return NS_OK;
             }
@@ -1177,29 +1178,29 @@ InMemoryDataSource::LockedAssert(nsIRDFR
     // Add the datasource's owning reference.
     as->AddRef();
 
     if (haveHash)
     {
         PLDHashEntryHdr* hdr = PL_DHashTableLookup(next->u.hash.mPropertyHash,
             aProperty);
         Assertion *asRef = PL_DHASH_ENTRY_IS_BUSY(hdr)
-            ? reinterpret_cast<Entry*>(hdr)->mAssertions
+            ? static_cast<Entry*>(hdr)->mAssertions
             : nullptr;
         if (asRef)
         {
             as->mNext = asRef->mNext;
             asRef->mNext = as;
         }
         else
         {
             hdr = PL_DHashTableAdd(next->u.hash.mPropertyHash, aProperty);
             if (hdr)
             {
-                Entry* entry = reinterpret_cast<Entry*>(hdr);
+                Entry* entry = static_cast<Entry*>(hdr);
                 entry->mNode = aProperty;
                 entry->mAssertions = as;
             }
         }
     }
     else
     {
         // Link it in to the "forward arcs" table
@@ -1272,23 +1273,23 @@ InMemoryDataSource::LockedUnassert(nsIRD
 #ifdef PR_LOGGING
     LogOperation("UNASSERT", aSource, aProperty, aTarget);
 #endif
 
     Assertion* next = GetForwardArcs(aSource);
     Assertion* prev = next;
     Assertion* root = next;
     Assertion* as = nullptr;
-    
+
     bool    haveHash = (next) ? next->mHashEntry : false;
     if (haveHash) {
         PLDHashEntryHdr* hdr = PL_DHashTableLookup(next->u.hash.mPropertyHash,
             aProperty);
         prev = next = PL_DHASH_ENTRY_IS_BUSY(hdr)
-            ? reinterpret_cast<Entry*>(hdr)->mAssertions
+            ? static_cast<Entry*>(hdr)->mAssertions
             : nullptr;
         bool first = true;
         while (next) {
             if (aTarget == next->u.as.mTarget) {
                 break;
             }
             first = false;
             prev = next;
@@ -1302,17 +1303,17 @@ InMemoryDataSource::LockedUnassert(nsIRD
 
         if (first) {
             PL_DHashTableRawRemove(root->u.hash.mPropertyHash, hdr);
 
             if (next && next->mNext) {
                 PLDHashEntryHdr* hdr = PL_DHashTableAdd(root->u.hash.mPropertyHash,
                                      aProperty);
                 if (hdr) {
-                    Entry* entry = reinterpret_cast<Entry*>(hdr);
+                    Entry* entry = static_cast<Entry*>(hdr);
                     entry->mNode = aProperty;
                     entry->mAssertions = next->mNext;
                 }
             }
             else {
                 // If this second-level hash empties out, clean it up.
                 if (!root->u.hash.mPropertyHash->EntryCount()) {
                     root->Release();
@@ -1573,25 +1574,25 @@ InMemoryDataSource::HasArcIn(nsIRDFNode 
             return NS_OK;
         }
         ass = ass->u.as.mInvNext;
     }
     *result = false;
     return NS_OK;
 }
 
-NS_IMETHODIMP 
+NS_IMETHODIMP
 InMemoryDataSource::HasArcOut(nsIRDFResource *aSource, nsIRDFResource *aArc, bool *result)
 {
     Assertion* ass = GetForwardArcs(aSource);
     if (ass && ass->mHashEntry) {
         PLDHashEntryHdr* hdr = PL_DHashTableLookup(ass->u.hash.mPropertyHash,
             aArc);
         Assertion* val = PL_DHASH_ENTRY_IS_BUSY(hdr)
-            ? reinterpret_cast<Entry*>(hdr)->mAssertions
+            ? static_cast<Entry*>(hdr)->mAssertions
             : nullptr;
         if (val) {
             *result = true;
             return NS_OK;
         }
         ass = ass->mNext;
     }
     while (ass) {
@@ -1644,17 +1645,17 @@ InMemoryDataSource::ArcLabelsOut(nsIRDFR
     return NS_OK;
 }
 
 PLDHashOperator
 InMemoryDataSource::ResourceEnumerator(PLDHashTable* aTable,
                                        PLDHashEntryHdr* aHdr,
                                        uint32_t aNumber, void* aArg)
 {
-    Entry* entry = reinterpret_cast<Entry*>(aHdr);
+    Entry* entry = static_cast<Entry*>(aHdr);
     static_cast<nsCOMArray<nsIRDFNode>*>(aArg)->AppendObject(entry->mNode);
     return PL_DHASH_NEXT;
 }
 
 
 NS_IMETHODIMP
 InMemoryDataSource::GetAllResources(nsISimpleEnumerator** aResult)
 {
@@ -1742,26 +1743,26 @@ InMemoryDataSource::EnsureFastContainmen
     Assertion *nextRef;
     while(first) {
         nextRef = first->mNext;
         nsIRDFResource *prop = first->u.as.mProperty;
 
         PLDHashEntryHdr* hdr = PL_DHashTableLookup(table,
             prop);
         Assertion* val = PL_DHASH_ENTRY_IS_BUSY(hdr)
-            ? reinterpret_cast<Entry*>(hdr)->mAssertions
+            ? static_cast<Entry*>(hdr)->mAssertions
             : nullptr;
         if (val) {
             first->mNext = val->mNext;
             val->mNext = first;
         }
         else {
             PLDHashEntryHdr* hdr = PL_DHashTableAdd(table, prop);
             if (hdr) {
-                Entry* entry = reinterpret_cast<Entry*>(hdr);
+                Entry* entry = static_cast<Entry*>(hdr);
                 entry->mNode = prop;
                 entry->mAssertions = first;
                 first->mNext = nullptr;
             }
         }
         first = nextRef;
     }
     return(NS_OK);
@@ -1807,17 +1808,17 @@ InMemoryDataSource::Mark(nsIRDFResource*
     if (! aTarget)
         return NS_ERROR_NULL_POINTER;
 
     Assertion *as = GetForwardArcs(aSource);
     if (as && as->mHashEntry) {
         PLDHashEntryHdr* hdr = PL_DHashTableLookup(as->u.hash.mPropertyHash,
             aProperty);
         Assertion* val = PL_DHASH_ENTRY_IS_BUSY(hdr)
-            ? reinterpret_cast<Entry*>(hdr)->mAssertions
+            ? static_cast<Entry*>(hdr)->mAssertions
             : nullptr;
         while (val) {
             if ((val->u.as.mTarget == aTarget) &&
                 (aTruthValue == (val->u.as.mTruthValue))) {
 
                 // found it! so mark it.
                 as->Mark();
                 *aDidMark = true;
@@ -1901,17 +1902,17 @@ InMemoryDataSource::Sweep()
 
 
 PLDHashOperator
 InMemoryDataSource::SweepForwardArcsEntries(PLDHashTable* aTable,
                                             PLDHashEntryHdr* aHdr,
                                             uint32_t aNumber, void* aArg)
 {
     PLDHashOperator result = PL_DHASH_NEXT;
-    Entry* entry = reinterpret_cast<Entry*>(aHdr);
+    Entry* entry = static_cast<Entry*>(aHdr);
     SweepInfo* info = static_cast<SweepInfo*>(aArg);
 
     Assertion* as = entry->mAssertions;
     if (as && (as->mHashEntry))
     {
         // Stuff in sub-hashes must be swept recursively (max depth: 1)
         PL_DHashTableEnumerate(as->u.hash.mPropertyHash,
                                SweepForwardArcsEntries, info);
@@ -1943,17 +1944,17 @@ InMemoryDataSource::SweepForwardArcsEntr
                 entry->mAssertions = next;
             }
 
             // remove from the reverse arcs
             PLDHashEntryHdr* hdr =
                 PL_DHashTableLookup(info->mReverseArcs, as->u.as.mTarget);
             NS_ASSERTION(PL_DHASH_ENTRY_IS_BUSY(hdr), "no assertion in reverse arcs");
 
-            Entry* rentry = reinterpret_cast<Entry*>(hdr);
+            Entry* rentry = static_cast<Entry*>(hdr);
             Assertion* ras = rentry->mAssertions;
             Assertion* rprev = nullptr;
             while (ras) {
                 if (ras == as) {
                     if (rprev) {
                         rprev->u.as.mInvNext = ras->u.as.mInvNext;
                     }
                     else {
@@ -2001,17 +2002,17 @@ public:
     {}
     rdfITripleVisitor* mVisitor;
     nsresult mRv;
 };
 
 PLDHashOperator
 SubjectEnumerator(PLDHashTable* aTable, PLDHashEntryHdr* aHdr,
                   uint32_t aNumber, void* aArg) {
-    Entry* entry = reinterpret_cast<Entry*>(aHdr);
+    Entry* entry = static_cast<Entry*>(aHdr);
     VisitorClosure* closure = static_cast<VisitorClosure*>(aArg);
 
     nsresult rv;
     nsCOMPtr<nsIRDFNode> subject = do_QueryInterface(entry->mNode, &rv);
     NS_ENSURE_SUCCESS(rv, PL_DHASH_NEXT);
 
     closure->mRv = closure->mVisitor->Visit(subject, nullptr, nullptr, true);
     if (NS_FAILED(closure->mRv) || closure->mRv == NS_RDF_STOP_VISIT)
@@ -2043,17 +2044,17 @@ public:
         mSubject(aSubject), mOuter(aClosure) {}
     nsIRDFNode* mSubject;
     VisitorClosure* mOuter;
 };
 
 PLDHashOperator
 TriplesInnerEnumerator(PLDHashTable* aTable, PLDHashEntryHdr* aHdr,
                   uint32_t aNumber, void* aArg) {
-    Entry* entry = reinterpret_cast<Entry*>(aHdr);
+    Entry* entry = static_cast<Entry*>(aHdr);
     Assertion* assertion = entry->mAssertions;
     TriplesInnerClosure* closure = 
         static_cast<TriplesInnerClosure*>(aArg);
     while (assertion) {
         NS_ASSERTION(!assertion->mHashEntry, "shouldn't have to hashes");
         VisitorClosure* cls = closure->mOuter;
         cls->mRv = cls->mVisitor->Visit(closure->mSubject,
                                         assertion->u.as.mProperty,
@@ -2064,17 +2065,17 @@ TriplesInnerEnumerator(PLDHashTable* aTa
         }
         assertion = assertion->mNext;
     }
     return PL_DHASH_NEXT;
 }
 PLDHashOperator
 TriplesEnumerator(PLDHashTable* aTable, PLDHashEntryHdr* aHdr,
                   uint32_t aNumber, void* aArg) {
-    Entry* entry = reinterpret_cast<Entry*>(aHdr);
+    Entry* entry = static_cast<Entry*>(aHdr);
     VisitorClosure* closure = static_cast<VisitorClosure*>(aArg);
 
     nsresult rv;
     nsCOMPtr<nsIRDFNode> subject = do_QueryInterface(entry->mNode, &rv);
     NS_ENSURE_SUCCESS(rv, PL_DHASH_NEXT);
 
     if (entry->mAssertions->mHashEntry) {
         TriplesInnerClosure cls(subject, closure);
--- a/xpcom/glue/nsTHashtable.h
+++ b/xpcom/glue/nsTHashtable.h
@@ -124,17 +124,17 @@ public:
    * @return    pointer to the entry class, if the key exists; nullptr if the
    *            key doesn't exist
    */
   EntryType* GetEntry(KeyType aKey) const
   {
     NS_ASSERTION(mTable.IsInitialized(),
                  "nsTHashtable was not initialized properly.");
 
-    EntryType* entry = reinterpret_cast<EntryType*>(
+    EntryType* entry = static_cast<EntryType*>(
       PL_DHashTableLookup(const_cast<PLDHashTable*>(&mTable),
                           EntryType::KeyToPointer(aKey)));
     return PL_DHASH_ENTRY_IS_BUSY(entry) ? entry : nullptr;
   }
 
   /**
    * Return true if an entry for the given key exists, false otherwise.
    * @param     aKey the key to retrieve
@@ -478,17 +478,17 @@ nsTHashtable<EntryType>::s_CopyEntry(PLD
   fromEntry->~EntryType();
 }
 
 template<class EntryType>
 void
 nsTHashtable<EntryType>::s_ClearEntry(PLDHashTable* aTable,
                                       PLDHashEntryHdr* aEntry)
 {
-  reinterpret_cast<EntryType*>(aEntry)->~EntryType();
+  static_cast<EntryType*>(aEntry)->~EntryType();
 }
 
 template<class EntryType>
 bool
 nsTHashtable<EntryType>::s_InitEntry(PLDHashTable* aTable,
                                      PLDHashEntryHdr* aEntry,
                                      const void* aKey)
 {
@@ -500,29 +500,29 @@ template<class EntryType>
 PLDHashOperator
 nsTHashtable<EntryType>::s_EnumStub(PLDHashTable* aTable,
                                     PLDHashEntryHdr* aEntry,
                                     uint32_t aNumber,
                                     void* aArg)
 {
   // dereferences the function-pointer to the user's enumeration function
   return (*reinterpret_cast<s_EnumArgs*>(aArg)->userFunc)(
-    reinterpret_cast<EntryType*>(aEntry),
+    static_cast<EntryType*>(aEntry),
     reinterpret_cast<s_EnumArgs*>(aArg)->userArg);
 }
 
 template<class EntryType>
 size_t
 nsTHashtable<EntryType>::s_SizeOfStub(PLDHashEntryHdr* aEntry,
                                       mozilla::MallocSizeOf aMallocSizeOf,
                                       void* aArg)
 {
   // dereferences the function-pointer to the user's enumeration function
   return (*reinterpret_cast<s_SizeOfArgs*>(aArg)->userFunc)(
-    reinterpret_cast<EntryType*>(aEntry),
+    static_cast<EntryType*>(aEntry),
     aMallocSizeOf,
     reinterpret_cast<s_SizeOfArgs*>(aArg)->userArg);
 }
 
 class nsCycleCollectionTraversalCallback;
 
 struct MOZ_STACK_CLASS nsTHashtableCCTraversalData
 {
--- a/xpcom/glue/pldhash.h
+++ b/xpcom/glue/pldhash.h
@@ -65,21 +65,21 @@ struct PLDHashTableOps;
  * In order to allow in-line allocation of key and value, we do not declare
  * either here.  Instead, the API uses const void *key as a formal parameter.
  * The key need not be stored in the entry; it may be part of the value, but
  * need not be stored at all.
  *
  * Callback types are defined below and grouped into the PLDHashTableOps
  * structure, for single static initialization per hash table sub-type.
  *
- * Each hash table sub-type should nest the PLDHashEntryHdr structure at the
- * front of its particular entry type.  The keyHash member contains the result
- * of multiplying the hash code returned from the hashKey callback (see below)
- * by PL_DHASH_GOLDEN_RATIO, then constraining the result to avoid the magic 0
- * and 1 values.  The stored keyHash value is table size invariant, and it is
+ * Each hash table sub-type should make its entry type a subclass of
+ * PLDHashEntryHdr. The keyHash member contains the result of multiplying the
+ * hash code returned from the hashKey callback (see below) by
+ * PL_DHASH_GOLDEN_RATIO, then constraining the result to avoid the magic 0 and
+ * 1 values. The stored keyHash value is table size invariant, and it is
  * maintained automatically -- users should never set it, and its only uses
  * should be via the entry macros below.
  *
  * However, use PL_DHASH_ENTRY_IS_BUSY for faster liveness testing of entries
  * returned by PL_DHashTableLookup and PL_DHashTableAdd, as these functions
  * never return a non-live, busy (i.e., removed) entry pointer to its caller.
  * See below for more details on these functions.
  */