[ADT] Add [[nodiscard]] to more classes (NFC) #161044
Merged
+81
−54
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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/pr-subscribers-llvm-adt Author: Kazu Hirata (kazutakahirata) ChangesThis patch adds [[nodiscard]] to user-facing functions in set/map Full diff: https://github.com/llvm/llvm-project/pull/161044.diff 4 Files Affected:
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
|
kuhar
approved these changes
Sep 28, 2025
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Oct 3, 2025
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.