[ADT] Add [[nodiscard]] to set/map classes (NFC) #160978
Merged
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 are: - const and return non-void values (e.g., size()), or - non-const and have no side effects (e.g., find()).
|
@llvm/pr-subscribers-llvm-adt Author: Kazu Hirata (kazutakahirata) ChangesThis patch adds [[nodiscard]] to user-facing functions in set/map
Patch is 26.26 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/160978.diff 7 Files Affected:
diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index 5f716751751c4..1040f5c7a7aa5 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -75,37 +75,39 @@ class DenseMapBase : public DebugEpochBase {
using const_iterator =
DenseMapIterator<KeyT, ValueT, KeyInfoT, BucketT, true>;
- inline iterator begin() {
+ [[nodiscard]] inline iterator begin() {
return iterator::makeBegin(buckets(), empty(), *this);
}
- inline iterator end() { return iterator::makeEnd(buckets(), *this); }
- inline const_iterator begin() const {
+ [[nodiscard]] inline iterator end() {
+ return iterator::makeEnd(buckets(), *this);
+ }
+ [[nodiscard]] inline const_iterator begin() const {
return const_iterator::makeBegin(buckets(), empty(), *this);
}
- inline const_iterator end() const {
+ [[nodiscard]] inline const_iterator end() const {
return const_iterator::makeEnd(buckets(), *this);
}
// Return an iterator to iterate over keys in the map.
- inline auto keys() {
+ [[nodiscard]] inline auto keys() {
return map_range(*this, [](const BucketT &P) { return P.getFirst(); });
}
// Return an iterator to iterate over values in the map.
- inline auto values() {
+ [[nodiscard]] inline auto values() {
return map_range(*this, [](const BucketT &P) { return P.getSecond(); });
}
- inline auto keys() const {
+ [[nodiscard]] inline auto keys() const {
return map_range(*this, [](const BucketT &P) { return P.getFirst(); });
}
- inline auto values() const {
+ [[nodiscard]] inline auto values() const {
return map_range(*this, [](const BucketT &P) { return P.getSecond(); });
}
[[nodiscard]] bool empty() const { return getNumEntries() == 0; }
- unsigned size() const { return getNumEntries(); }
+ [[nodiscard]] unsigned size() const { return getNumEntries(); }
/// Grow the densemap so that it can contain at least \p NumEntries items
/// before resizing again.
@@ -153,30 +155,35 @@ class DenseMapBase : public DebugEpochBase {
}
/// Return true if the specified key is in the map, false otherwise.
- bool contains(const_arg_type_t<KeyT> Val) const {
+ [[nodiscard]] bool contains(const_arg_type_t<KeyT> Val) const {
return doFind(Val) != nullptr;
}
/// Return 1 if the specified key is in the map, 0 otherwise.
- size_type count(const_arg_type_t<KeyT> Val) const {
+ [[nodiscard]] size_type count(const_arg_type_t<KeyT> Val) const {
return contains(Val) ? 1 : 0;
}
- iterator find(const_arg_type_t<KeyT> Val) { return find_as(Val); }
- const_iterator find(const_arg_type_t<KeyT> Val) const { return find_as(Val); }
+ [[nodiscard]] iterator find(const_arg_type_t<KeyT> Val) {
+ return find_as(Val);
+ }
+ [[nodiscard]] const_iterator find(const_arg_type_t<KeyT> Val) const {
+ return find_as(Val);
+ }
/// Alternate version of find() which allows a different, and possibly
/// less expensive, key type.
/// The DenseMapInfo is responsible for supplying methods
/// getHashValue(LookupKeyT) and isEqual(LookupKeyT, KeyT) for each key
/// type used.
- template <class LookupKeyT> iterator find_as(const LookupKeyT &Val) {
+ template <class LookupKeyT>
+ [[nodiscard]] iterator find_as(const LookupKeyT &Val) {
if (BucketT *Bucket = doFind(Val))
return makeIterator(Bucket);
return end();
}
template <class LookupKeyT>
- const_iterator find_as(const LookupKeyT &Val) const {
+ [[nodiscard]] const_iterator find_as(const LookupKeyT &Val) const {
if (const BucketT *Bucket = doFind(Val))
return makeConstIterator(Bucket);
return end();
@@ -184,7 +191,7 @@ class DenseMapBase : public DebugEpochBase {
/// lookup - Return the entry for the specified key, or a default
/// constructed value if no such entry exists.
- ValueT lookup(const_arg_type_t<KeyT> Val) const {
+ [[nodiscard]] ValueT lookup(const_arg_type_t<KeyT> Val) const {
if (const BucketT *Bucket = doFind(Val))
return Bucket->getSecond();
return ValueT();
@@ -194,7 +201,8 @@ class DenseMapBase : public DebugEpochBase {
// useful, because `lookup` cannot be used with non-default-constructible
// values.
template <typename U = std::remove_cv_t<ValueT>>
- ValueT lookup_or(const_arg_type_t<KeyT> Val, U &&Default) const {
+ [[nodiscard]] ValueT lookup_or(const_arg_type_t<KeyT> Val,
+ U &&Default) const {
if (const BucketT *Bucket = doFind(Val))
return Bucket->getSecond();
return Default;
@@ -202,7 +210,7 @@ class DenseMapBase : public DebugEpochBase {
/// at - Return the entry for the specified key, or abort if no such
/// entry exists.
- const ValueT &at(const_arg_type_t<KeyT> Val) const {
+ [[nodiscard]] const ValueT &at(const_arg_type_t<KeyT> Val) const {
auto Iter = this->find(std::move(Val));
assert(Iter != this->end() && "DenseMap::at failed due to a missing key");
return Iter->second;
@@ -330,14 +338,16 @@ class DenseMapBase : public DebugEpochBase {
/// isPointerIntoBucketsArray - Return true if the specified pointer points
/// somewhere into the DenseMap's array of buckets (i.e. either to a key or
/// value in the DenseMap).
- bool isPointerIntoBucketsArray(const void *Ptr) const {
+ [[nodiscard]] bool isPointerIntoBucketsArray(const void *Ptr) const {
return Ptr >= getBuckets() && Ptr < getBucketsEnd();
}
/// getPointerIntoBucketsArray() - Return an opaque pointer into the buckets
/// array. In conjunction with the previous method, this can be used to
/// determine whether an insertion caused the DenseMap to reallocate.
- const void *getPointerIntoBucketsArray() const { return getBuckets(); }
+ [[nodiscard]] const void *getPointerIntoBucketsArray() const {
+ return getBuckets();
+ }
protected:
DenseMapBase() = default;
@@ -656,7 +666,9 @@ class DenseMapBase : public DebugEpochBase {
/// This is just the raw memory used by DenseMap.
/// If entries are pointers to objects, the size of the referenced objects
/// are not included.
- size_t getMemorySize() const { return getNumBuckets() * sizeof(BucketT); }
+ [[nodiscard]] size_t getMemorySize() const {
+ return getNumBuckets() * sizeof(BucketT);
+ }
};
/// Equality comparison for DenseMap.
@@ -667,9 +679,9 @@ class DenseMapBase : public DebugEpochBase {
/// complexity is linear, worst case is O(N^2) (if every hash collides).
template <typename DerivedT, typename KeyT, typename ValueT, typename KeyInfoT,
typename BucketT>
-bool operator==(
- const DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT> &LHS,
- const DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT> &RHS) {
+[[nodiscard]] bool
+operator==(const DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT> &LHS,
+ const DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT> &RHS) {
if (LHS.size() != RHS.size())
return false;
@@ -687,9 +699,9 @@ bool operator==(
/// Equivalent to !(LHS == RHS). See operator== for performance notes.
template <typename DerivedT, typename KeyT, typename ValueT, typename KeyInfoT,
typename BucketT>
-bool operator!=(
- const DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT> &LHS,
- const DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT> &RHS) {
+[[nodiscard]] bool
+operator!=(const DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT> &LHS,
+ const DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT> &RHS) {
return !(LHS == RHS);
}
@@ -1227,15 +1239,15 @@ class DenseMapIterator : DebugEpochBase::HandleBase {
const DenseMapIterator<KeyT, ValueT, KeyInfoT, Bucket, IsConstSrc> &I)
: DebugEpochBase::HandleBase(I), Ptr(I.Ptr), End(I.End) {}
- reference operator*() const {
+ [[nodiscard]] reference operator*() const {
assert(isHandleInSync() && "invalid iterator access!");
assert(Ptr != End && "dereferencing end() iterator");
return *Ptr;
}
- pointer operator->() const { return &operator*(); }
+ [[nodiscard]] pointer operator->() const { return &operator*(); }
- friend bool operator==(const DenseMapIterator &LHS,
- const DenseMapIterator &RHS) {
+ [[nodiscard]] friend bool operator==(const DenseMapIterator &LHS,
+ const DenseMapIterator &RHS) {
assert((!LHS.getEpochAddress() || LHS.isHandleInSync()) &&
"handle not in sync!");
assert((!RHS.getEpochAddress() || RHS.isHandleInSync()) &&
@@ -1245,8 +1257,8 @@ class DenseMapIterator : DebugEpochBase::HandleBase {
return LHS.Ptr == RHS.Ptr;
}
- friend bool operator!=(const DenseMapIterator &LHS,
- const DenseMapIterator &RHS) {
+ [[nodiscard]] friend bool operator!=(const DenseMapIterator &LHS,
+ const DenseMapIterator &RHS) {
return !(LHS == RHS);
}
@@ -1284,7 +1296,8 @@ class DenseMapIterator : DebugEpochBase::HandleBase {
};
template <typename KeyT, typename ValueT, typename KeyInfoT>
-inline size_t capacity_in_bytes(const DenseMap<KeyT, ValueT, KeyInfoT> &X) {
+[[nodiscard]] inline size_t
+capacity_in_bytes(const DenseMap<KeyT, ValueT, KeyInfoT> &X) {
return X.getMemorySize();
}
diff --git a/llvm/include/llvm/ADT/DenseSet.h b/llvm/include/llvm/ADT/DenseSet.h
index 60ad9b2eb7762..eec800d07b6df 100644
--- a/llvm/include/llvm/ADT/DenseSet.h
+++ b/llvm/include/llvm/ADT/DenseSet.h
@@ -83,9 +83,9 @@ class DenseSetImpl {
DenseSetImpl(llvm::from_range_t, Range &&R)
: DenseSetImpl(adl_begin(R), adl_end(R)) {}
- bool empty() const { return TheMap.empty(); }
- size_type size() const { return TheMap.size(); }
- size_t getMemorySize() const { return TheMap.getMemorySize(); }
+ [[nodiscard]] bool empty() const { return TheMap.empty(); }
+ [[nodiscard]] size_type size() const { return TheMap.size(); }
+ [[nodiscard]] size_t getMemorySize() const { return TheMap.getMemorySize(); }
/// Grow the DenseSet so that it has at least Size buckets. Will not shrink
/// the Size of the set.
@@ -154,14 +154,20 @@ class DenseSetImpl {
using iterator = DenseSetIterator<false>;
using const_iterator = DenseSetIterator<true>;
- iterator begin() { return iterator(TheMap.begin()); }
- iterator end() { return iterator(TheMap.end()); }
+ [[nodiscard]] iterator begin() { return iterator(TheMap.begin()); }
+ [[nodiscard]] iterator end() { return iterator(TheMap.end()); }
- const_iterator begin() const { return const_iterator(TheMap.begin()); }
- const_iterator end() const { return const_iterator(TheMap.end()); }
+ [[nodiscard]] const_iterator begin() const {
+ return const_iterator(TheMap.begin());
+ }
+ [[nodiscard]] const_iterator end() const {
+ return const_iterator(TheMap.end());
+ }
- iterator find(const_arg_type_t<ValueT> V) { return iterator(TheMap.find(V)); }
- const_iterator find(const_arg_type_t<ValueT> V) const {
+ [[nodiscard]] iterator find(const_arg_type_t<ValueT> V) {
+ return iterator(TheMap.find(V));
+ }
+ [[nodiscard]] const_iterator find(const_arg_type_t<ValueT> V) const {
return const_iterator(TheMap.find(V));
}
@@ -180,10 +186,12 @@ class DenseSetImpl {
/// The DenseMapInfo is responsible for supplying methods
/// getHashValue(LookupKeyT) and isEqual(LookupKeyT, KeyT) for each key type
/// used.
- template <class LookupKeyT> iterator find_as(const LookupKeyT &Val) {
+ template <class LookupKeyT>
+ [[nodiscard]] iterator find_as(const LookupKeyT &Val) {
return iterator(TheMap.find_as(Val));
}
template <class LookupKeyT>
+ [[nodiscard]]
const_iterator find_as(const LookupKeyT &Val) const {
return const_iterator(TheMap.find_as(Val));
}
@@ -229,8 +237,9 @@ class DenseSetImpl {
/// Equivalent to N calls to RHS.count. Amortized complexity is linear, worst
/// case is O(N^2) (if every hash collides).
template <typename ValueT, typename MapTy, typename ValueInfoT>
-bool operator==(const DenseSetImpl<ValueT, MapTy, ValueInfoT> &LHS,
- const DenseSetImpl<ValueT, MapTy, ValueInfoT> &RHS) {
+[[nodiscard]] bool
+operator==(const DenseSetImpl<ValueT, MapTy, ValueInfoT> &LHS,
+ const DenseSetImpl<ValueT, MapTy, ValueInfoT> &RHS) {
if (LHS.size() != RHS.size())
return false;
@@ -245,8 +254,9 @@ bool operator==(const DenseSetImpl<ValueT, MapTy, ValueInfoT> &LHS,
///
/// Equivalent to !(LHS == RHS). See operator== for performance notes.
template <typename ValueT, typename MapTy, typename ValueInfoT>
-bool operator!=(const DenseSetImpl<ValueT, MapTy, ValueInfoT> &LHS,
- const DenseSetImpl<ValueT, MapTy, ValueInfoT> &RHS) {
+[[nodiscard]] bool
+operator!=(const DenseSetImpl<ValueT, MapTy, ValueInfoT> &LHS,
+ const DenseSetImpl<ValueT, MapTy, ValueInfoT> &RHS) {
return !(LHS == RHS);
}
diff --git a/llvm/include/llvm/ADT/MapVector.h b/llvm/include/llvm/ADT/MapVector.h
index 4a50126ff5aad..82f2c4977e01d 100644
--- a/llvm/include/llvm/ADT/MapVector.h
+++ b/llvm/include/llvm/ADT/MapVector.h
@@ -45,15 +45,15 @@ class MapVector {
using const_reverse_iterator = typename VectorType::const_reverse_iterator;
/// Clear the MapVector and return the underlying vector.
- VectorType takeVector() {
+ [[nodiscard]] VectorType takeVector() {
Map.clear();
return std::move(Vector);
}
/// Returns an array reference of the underlying vector.
- ArrayRef<value_type> getArrayRef() const { return Vector; }
+ [[nodiscard]] ArrayRef<value_type> getArrayRef() const { return Vector; }
- size_type size() const { return Vector.size(); }
+ [[nodiscard]] size_type size() const { return Vector.size(); }
/// Grow the MapVector so that it can contain at least \p NumEntries items
/// before resizing again.
@@ -62,24 +62,28 @@ class MapVector {
Vector.reserve(NumEntries);
}
- iterator begin() { return Vector.begin(); }
- const_iterator begin() const { return Vector.begin(); }
- iterator end() { return Vector.end(); }
- const_iterator end() const { return Vector.end(); }
+ [[nodiscard]] iterator begin() { return Vector.begin(); }
+ [[nodiscard]] const_iterator begin() const { return Vector.begin(); }
+ [[nodiscard]] iterator end() { return Vector.end(); }
+ [[nodiscard]] const_iterator end() const { return Vector.end(); }
- reverse_iterator rbegin() { return Vector.rbegin(); }
- const_reverse_iterator rbegin() const { return Vector.rbegin(); }
- reverse_iterator rend() { return Vector.rend(); }
- const_reverse_iterator rend() const { return Vector.rend(); }
-
- bool empty() const {
- return Vector.empty();
+ [[nodiscard]] reverse_iterator rbegin() { return Vector.rbegin(); }
+ [[nodiscard]] const_reverse_iterator rbegin() const {
+ return Vector.rbegin();
}
+ [[nodiscard]] reverse_iterator rend() { return Vector.rend(); }
+ [[nodiscard]] const_reverse_iterator rend() const { return Vector.rend(); }
+
+ [[nodiscard]] bool empty() const { return Vector.empty(); }
- std::pair<KeyT, ValueT> &front() { return Vector.front(); }
- const std::pair<KeyT, ValueT> &front() const { return Vector.front(); }
- std::pair<KeyT, ValueT> &back() { return Vector.back(); }
- const std::pair<KeyT, ValueT> &back() const { return Vector.back(); }
+ [[nodiscard]] std::pair<KeyT, ValueT> &front() { return Vector.front(); }
+ [[nodiscard]] const std::pair<KeyT, ValueT> &front() const {
+ return Vector.front();
+ }
+ [[nodiscard]] std::pair<KeyT, ValueT> &back() { return Vector.back(); }
+ [[nodiscard]] const std::pair<KeyT, ValueT> &back() const {
+ return Vector.back();
+ }
void clear() {
Map.clear();
@@ -96,7 +100,7 @@ class MapVector {
}
// Returns a copy of the value. Only allowed if ValueT is copyable.
- ValueT lookup(const KeyT &Key) const {
+ [[nodiscard]] ValueT lookup(const KeyT &Key) const {
static_assert(std::is_copy_constructible_v<ValueT>,
"Cannot call lookup() if ValueT is not copyable.");
typename MapType::const_iterator Pos = Map.find(Key);
@@ -134,17 +138,21 @@ class MapVector {
return Ret;
}
- bool contains(const KeyT &Key) const { return Map.find(Key) != Map.end(); }
+ [[nodiscard]] bool contains(const KeyT &Key) const {
+ return Map.find(Key) != Map.end();
+ }
- 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;
+ }
- iterator find(const KeyT &Key) {
+ [[nodiscard]] iterator find(const KeyT &Key) {
typename MapType::const_iterator Pos = Map.find(Key);
return Pos == Map.end()? Vector.end() :
(Vector.begin() + Pos->second);
}
- const_iterator find(const KeyT &Key) const {
+ [[nodiscard]] const_iterator find(const KeyT &Key) const {
typename MapType::const_iterator Pos = Map.find(Key);
return Pos == Map.end()? Vector.end() :
(Vector.begin() + Pos->second);
diff --git a/llvm/include/llvm/ADT/SetVector.h b/llvm/include/llvm/ADT/SetVector.h
index 5f6db9a78a003..c129f3a695b9e 100644
--- a/llvm/include/llvm/ADT/SetVector.h
+++ b/llvm/include/llvm/ADT/SetVector.h
@@ -87,72 +87,54 @@ class SetVector {
SetVector(llvm::from_range_t, Range &&R)
: SetVector(adl_begin(R), adl_end(R)) {}
- ArrayRef<value_type> getArrayRef() const { return vector_; }
+ [[nodiscard]] ArrayRef<value_type> getArrayRef() const { return vector_; }
/// Clear the SetVector and return the underlying vector.
- Vector takeVector() {
+ [[nodiscard]] Vector takeVector() {
set_.clear();
return std::move(vector_);
}
/// Determine if the SetVector is empty or not.
- bool empty() const {
- return vector_.empty();
- }
+ [[nodiscard]] bool empty() const { return vector_.empty(); }
/// Determine the number of elements in the SetVector.
- size_type size() const {
- return vector_.size();
- }
+ [[nodiscard]] size_type size() const { return vector_.size(); }
/// Get an iterator to the beginning of the SetVector.
- iterator begin() {
- return vector_.begin();
- }
+ [[nodiscard]] iterator begin() { return vector_.begin(); }
/// Get a const_iterator to the beginning of the SetVector.
- const_iterator begin() const {
- return vector_.begin();
- }
+ [[nodiscard]] const_iterator begin() const { return vector_.begin(); }
/// Get an iterator to the end of the SetVector.
- iterator end() {
- return vector_.end();
- }
+ [[nodiscard]] iterator end() { return vector_.end(); }
/// Get a const_iterator to the end of the SetVector.
- const_iterator end() const {
- return vector_.end();
- }
+ [[nodiscard]] const_iterator end() const { return vector_.end(); }
/// Get an reverse_iterator to the end of the SetVector.
- reverse_iterator rbegin() {
- return vector_.rbegin();
- }
+ [[nodiscard]] reverse_iterator rbegin() { return vector_.rbegin(); }
/// Get a const_reverse_iterator to the end of the SetVector.
- const_reverse_iterator rbegin() const {
+ [[nodiscard]] const_reverse_iterator rbegin() const {
return vector_.rbegin();
}
/// Get a reverse_iterator to the beginning of the SetVector.
- reverse_iterator rend() {
- return vector_.rend();
- }
+ [[nodiscard]] reverse_iterator rend() { return vector_.rend(); }
/// Get a const_reverse_iterator to the beginning of the SetVector.
- const_reverse_iterator rend() const {
- return vector_.rend();
- }
+ [[nodiscard]] const_reverse_iterator rend() const { return vector_.rend(); }
/// Return the first element of the SetVector.
- const value_type &front() const {
+ [[nodiscard]] const value_type &front() const {
assert(!empty() && "Cannot call front() on empty SetVector!");
return vector_.front();
}
/// Return the last element of the SetVector.
- const value_type &back() const {
+ [[nodiscard]] const value_type &back() const {
assert(!empty() && "Cannot call back() on empty SetVector!");
return vector_.back();
}
@@ -299,11 +281,11 @@ class SetVector {
return Ret;
}
- bool operator==(const SetVector &that) const {
+ [[nodiscard]] bool operator==(const SetVector &that) const {
return vector_ == that.vector_;
}
- bool...
[truncated]
|
kuhar
approved these changes
Sep 27, 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 are: - const and return non-void values (e.g., size()), or - non-const and have no side effects (e.g., find()).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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 are: