From 127c7b35148caf38167fd9a5932f790ee04d462d Mon Sep 17 00:00:00 2001 From: Kazu Hirata Date: Fri, 26 Sep 2025 22:42:12 -0700 Subject: [PATCH] [ADT] Add [[nodiscard]] to more classes (NFC) This patch adds [[nodiscard]] to user-facing functions in set/map classes if they return non-void values and appear to have no side effect. --- llvm/include/llvm/ADT/ImmutableMap.h | 44 +++++++++++--------- llvm/include/llvm/ADT/SparseSet.h | 26 +++++++----- llvm/include/llvm/ADT/StringMap.h | 61 +++++++++++++++++----------- llvm/include/llvm/ADT/StringSet.h | 4 +- 4 files changed, 81 insertions(+), 54 deletions(-) diff --git a/llvm/include/llvm/ADT/ImmutableMap.h b/llvm/include/llvm/ADT/ImmutableMap.h index 3d19ca41a5be0..32634a96ee9ea 100644 --- a/llvm/include/llvm/ADT/ImmutableMap.h +++ b/llvm/include/llvm/ADT/ImmutableMap.h @@ -111,25 +111,25 @@ class ImmutableMap { } }; - bool contains(key_type_ref K) const { + [[nodiscard]] bool contains(key_type_ref K) const { return Root ? Root->contains(K) : false; } - bool operator==(const ImmutableMap &RHS) const { + [[nodiscard]] bool operator==(const ImmutableMap &RHS) const { return Root && RHS.Root ? Root->isEqual(*RHS.Root.get()) : Root == RHS.Root; } - bool operator!=(const ImmutableMap &RHS) const { + [[nodiscard]] bool operator!=(const ImmutableMap &RHS) const { return Root && RHS.Root ? Root->isNotEqual(*RHS.Root.get()) : Root != RHS.Root; } - TreeTy *getRoot() const { + [[nodiscard]] TreeTy *getRoot() const { if (Root) { Root->retain(); } return Root.get(); } - TreeTy *getRootWithoutRetain() const { return Root.get(); } + [[nodiscard]] TreeTy *getRootWithoutRetain() const { return Root.get(); } void manualRetain() { if (Root) Root->retain(); @@ -139,7 +139,7 @@ class ImmutableMap { if (Root) Root->release(); } - bool isEmpty() const { return !Root; } + [[nodiscard]] bool isEmpty() const { return !Root; } public: //===--------------------------------------------------===// @@ -163,10 +163,10 @@ class ImmutableMap { data_type_ref getData() const { return (*this)->second; } }; - iterator begin() const { return iterator(Root.get()); } - iterator end() const { return iterator(); } + [[nodiscard]] iterator begin() const { return iterator(Root.get()); } + [[nodiscard]] iterator end() const { return iterator(); } - data_type* lookup(key_type_ref K) const { + [[nodiscard]] data_type *lookup(key_type_ref K) const { if (Root) { TreeTy* T = Root->find(K); if (T) return &T->getValue().second; @@ -178,7 +178,7 @@ class ImmutableMap { /// getMaxElement - Returns the pair in the ImmutableMap for /// which key is the highest in the ordering of keys in the map. This /// method returns NULL if the map is empty. - value_type* getMaxElement() const { + [[nodiscard]] value_type *getMaxElement() const { return Root ? &(Root->getMaxElement()->getValue()) : nullptr; } @@ -186,7 +186,9 @@ class ImmutableMap { // Utility methods. //===--------------------------------------------------===// - unsigned getHeight() const { return Root ? Root->getHeight() : 0; } + [[nodiscard]] unsigned getHeight() const { + return Root ? Root->getHeight() : 0; + } static inline void Profile(FoldingSetNodeID& ID, const ImmutableMap& M) { ID.AddPointer(M.Root.get()); @@ -250,7 +252,7 @@ class ImmutableMapRef { return ImmutableMapRef(NewT, Factory); } - bool contains(key_type_ref K) const { + [[nodiscard]] bool contains(key_type_ref K) const { return Root ? Root->contains(K) : false; } @@ -258,16 +260,16 @@ class ImmutableMapRef { return ImmutableMap(Factory->getCanonicalTree(Root.get())); } - bool operator==(const ImmutableMapRef &RHS) const { + [[nodiscard]] bool operator==(const ImmutableMapRef &RHS) const { return Root && RHS.Root ? Root->isEqual(*RHS.Root.get()) : Root == RHS.Root; } - bool operator!=(const ImmutableMapRef &RHS) const { + [[nodiscard]] bool operator!=(const ImmutableMapRef &RHS) const { return Root && RHS.Root ? Root->isNotEqual(*RHS.Root.get()) : Root != RHS.Root; } - bool isEmpty() const { return !Root; } + [[nodiscard]] bool isEmpty() const { return !Root; } //===--------------------------------------------------===// // For testing. @@ -293,10 +295,10 @@ class ImmutableMapRef { data_type_ref getData() const { return (*this)->second; } }; - iterator begin() const { return iterator(Root.get()); } - iterator end() const { return iterator(); } + [[nodiscard]] iterator begin() const { return iterator(Root.get()); } + [[nodiscard]] iterator end() const { return iterator(); } - data_type *lookup(key_type_ref K) const { + [[nodiscard]] data_type *lookup(key_type_ref K) const { if (Root) { TreeTy* T = Root->find(K); if (T) return &T->getValue().second; @@ -308,7 +310,7 @@ class ImmutableMapRef { /// getMaxElement - Returns the pair in the ImmutableMap for /// which key is the highest in the ordering of keys in the map. This /// method returns NULL if the map is empty. - value_type* getMaxElement() const { + [[nodiscard]] value_type *getMaxElement() const { return Root ? &(Root->getMaxElement()->getValue()) : nullptr; } @@ -316,7 +318,9 @@ class ImmutableMapRef { // Utility methods. //===--------------------------------------------------===// - unsigned getHeight() const { return Root ? Root->getHeight() : 0; } + [[nodiscard]] unsigned getHeight() const { + return Root ? Root->getHeight() : 0; + } static inline void Profile(FoldingSetNodeID &ID, const ImmutableMapRef &M) { ID.AddPointer(M.Root.get()); diff --git a/llvm/include/llvm/ADT/SparseSet.h b/llvm/include/llvm/ADT/SparseSet.h index 395cfc3ebfd43..9783301be4b64 100644 --- a/llvm/include/llvm/ADT/SparseSet.h +++ b/llvm/include/llvm/ADT/SparseSet.h @@ -171,23 +171,23 @@ class SparseSet { using iterator = typename DenseT::iterator; using const_iterator = typename DenseT::const_iterator; - const_iterator begin() const { return Dense.begin(); } - const_iterator end() const { return Dense.end(); } - iterator begin() { return Dense.begin(); } - iterator end() { return Dense.end(); } + [[nodiscard]] const_iterator begin() const { return Dense.begin(); } + [[nodiscard]] const_iterator end() const { return Dense.end(); } + [[nodiscard]] iterator begin() { return Dense.begin(); } + [[nodiscard]] iterator end() { return Dense.end(); } /// empty - Returns true if the set is empty. /// /// This is not the same as BitVector::empty(). /// - bool empty() const { return Dense.empty(); } + [[nodiscard]] bool empty() const { return Dense.empty(); } /// size - Returns the number of elements in the set. /// /// This is not the same as BitVector::size() which returns the size of the /// universe. /// - size_type size() const { return Dense.size(); } + [[nodiscard]] size_type size() const { return Dense.size(); } /// clear - Clears the set. This is a very fast constant time operation. /// @@ -222,21 +222,27 @@ class SparseSet { /// @param Key A valid key to find. /// @returns An iterator to the element identified by key, or end(). /// - iterator find(const KeyT &Key) { return findIndex(KeyIndexOf(Key)); } + [[nodiscard]] iterator find(const KeyT &Key) { + return findIndex(KeyIndexOf(Key)); + } - const_iterator find(const KeyT &Key) const { + [[nodiscard]] const_iterator find(const KeyT &Key) const { return const_cast(this)->findIndex(KeyIndexOf(Key)); } /// Check if the set contains the given \c Key. /// /// @param Key A valid key to find. - bool contains(const KeyT &Key) const { return find(Key) != end(); } + [[nodiscard]] bool contains(const KeyT &Key) const { + return find(Key) != end(); + } /// count - Returns 1 if this set contains an element identified by Key, /// 0 otherwise. /// - size_type count(const KeyT &Key) const { return contains(Key) ? 1 : 0; } + [[nodiscard]] size_type count(const KeyT &Key) const { + return contains(Key) ? 1 : 0; + } /// insert - Attempts to insert a new element. /// diff --git a/llvm/include/llvm/ADT/StringMap.h b/llvm/include/llvm/ADT/StringMap.h index 2c146fbf08df1..01cbf2d3fff71 100644 --- a/llvm/include/llvm/ADT/StringMap.h +++ b/llvm/include/llvm/ADT/StringMap.h @@ -102,18 +102,18 @@ class StringMapImpl { return reinterpret_cast(TombstoneIntVal); } - unsigned getNumBuckets() const { return NumBuckets; } - unsigned getNumItems() const { return NumItems; } + [[nodiscard]] unsigned getNumBuckets() const { return NumBuckets; } + [[nodiscard]] unsigned getNumItems() const { return NumItems; } - bool empty() const { return NumItems == 0; } - unsigned size() const { return NumItems; } + [[nodiscard]] bool empty() const { return NumItems == 0; } + [[nodiscard]] unsigned size() const { return NumItems; } /// Returns the hash value that will be used for the given string. /// This allows precomputing the value and passing it explicitly /// to some of the functions. /// The implementation of this function is not guaranteed to be stable /// and may change. - LLVM_ABI static uint32_t hash(StringRef Key); + [[nodiscard]] LLVM_ABI static uint32_t hash(StringRef Key); void swap(StringMapImpl &Other) { std::swap(TheTable, Other.TheTable); @@ -220,30 +220,35 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap using const_iterator = StringMapIterBase; using iterator = StringMapIterBase; - iterator begin() { return iterator(TheTable, NumBuckets != 0); } - iterator end() { return iterator(TheTable + NumBuckets); } - const_iterator begin() const { + [[nodiscard]] iterator begin() { return iterator(TheTable, NumBuckets != 0); } + [[nodiscard]] iterator end() { return iterator(TheTable + NumBuckets); } + [[nodiscard]] const_iterator begin() const { return const_iterator(TheTable, NumBuckets != 0); } - const_iterator end() const { return const_iterator(TheTable + NumBuckets); } + [[nodiscard]] const_iterator end() const { + return const_iterator(TheTable + NumBuckets); + } - iterator_range> keys() const { + [[nodiscard]] iterator_range> keys() const { return make_range(StringMapKeyIterator(begin()), StringMapKeyIterator(end())); } - iterator find(StringRef Key) { return find(Key, hash(Key)); } + [[nodiscard]] iterator find(StringRef Key) { return find(Key, hash(Key)); } - iterator find(StringRef Key, uint32_t FullHashValue) { + [[nodiscard]] iterator find(StringRef Key, uint32_t FullHashValue) { int Bucket = FindKey(Key, FullHashValue); if (Bucket == -1) return end(); return iterator(TheTable + Bucket); } - const_iterator find(StringRef Key) const { return find(Key, hash(Key)); } + [[nodiscard]] const_iterator find(StringRef Key) const { + return find(Key, hash(Key)); + } - const_iterator find(StringRef Key, uint32_t FullHashValue) const { + [[nodiscard]] const_iterator find(StringRef Key, + uint32_t FullHashValue) const { int Bucket = FindKey(Key, FullHashValue); if (Bucket == -1) return end(); @@ -252,7 +257,7 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap /// lookup - Return the entry for the specified key, or a default /// constructed value if no such entry exists. - ValueTy lookup(StringRef Key) const { + [[nodiscard]] ValueTy lookup(StringRef Key) const { const_iterator Iter = find(Key); if (Iter != end()) return Iter->second; @@ -261,7 +266,7 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap /// at - Return the entry for the specified key, or abort if no such /// entry exists. - const ValueTy &at(StringRef Val) const { + [[nodiscard]] const ValueTy &at(StringRef Val) const { auto Iter = this->find(Val); assert(Iter != this->end() && "StringMap::at failed due to a missing key"); return Iter->second; @@ -272,18 +277,22 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap ValueTy &operator[](StringRef Key) { return try_emplace(Key).first->second; } /// contains - Return true if the element is in the map, false otherwise. - bool contains(StringRef Key) const { return find(Key) != end(); } + [[nodiscard]] bool contains(StringRef Key) const { + return find(Key) != end(); + } /// count - Return 1 if the element is in the map, 0 otherwise. - size_type count(StringRef Key) const { return contains(Key) ? 1 : 0; } + [[nodiscard]] size_type count(StringRef Key) const { + return contains(Key) ? 1 : 0; + } template - size_type count(const StringMapEntry &MapEntry) const { + [[nodiscard]] size_type count(const StringMapEntry &MapEntry) const { return count(MapEntry.getKey()); } /// equal - check whether both of the containers are equal. - bool operator==(const StringMap &RHS) const { + [[nodiscard]] bool operator==(const StringMap &RHS) const { if (size() != RHS.size()) return false; @@ -302,7 +311,9 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap return true; } - bool operator!=(const StringMap &RHS) const { return !(*this == RHS); } + [[nodiscard]] bool operator!=(const StringMap &RHS) const { + return !(*this == RHS); + } /// insert - Insert the specified key/value pair into the map. If the key /// already exists in the map, return false and ignore the request, otherwise @@ -447,8 +458,12 @@ template class StringMapIterBase { AdvancePastEmptyBuckets(); } - reference operator*() const { return *static_cast(*Ptr); } - pointer operator->() const { return static_cast(*Ptr); } + [[nodiscard]] reference operator*() const { + return *static_cast(*Ptr); + } + [[nodiscard]] pointer operator->() const { + return static_cast(*Ptr); + } StringMapIterBase &operator++() { // Preincrement ++Ptr; diff --git a/llvm/include/llvm/ADT/StringSet.h b/llvm/include/llvm/ADT/StringSet.h index b4853423a1ef3..c8be3f2a503e4 100644 --- a/llvm/include/llvm/ADT/StringSet.h +++ b/llvm/include/llvm/ADT/StringSet.h @@ -57,7 +57,9 @@ class StringSet : public StringMap { } /// Check if the set contains the given \c key. - bool contains(StringRef key) const { return Base::contains(key); } + [[nodiscard]] bool contains(StringRef key) const { + return Base::contains(key); + } }; } // end namespace llvm