[llvm] [ADT] Add [[nodiscard]] to more classes (NFC) (PR #161044)
via llvm-commits
llvm-commits at lists.llvm.org
Sat Sep 27 22:50:17 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-adt
Author: Kazu Hirata (kazutakahirata)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/161044.diff
4 Files Affected:
- (modified) llvm/include/llvm/ADT/ImmutableMap.h (+24-20)
- (modified) llvm/include/llvm/ADT/SparseSet.h (+16-10)
- (modified) llvm/include/llvm/ADT/StringMap.h (+38-23)
- (modified) llvm/include/llvm/ADT/StringSet.h (+3-1)
``````````diff
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 <key,value> 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<KeyT, ValT>(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 <key,value> 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<SparseSet *>(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<StringMapEntryBase *>(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<ValueTy, true>;
using iterator = StringMapIterBase<ValueTy, false>;
- 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<StringMapKeyIterator<ValueTy>> keys() const {
+ [[nodiscard]] iterator_range<StringMapKeyIterator<ValueTy>> keys() const {
return make_range(StringMapKeyIterator<ValueTy>(begin()),
StringMapKeyIterator<ValueTy>(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 <typename InputTy>
- size_type count(const StringMapEntry<InputTy> &MapEntry) const {
+ [[nodiscard]] size_type count(const StringMapEntry<InputTy> &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 <typename ValueTy, bool IsConst> class StringMapIterBase {
AdvancePastEmptyBuckets();
}
- reference operator*() const { return *static_cast<value_type *>(*Ptr); }
- pointer operator->() const { return static_cast<value_type *>(*Ptr); }
+ [[nodiscard]] reference operator*() const {
+ return *static_cast<value_type *>(*Ptr);
+ }
+ [[nodiscard]] pointer operator->() const {
+ return static_cast<value_type *>(*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<std::nullopt_t, AllocatorTy> {
}
/// 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
``````````
</details>
https://github.com/llvm/llvm-project/pull/161044
More information about the llvm-commits
mailing list