Skip to content

[ADT] Add [[nodiscard]] to set/map classes (NFC) #160978

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 27, 2025
+144 −126
Merged

[ADT] Add [[nodiscard]] to set/map classes (NFC) #160978

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 47 additions & 34 deletions llvm/include/llvm/ADT/DenseMap.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -153,38 +155,43 @@ 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();
}

/// 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();
Expand All @@ -194,15 +201,16 @@ 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;
}

/// 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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand All @@ -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;

Expand All @@ -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);
}

Expand Down Expand Up @@ -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()) &&
Expand All @@ -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);
}

Expand Down Expand Up @@ -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();
}

Expand Down
38 changes: 24 additions & 14 deletions llvm/include/llvm/ADT/DenseSet.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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));
}

Expand All @@ -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));
}
Expand Down Expand Up @@ -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;

Expand All @@ -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);
}

Expand Down
54 changes: 31 additions & 23 deletions llvm/include/llvm/ADT/MapVector.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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();
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
Loading