[llvm] [ADT] Add [[nodiscard]] to set/map classes (NFC) (PR #160978)
via llvm-commits
llvm-commits at lists.llvm.org
Fri Sep 26 22:31:13 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 are:
- const and return non-void values (e.g., size()), or
- non-const and have no side effects (e.g., find()).
---
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:
- (modified) llvm/include/llvm/ADT/DenseMap.h (+47-34)
- (modified) llvm/include/llvm/ADT/DenseSet.h (+24-14)
- (modified) llvm/include/llvm/ADT/MapVector.h (+31-23)
- (modified) llvm/include/llvm/ADT/SetVector.h (+16-34)
- (modified) llvm/include/llvm/ADT/SmallPtrSet.h (+12-12)
- (modified) llvm/include/llvm/ADT/SmallSet.h (+11-7)
- (modified) llvm/unittests/ADT/APIntTest.cpp (+3-2)
``````````diff
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]
``````````
</details>
https://github.com/llvm/llvm-project/pull/160978
More information about the llvm-commits
mailing list