Bug 1691913 - Mark nsBaseHashtable (and its subclasses') member functions nodiscard where appropriate. r=xpcom-reviewers,necko-reviewers,dragana,nika

Differential Revision: https://phabricator.services.mozilla.com/D105474

dom/base/nsFrameMessageManager.cpp

@@ -1021,8 +1021,7 @@ void MessageManagerReporter::CountReferents(
     }
 
     nsString key(it.Key());
-    uint32_t oldCount = 0;
-    aReferentCount->mMessageCounter.Get(key, &oldCount);
+    const uint32_t oldCount = aReferentCount->mMessageCounter.Get(key);
     uint32_t currentCount = oldCount + listenerCount;
     aReferentCount->mMessageCounter.InsertOrUpdate(key, currentCount);
 
@@ -1084,9 +1083,8 @@ static void ReportReferentCount(
                          aManagerType));
 
   for (uint32_t i = 0; i < aReferentCount.mSuspectMessages.Length(); i++) {
-    uint32_t totalReferentCount = 0;
-    aReferentCount.mMessageCounter.Get(aReferentCount.mSuspectMessages[i],
-                                       &totalReferentCount);
+    const uint32_t totalReferentCount =
+        aReferentCount.mMessageCounter.Get(aReferentCount.mSuspectMessages[i]);
     NS_ConvertUTF16toUTF8 suspect(aReferentCount.mSuspectMessages[i]);
     REPORT(nsPrintfCString("message-manager-suspect/%s/referent(message=%s)",
                            aManagerType, suspect.get()),

dom/media/MediaTrackGraph.cpp

@@ -3132,9 +3132,7 @@ MediaTrackGraph* MediaTrackGraph::GetInstanceIfExists(
       aSampleRate ? aSampleRate : CubebUtils::PreferredSampleRate();
   uint32_t hashkey = WindowToHash(aWindow, sampleRate, aOutputDeviceID);
 
-  MediaTrackGraphImpl* graph = nullptr;
-  gGraphs.Get(hashkey, &graph);
-  return graph;
+  return gGraphs.Get(hashkey);
 }
 
 MediaTrackGraph* MediaTrackGraph::GetInstance(

dom/media/webspeech/synth/nsSynthVoiceRegistry.cpp

@@ -477,8 +477,7 @@ nsSynthVoiceRegistry::GetVoiceName(const nsAString& aUri, nsAString& aRetval) {
 nsresult nsSynthVoiceRegistry::AddVoiceImpl(
     nsISpeechService* aService, const nsAString& aUri, const nsAString& aName,
     const nsAString& aLang, bool aLocalService, bool aQueuesUtterances) {
-  bool found = false;
-  mUriVoiceMap.GetWeak(aUri, &found);
+  const bool found = mUriVoiceMap.Contains(aUri);
   if (NS_WARN_IF(found)) {
     return NS_ERROR_INVALID_ARG;
   }

dom/svg/SVGAttrTearoffTable.h

@@ -7,6 +7,7 @@
 #ifndef DOM_SVG_SVGATTRTEAROFFTABLE_H_
 #define DOM_SVG_SVGATTRTEAROFFTABLE_H_
 
+#include "mozilla/DebugOnly.h"
 #include "nsDataHashtable.h"
 #include "nsDebug.h"
 #include "nsHashKeys.h"
@@ -50,10 +51,7 @@ TearoffType* SVGAttrTearoffTable<SimpleType, TearoffType>::GetTearoff(
 
   TearoffType* tearoff = nullptr;
 
-#ifdef DEBUG
-  bool found =
-#endif
-      mTable->Get(aSimple, &tearoff);
+  DebugOnly<bool> found = mTable->Get(aSimple, &tearoff);
   MOZ_ASSERT(!found || tearoff,
              "null pointer stored in attribute tear-off map");
 

hal/HalWakeLock.cpp

@@ -184,7 +184,7 @@ void ModifyWakeLock(const nsAString& aTopic, hal::WakeLockControl aLockAdjust,
         if (!entry) {
           entry.Insert(MakeUnique<ProcessLockTable>());
         } else {
-          entry.Data()->Get(aProcessID, &processCount);
+          Unused << entry.Data()->Get(aProcessID, &processCount);
           CountWakeLocks(entry.Data().get(), &totalCount);
         }
         return entry.Data().get();

layout/base/nsBidiPresUtils.cpp

@@ -414,9 +414,7 @@ struct MOZ_STACK_CLASS BidiParagraphData {
   }
 
   int32_t GetLastFrameForContent(nsIContent* aContent) {
-    int32_t index = 0;
-    mContentToFrameIndex.Get(aContent, &index);
-    return index;
+    return mContentToFrameIndex.Get(aContent);
   }
 
   int32_t FrameCount() { return mLogicalFrames.Length(); }

layout/generic/nsGridContainerFrame.cpp

@@ -4650,10 +4650,7 @@ void nsGridContainerFrame::Grid::PlaceGridItems(
       LineRange& minor = isRowOrder ? area.mCols : area.mRows;
       if (major.IsDefinite() && minor.IsAuto()) {
         // Items with 'auto' in the minor dimension only.
-        uint32_t cursor = 0;
-        if (isSparse) {
-          cursors->Get(major.mStart, &cursor);
-        }
+        const uint32_t cursor = isSparse ? cursors->Get(major.mStart) : 0;
         (this->*placeAutoMinorFunc)(cursor, &area, clampMaxLine);
         if (isMasonry) {
           item.MaybeInhibitSubgridInMasonry(aState.mFrame, gridAxisTrackCount);

modules/libpref/Preferences.cpp

@@ -3204,9 +3204,7 @@ PreferenceServiceReporter::CollectReports(
       numStrong++;
     }
 
-    uint32_t oldCount = 0;
-    prefCounter.Get(callback->GetDomain(), &oldCount);
-    uint32_t currentCount = oldCount + 1;
+    const uint32_t currentCount = prefCounter.Get(callback->GetDomain()) + 1;
     prefCounter.InsertOrUpdate(callback->GetDomain(), currentCount);
 
     // Keep track of preferences that have a suspiciously large number of
@@ -3218,8 +3216,7 @@ PreferenceServiceReporter::CollectReports(
 
   for (uint32_t i = 0; i < suspectPreferences.Length(); i++) {
     nsCString& suspect = suspectPreferences[i];
-    uint32_t totalReferentCount = 0;
-    prefCounter.Get(suspect, &totalReferentCount);
+    const uint32_t totalReferentCount = prefCounter.Get(suspect);
 
     nsPrintfCString suspectPath(
         "preference-service-suspect/"

netwerk/protocol/http/Http2Session.cpp

@@ -3973,8 +3973,7 @@ uint32_t Http2Session::FindTunnelCount(nsHttpConnectionInfo* aConnInfo) {
 }
 uint32_t Http2Session::FindTunnelCount(nsCString const& aHashKey) {
   MOZ_ASSERT(OnSocketThread(), "not on socket thread");
-  uint32_t rv = 0;
-  mTunnelHash.Get(aHashKey, &rv);
+  uint32_t rv = mTunnelHash.Get(aHashKey);
   return rv;
 }
 

uriloader/preload/PreloadService.cpp

@@ -39,9 +39,7 @@ void PreloadService::DeregisterPreload(const PreloadHashKey& aKey) {
 void PreloadService::ClearAllPreloads() { mPreloads.Clear(); }
 
 bool PreloadService::PreloadExists(const PreloadHashKey& aKey) {
-  bool found;
-  mPreloads.GetWeak(aKey, &found);
-  return found;
+  return mPreloads.Contains(aKey);
 }
 
 already_AddRefed<PreloaderBase> PreloadService::LookupPreload(

widget/WidgetEventImpl.cpp

@@ -1179,9 +1179,8 @@ KeyNameIndex WidgetKeyboardEvent::GetKeyNameIndex(const nsAString& aKeyValue) {
                                              static_cast<KeyNameIndex>(i));
     }
   }
-  KeyNameIndex result = KEY_NAME_INDEX_USE_STRING;
-  sKeyNameIndexHashtable->Get(aKeyValue, &result);
-  return result;
+  return sKeyNameIndexHashtable->MaybeGet(aKeyValue).valueOr(
+      KEY_NAME_INDEX_USE_STRING);
 }
 
 /* static */
@@ -1195,9 +1194,8 @@ CodeNameIndex WidgetKeyboardEvent::GetCodeNameIndex(
                                               static_cast<CodeNameIndex>(i));
     }
   }
-  CodeNameIndex result = CODE_NAME_INDEX_USE_STRING;
-  sCodeNameIndexHashtable->Get(aCodeValue, &result);
-  return result;
+  return sCodeNameIndexHashtable->MaybeGet(aCodeValue)
+      .valueOr(CODE_NAME_INDEX_USE_STRING);
 }
 
 /* static */
@@ -1895,9 +1893,8 @@ EditorInputType InternalEditorInputEvent::GetEditorInputType(
                                           static_cast<EditorInputType>(i));
     }
   }
-  EditorInputType result = EditorInputType::eUnknown;
-  sInputTypeHashtable->Get(aInputType, &result);
-  return result;
+  return sInputTypeHashtable->MaybeGet(aInputType)
+      .valueOr(EditorInputType::eUnknown);
 }
 
 }  // namespace mozilla

xpcom/ds/nsBaseHashtable.h

@@ -117,13 +117,17 @@ class nsBaseHashtable
    * Return the number of entries in the table.
    * @return    number of entries
    */
-  uint32_t Count() const { return nsTHashtable<EntryType>::Count(); }
+  [[nodiscard]] uint32_t Count() const {
+    return nsTHashtable<EntryType>::Count();
+  }
 
   /**
    * Return whether the table is empty.
    * @return    whether empty
    */
-  bool IsEmpty() const { return nsTHashtable<EntryType>::IsEmpty(); }
+  [[nodiscard]] bool IsEmpty() const {
+    return nsTHashtable<EntryType>::IsEmpty();
+  }
 
   /**
    * Get the value, returning a flag indicating the presence of the entry in
@@ -134,8 +138,12 @@ class nsBaseHashtable
    *        If you only need to check if the key exists, aData may be null.
    * @return true if the key exists. If key does not exist, aData is not
    *   modified.
+   *
+   * @attention As opposed to Remove, this does not assign a value to *aData if
+   * no entry is present! (And also as opposed to the member function Get with
+   * the same signature that nsClassHashtable defines and hides this one.)
    */
-  bool Get(KeyType aKey, UserDataType* aData) const {
+  [[nodiscard]] bool Get(KeyType aKey, UserDataType* aData) const {
     EntryType* ent = this->GetEntry(aKey);
     if (!ent) {
       return false;
@@ -162,7 +170,7 @@ class nsBaseHashtable
    * @note If zero/default-initialized values are stored in the table, it is
    *       not possible to distinguish between such a value and a missing entry.
    */
-  UserDataType Get(KeyType aKey) const {
+  [[nodiscard]] UserDataType Get(KeyType aKey) const {
     EntryType* ent = this->GetEntry(aKey);
     if (!ent) {
       return UserDataType{};
@@ -178,7 +186,7 @@ class nsBaseHashtable
    * @return The found value wrapped in a Maybe, or Nothing if no entry was
    *         found with the given key.
    */
-  mozilla::Maybe<UserDataType> MaybeGet(KeyType aKey) const {
+  [[nodiscard]] mozilla::Maybe<UserDataType> MaybeGet(KeyType aKey) const {
     EntryType* ent = this->GetEntry(aKey);
     if (!ent) {
       return mozilla::Nothing();
@@ -257,6 +265,10 @@ class nsBaseHashtable
    *              zero or nullptr for primitive types).
    * @return true if an entry for aKey was found (and removed)
    */
+  // XXX This should also better be marked nodiscard, but due to
+  // nsClassHashtable not guaranteeing non-nullness of entries, it is usually
+  // only checked if aData is nullptr in such cases.
+  // [[nodiscard]]
   bool Remove(KeyType aKey, DataType* aData) {
     if (auto* ent = this->GetEntry(aKey)) {
       if (aData) {
@@ -534,7 +546,7 @@ class nsBaseHashtable
      *
      * \pre HasEntry()
      */
-    DataType& Data() { return Entry()->mData; }
+    [[nodiscard]] DataType& Data() { return Entry()->mData; }
 
    private:
     friend class nsBaseHashtable;
@@ -558,7 +570,7 @@ class nsBaseHashtable
    * trigger debug assertions, and result in undefined behaviour otherwise.
    */
   template <class F>
-  auto WithEntryHandle(KeyType aKey, F&& aFunc)
+  [[nodiscard]] auto WithEntryHandle(KeyType aKey, F&& aFunc)
       -> std::invoke_result_t<F, EntryHandle&&> {
     return Base::WithEntryHandle(
         aKey, [&aFunc](auto entryHandle) -> decltype(auto) {
@@ -576,7 +588,8 @@ class nsBaseHashtable
    * For more details, see the explanation on the non-fallible overload above.
    */
   template <class F>
-  auto WithEntryHandle(KeyType aKey, const fallible_t& aFallible, F&& aFunc)
+  [[nodiscard]] auto WithEntryHandle(KeyType aKey, const fallible_t& aFallible,
+                                     F&& aFunc)
       -> std::invoke_result_t<F, mozilla::Maybe<EntryHandle>&&> {
     return Base::WithEntryHandle(
         aKey, aFallible, [&aFunc](auto maybeEntryHandle) {

xpcom/ds/nsClassHashtable.h

@@ -68,7 +68,7 @@ class nsClassHashtable : public nsBaseHashtable<KeyClass, mozilla::UniquePtr<T>,
    * @copydoc nsBaseHashtable::Get
    * @returns nullptr if the key is not present.
    */
-  UserDataType Get(KeyType aKey) const;
+  [[nodiscard]] UserDataType Get(KeyType aKey) const;
 };
 
 template <typename K, typename T>

xpcom/ds/nsDataHashtable.h

@@ -38,7 +38,7 @@ class nsDataHashtable : public nsBaseHashtable<KeyClass, DataType, DataType> {
    * @return a reference to the found value, or nullptr if no entry was found
    * with the given key.
    */
-  DataType* GetValue(KeyType aKey) {
+  [[nodiscard]] DataType* GetValue(KeyType aKey) {
     if (EntryType* ent = this->GetEntry(aKey)) {
       return ent->GetModifiableData();
     }

xpcom/ds/nsInterfaceHashtable.h

@@ -42,7 +42,7 @@ class nsInterfaceHashtable
   /**
    * @copydoc nsBaseHashtable::Get
    */
-  already_AddRefed<Interface> Get(KeyType aKey) const;
+  [[nodiscard]] already_AddRefed<Interface> Get(KeyType aKey) const;
 
   /**
    * Gets a weak reference to the hashtable entry.
@@ -50,7 +50,7 @@ class nsInterfaceHashtable
    *               to false otherwise.
    * @return The entry, or nullptr if not found. Do not release this pointer!
    */
-  Interface* GetWeak(KeyType aKey, bool* aFound = nullptr) const;
+  [[nodiscard]] Interface* GetWeak(KeyType aKey, bool* aFound = nullptr) const;
 
   /**
    * Allows inserting a value into the hashtable, moving its owning reference

xpcom/ds/nsRefPtrHashtable.h

@@ -33,14 +33,14 @@ class nsRefPtrHashtable
   /**
    * @copydoc nsBaseHashtable::Get
    * @param aData This is an XPCOM getter, so aData is already_addrefed.
-   *   If the key doesn't exist, aData will be set to nullptr.
+   *   If the key doesn't exist, *aData will be set to nullptr.
    */
   bool Get(KeyType aKey, UserDataType* aData) const;
 
   /**
    * @copydoc nsBaseHashtable::Get
    */
-  already_AddRefed<PtrType> Get(KeyType aKey) const;
+  [[nodiscard]] already_AddRefed<PtrType> Get(KeyType aKey) const;
 
   /**
    * Gets a weak reference to the hashtable entry.
@@ -48,7 +48,7 @@ class nsRefPtrHashtable
    *               to false otherwise.
    * @return The entry, or nullptr if not found. Do not release this pointer!
    */
-  PtrType* GetWeak(KeyType aKey, bool* aFound = nullptr) const;
+  [[nodiscard]] PtrType* GetWeak(KeyType aKey, bool* aFound = nullptr) const;
 
   // Hide base class' InsertOrUpdate overloads intentionally, to make any
   // necessary refcounting explicit when calling InsertOrUpdate.