Bug 1481138 - Remove Hash{Map,Set}::putNewInfallible. r=luke
The difference between putNew() and putNewInfallible() is subtle -- the latter
omits three checks. Omitting two of them (SimulatedOOM and checkOverloaded())
is reasonable, but omitting the EnsureHash() check is dubious.
Furthermore, putNewInfallible() has very few uses; in my opinion, not enough to
warrant its existence. So this patch removes it, simplifying the API and the
implementation.
MozReview-Commit-ID: 3KfpVERhqTh
--- a/js/public/UbiNodeDominatorTree.h
+++ b/js/public/UbiNodeDominatorTree.h
@@ -359,17 +359,17 @@ class JS_PUBLIC_API(DominatorTree)
static MOZ_MUST_USE bool mapNodesToTheirIndices(JS::ubi::Vector<Node>& postOrder,
NodeToIndexMap& map) {
MOZ_ASSERT(!map.initialized());
MOZ_ASSERT(postOrder.length() < UINT32_MAX);
uint32_t length = postOrder.length();
if (!map.init(length))
return false;
for (uint32_t i = 0; i < length; i++)
- map.putNewInfallible(postOrder[i], i);
+ MOZ_ALWAYS_TRUE(map.putNew(postOrder[i], i));
return true;
}
// Convert the Node -> NodeSet predecessorSets to a index -> Vector<index>
// form.
static MOZ_MUST_USE bool convertPredecessorSetsToVectors(
const Node& root,
JS::ubi::Vector<Node>& postOrder,
--- a/js/src/vm/JSScript.cpp
+++ b/js/src/vm/JSScript.cpp
@@ -3230,17 +3230,17 @@ js::GetSrcNote(GSNCache& cache, JSScript
}
if (cache.map.init(nsrcnotes)) {
pc = script->code();
for (jssrcnote* sn = script->notes(); !SN_IS_TERMINATOR(sn);
sn = SN_NEXT(sn))
{
pc += SN_DELTA(sn);
if (SN_IS_GETTABLE(sn))
- cache.map.putNewInfallible(pc, sn);
+ MOZ_ALWAYS_TRUE(cache.map.putNew(pc, sn));
}
cache.code = script->code();
}
}
return result;
}
--- a/js/src/vm/Shape.cpp
+++ b/js/src/vm/Shape.cpp
@@ -1628,18 +1628,18 @@ ShapeHasher::match(const Key k, const Lo
static KidsHash*
HashChildren(Shape* kid1, Shape* kid2)
{
auto hash = MakeUnique<KidsHash>();
if (!hash || !hash->init(2))
return nullptr;
- hash->putNewInfallible(StackShape(kid1), kid1);
- hash->putNewInfallible(StackShape(kid2), kid2);
+ MOZ_ALWAYS_TRUE(hash->putNew(StackShape(kid1), kid1));
+ MOZ_ALWAYS_TRUE(hash->putNew(StackShape(kid2), kid2));
return hash.release();
}
bool
PropertyTree::insertChild(JSContext* cx, Shape* parent, Shape* child)
{
MOZ_ASSERT(!parent->inDictionary());
MOZ_ASSERT(!child->parent);
--- a/mfbt/HashTable.h
+++ b/mfbt/HashTable.h
@@ -276,24 +276,16 @@ public:
// Like put(), but asserts that the given key is not already present.
template<typename KeyInput, typename ValueInput>
MOZ_MUST_USE bool putNew(KeyInput&& aKey, ValueInput&& aValue)
{
return mImpl.putNew(
aKey, std::forward<KeyInput>(aKey), std::forward<ValueInput>(aValue));
}
- // Only call this to populate an empty map after reserving space with init().
- template<typename KeyInput, typename ValueInput>
- void putNewInfallible(KeyInput&& aKey, ValueInput&& aValue)
- {
- mImpl.putNewInfallible(
- aKey, std::forward<KeyInput>(aKey), std::forward<ValueInput>(aValue));
- }
-
// Like |lookup(l)|, but on miss, |p = lookupForAdd(l)| allows efficient
// insertion of Key |k| (where |HashPolicy::match(k,l) == true|) using
// |add(p,k,v)|. After |add(p,k,v)|, |p| points to the new key/value. E.g.:
//
// using HM = HashMap<int,char>;
// HM h;
// HM::AddPtr p = h.lookupForAdd(3);
// if (!p) {
@@ -578,23 +570,16 @@ public:
// Like the other putNew(), but for when |Lookup| is different to |T|.
template<typename U>
MOZ_MUST_USE bool putNew(const Lookup& aLookup, U&& aU)
{
return mImpl.putNew(aLookup, std::forward<U>(aU));
}
- // Only call this to populate an empty set after reserving space with init().
- template<typename U>
- void putNewInfallible(const Lookup& aLookup, U&& aU)
- {
- mImpl.putNewInfallible(aLookup, std::forward<U>(aU));
- }
-
// Like |lookup(l)|, but on miss, |p = lookupForAdd(l)| allows efficient
// insertion of T value |t| (where |HashPolicy::match(t,l) == true|) using
// |add(p,t)|. After |add(p,t)|, |p| points to the new element. E.g.:
//
// using HS = HashSet<int>;
// HS h;
// HS::AddPtr p = h.lookupForAdd(3);
// if (!p) {
@@ -1969,20 +1954,19 @@ private:
// collision bits correctly on a subsequent pass or skipping the rehash
// unless we are totally filled with tombstones: benchmark to find out
// which approach is best.
}
// Note: |aLookup| may be a reference to a piece of |u|, so this function
// must take care not to use |aLookup| after moving |u|.
//
- // Prefer to use putNewInfallible; this function does not check
- // invariants.
+ // Prefer to use putNew; this function does not check invariants.
template<typename... Args>
- void putNewInfallibleInternal(const Lookup& aLookup, Args&&... aArgs)
+ void putNewInternal(const Lookup& aLookup, Args&&... aArgs)
{
MOZ_ASSERT(mTable);
HashNumber keyHash = prepareHash(aLookup);
Entry* entry = &findFreeEntry(keyHash);
MOZ_ASSERT(entry);
if (entry->isRemoved()) {
@@ -2166,41 +2150,34 @@ public:
#ifdef DEBUG
mMutationCount++;
aPtr.mGeneration = generation();
aPtr.mMutationCount = mMutationCount;
#endif
return true;
}
- // Note: |aLookup| may be a reference to a piece of |u|, so this function
- // must take care not to use |aLookup| after moving |u|.
- template<typename... Args>
- void putNewInfallible(const Lookup& aLookup, Args&&... aArgs)
- {
- MOZ_ASSERT(!lookup(aLookup).found());
- ReentrancyGuard g(*this);
- putNewInfallibleInternal(aLookup, std::forward<Args>(aArgs)...);
- }
-
// Note: |aLookup| may be alias arguments in |aArgs|, so this function must
// take care not to use |aLookup| after moving |aArgs|.
template<typename... Args>
MOZ_MUST_USE bool putNew(const Lookup& aLookup, Args&&... aArgs)
{
+ MOZ_ASSERT(!lookup(aLookup).found()); // must precede the ReentrancyGuard
+ ReentrancyGuard g(*this);
+
if (!this->checkSimulatedOOM()) {
return false;
}
if (!EnsureHash<HashPolicy>(aLookup)) {
return false;
}
if (checkOverloaded() == RehashFailed) {
return false;
}
- putNewInfallible(aLookup, std::forward<Args>(aArgs)...);
+ putNewInternal(aLookup, std::forward<Args>(aArgs)...);
return true;
}
// Note: |aLookup| may be a reference to a piece of |u|, so this function
// must take care not to use |aLookup| after moving |u|.
template<typename... Args>
MOZ_MUST_USE bool relookupOrAdd(AddPtr& aPtr,
const Lookup& aLookup,
@@ -2237,17 +2214,17 @@ public:
{
MOZ_ASSERT(mTable);
ReentrancyGuard g(*this);
MOZ_ASSERT(aPtr.found());
MOZ_ASSERT(aPtr.mGeneration == generation());
typename HashTableEntry<T>::NonConstT t(std::move(*aPtr));
HashPolicy::setKey(t, const_cast<Key&>(aKey));
remove(*aPtr.mEntry);
- putNewInfallibleInternal(aLookup, std::move(t));
+ putNewInternal(aLookup, std::move(t));
}
void rekeyAndMaybeRehash(Ptr aPtr, const Lookup& aLookup, const Key& aKey)
{
rekeyWithoutRehash(aPtr, aLookup, aKey);
checkOverRemoved();
}
};