Skip to content

Commit 5b5fdab

Browse files
kazutakahiratamahesh-attarde
kazutakahirata
authored and
mahesh-attarde
committed
[ADT] Add [[nodiscard]] to set/map classes (NFC) (llvm#160978)
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()).
1 parent a452395 commit 5b5fdab

File tree

7 files changed

+144
-126
lines changed

7 files changed

+144
-126
lines changed

‎llvm/include/llvm/ADT/DenseMap.h‎

Lines changed: 47 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -75,37 +75,39 @@ class DenseMapBase : public DebugEpochBase {
7575
using const_iterator =
7676
DenseMapIterator<KeyT, ValueT, KeyInfoT, BucketT, true>;
7777

78-
inline iterator begin() {
78+
[[nodiscard]] inline iterator begin() {
7979
return iterator::makeBegin(buckets(), empty(), *this);
8080
}
81-
inline iterator end() { return iterator::makeEnd(buckets(), *this); }
82-
inline const_iterator begin() const {
81+
[[nodiscard]] inline iterator end() {
82+
return iterator::makeEnd(buckets(), *this);
83+
}
84+
[[nodiscard]] inline const_iterator begin() const {
8385
return const_iterator::makeBegin(buckets(), empty(), *this);
8486
}
85-
inline const_iterator end() const {
87+
[[nodiscard]] inline const_iterator end() const {
8688
return const_iterator::makeEnd(buckets(), *this);
8789
}
8890

8991
// Return an iterator to iterate over keys in the map.
90-
inline auto keys() {
92+
[[nodiscard]] inline auto keys() {
9193
return map_range(*this, [](const BucketT &P) { return P.getFirst(); });
9294
}
9395

9496
// Return an iterator to iterate over values in the map.
95-
inline auto values() {
97+
[[nodiscard]] inline auto values() {
9698
return map_range(*this, [](const BucketT &P) { return P.getSecond(); });
9799
}
98100

99-
inline auto keys() const {
101+
[[nodiscard]] inline auto keys() const {
100102
return map_range(*this, [](const BucketT &P) { return P.getFirst(); });
101103
}
102104

103-
inline auto values() const {
105+
[[nodiscard]] inline auto values() const {
104106
return map_range(*this, [](const BucketT &P) { return P.getSecond(); });
105107
}
106108

107109
[[nodiscard]] bool empty() const { return getNumEntries() == 0; }
108-
unsigned size() const { return getNumEntries(); }
110+
[[nodiscard]] unsigned size() const { return getNumEntries(); }
109111

110112
/// Grow the densemap so that it can contain at least \p NumEntries items
111113
/// before resizing again.
@@ -153,38 +155,43 @@ class DenseMapBase : public DebugEpochBase {
153155
}
154156

155157
/// Return true if the specified key is in the map, false otherwise.
156-
bool contains(const_arg_type_t<KeyT> Val) const {
158+
[[nodiscard]] bool contains(const_arg_type_t<KeyT> Val) const {
157159
return doFind(Val) != nullptr;
158160
}
159161

160162
/// Return 1 if the specified key is in the map, 0 otherwise.
161-
size_type count(const_arg_type_t<KeyT> Val) const {
163+
[[nodiscard]] size_type count(const_arg_type_t<KeyT> Val) const {
162164
return contains(Val) ? 1 : 0;
163165
}
164166

165-
iterator find(const_arg_type_t<KeyT> Val) { return find_as(Val); }
166-
const_iterator find(const_arg_type_t<KeyT> Val) const { return find_as(Val); }
167+
[[nodiscard]] iterator find(const_arg_type_t<KeyT> Val) {
168+
return find_as(Val);
169+
}
170+
[[nodiscard]] const_iterator find(const_arg_type_t<KeyT> Val) const {
171+
return find_as(Val);
172+
}
167173

168174
/// Alternate version of find() which allows a different, and possibly
169175
/// less expensive, key type.
170176
/// The DenseMapInfo is responsible for supplying methods
171177
/// getHashValue(LookupKeyT) and isEqual(LookupKeyT, KeyT) for each key
172178
/// type used.
173-
template <class LookupKeyT> iterator find_as(const LookupKeyT &Val) {
179+
template <class LookupKeyT>
180+
[[nodiscard]] iterator find_as(const LookupKeyT &Val) {
174181
if (BucketT *Bucket = doFind(Val))
175182
return makeIterator(Bucket);
176183
return end();
177184
}
178185
template <class LookupKeyT>
179-
const_iterator find_as(const LookupKeyT &Val) const {
186+
[[nodiscard]] const_iterator find_as(const LookupKeyT &Val) const {
180187
if (const BucketT *Bucket = doFind(Val))
181188
return makeConstIterator(Bucket);
182189
return end();
183190
}
184191

185192
/// lookup - Return the entry for the specified key, or a default
186193
/// constructed value if no such entry exists.
187-
ValueT lookup(const_arg_type_t<KeyT> Val) const {
194+
[[nodiscard]] ValueT lookup(const_arg_type_t<KeyT> Val) const {
188195
if (const BucketT *Bucket = doFind(Val))
189196
return Bucket->getSecond();
190197
return ValueT();
@@ -194,15 +201,16 @@ class DenseMapBase : public DebugEpochBase {
194201
// useful, because `lookup` cannot be used with non-default-constructible
195202
// values.
196203
template <typename U = std::remove_cv_t<ValueT>>
197-
ValueT lookup_or(const_arg_type_t<KeyT> Val, U &&Default) const {
204+
[[nodiscard]] ValueT lookup_or(const_arg_type_t<KeyT> Val,
205+
U &&Default) const {
198206
if (const BucketT *Bucket = doFind(Val))
199207
return Bucket->getSecond();
200208
return Default;
201209
}
202210

203211
/// at - Return the entry for the specified key, or abort if no such
204212
/// entry exists.
205-
const ValueT &at(const_arg_type_t<KeyT> Val) const {
213+
[[nodiscard]] const ValueT &at(const_arg_type_t<KeyT> Val) const {
206214
auto Iter = this->find(std::move(Val));
207215
assert(Iter != this->end() && "DenseMap::at failed due to a missing key");
208216
return Iter->second;
@@ -330,14 +338,16 @@ class DenseMapBase : public DebugEpochBase {
330338
/// isPointerIntoBucketsArray - Return true if the specified pointer points
331339
/// somewhere into the DenseMap's array of buckets (i.e. either to a key or
332340
/// value in the DenseMap).
333-
bool isPointerIntoBucketsArray(const void *Ptr) const {
341+
[[nodiscard]] bool isPointerIntoBucketsArray(const void *Ptr) const {
334342
return Ptr >= getBuckets() && Ptr < getBucketsEnd();
335343
}
336344

337345
/// getPointerIntoBucketsArray() - Return an opaque pointer into the buckets
338346
/// array. In conjunction with the previous method, this can be used to
339347
/// determine whether an insertion caused the DenseMap to reallocate.
340-
const void *getPointerIntoBucketsArray() const { return getBuckets(); }
348+
[[nodiscard]] const void *getPointerIntoBucketsArray() const {
349+
return getBuckets();
350+
}
341351

342352
protected:
343353
DenseMapBase() = default;
@@ -649,7 +659,9 @@ class DenseMapBase : public DebugEpochBase {
649659
/// This is just the raw memory used by DenseMap.
650660
/// If entries are pointers to objects, the size of the referenced objects
651661
/// are not included.
652-
size_t getMemorySize() const { return getNumBuckets() * sizeof(BucketT); }
662+
[[nodiscard]] size_t getMemorySize() const {
663+
return getNumBuckets() * sizeof(BucketT);
664+
}
653665
};
654666

655667
/// Equality comparison for DenseMap.
@@ -660,9 +672,9 @@ class DenseMapBase : public DebugEpochBase {
660672
/// complexity is linear, worst case is O(N^2) (if every hash collides).
661673
template <typename DerivedT, typename KeyT, typename ValueT, typename KeyInfoT,
662674
typename BucketT>
663-
bool operator==(
664-
const DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT> &LHS,
665-
const DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT> &RHS) {
675+
[[nodiscard]] bool
676+
operator==(const DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT> &LHS,
677+
const DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT> &RHS) {
666678
if (LHS.size() != RHS.size())
667679
return false;
668680

@@ -680,9 +692,9 @@ bool operator==(
680692
/// Equivalent to !(LHS == RHS). See operator== for performance notes.
681693
template <typename DerivedT, typename KeyT, typename ValueT, typename KeyInfoT,
682694
typename BucketT>
683-
bool operator!=(
684-
const DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT> &LHS,
685-
const DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT> &RHS) {
695+
[[nodiscard]] bool
696+
operator!=(const DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT> &LHS,
697+
const DenseMapBase<DerivedT, KeyT, ValueT, KeyInfoT, BucketT> &RHS) {
686698
return !(LHS == RHS);
687699
}
688700

@@ -1220,15 +1232,15 @@ class DenseMapIterator : DebugEpochBase::HandleBase {
12201232
const DenseMapIterator<KeyT, ValueT, KeyInfoT, Bucket, IsConstSrc> &I)
12211233
: DebugEpochBase::HandleBase(I), Ptr(I.Ptr), End(I.End) {}
12221234

1223-
reference operator*() const {
1235+
[[nodiscard]] reference operator*() const {
12241236
assert(isHandleInSync() && "invalid iterator access!");
12251237
assert(Ptr != End && "dereferencing end() iterator");
12261238
return *Ptr;
12271239
}
1228-
pointer operator->() const { return &operator*(); }
1240+
[[nodiscard]] pointer operator->() const { return &operator*(); }
12291241

1230-
friend bool operator==(const DenseMapIterator &LHS,
1231-
const DenseMapIterator &RHS) {
1242+
[[nodiscard]] friend bool operator==(const DenseMapIterator &LHS,
1243+
const DenseMapIterator &RHS) {
12321244
assert((!LHS.getEpochAddress() || LHS.isHandleInSync()) &&
12331245
"handle not in sync!");
12341246
assert((!RHS.getEpochAddress() || RHS.isHandleInSync()) &&
@@ -1238,8 +1250,8 @@ class DenseMapIterator : DebugEpochBase::HandleBase {
12381250
return LHS.Ptr == RHS.Ptr;
12391251
}
12401252

1241-
friend bool operator!=(const DenseMapIterator &LHS,
1242-
const DenseMapIterator &RHS) {
1253+
[[nodiscard]] friend bool operator!=(const DenseMapIterator &LHS,
1254+
const DenseMapIterator &RHS) {
12431255
return !(LHS == RHS);
12441256
}
12451257

@@ -1277,7 +1289,8 @@ class DenseMapIterator : DebugEpochBase::HandleBase {
12771289
};
12781290

12791291
template <typename KeyT, typename ValueT, typename KeyInfoT>
1280-
inline size_t capacity_in_bytes(const DenseMap<KeyT, ValueT, KeyInfoT> &X) {
1292+
[[nodiscard]] inline size_t
1293+
capacity_in_bytes(const DenseMap<KeyT, ValueT, KeyInfoT> &X) {
12811294
return X.getMemorySize();
12821295
}
12831296

‎llvm/include/llvm/ADT/DenseSet.h‎

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,9 @@ class DenseSetImpl {
8383
DenseSetImpl(llvm::from_range_t, Range &&R)
8484
: DenseSetImpl(adl_begin(R), adl_end(R)) {}
8585

86-
bool empty() const { return TheMap.empty(); }
87-
size_type size() const { return TheMap.size(); }
88-
size_t getMemorySize() const { return TheMap.getMemorySize(); }
86+
[[nodiscard]] bool empty() const { return TheMap.empty(); }
87+
[[nodiscard]] size_type size() const { return TheMap.size(); }
88+
[[nodiscard]] size_t getMemorySize() const { return TheMap.getMemorySize(); }
8989

9090
/// Grow the DenseSet so that it has at least Size buckets. Will not shrink
9191
/// the Size of the set.
@@ -154,14 +154,20 @@ class DenseSetImpl {
154154
using iterator = DenseSetIterator<false>;
155155
using const_iterator = DenseSetIterator<true>;
156156

157-
iterator begin() { return iterator(TheMap.begin()); }
158-
iterator end() { return iterator(TheMap.end()); }
157+
[[nodiscard]] iterator begin() { return iterator(TheMap.begin()); }
158+
[[nodiscard]] iterator end() { return iterator(TheMap.end()); }
159159

160-
const_iterator begin() const { return const_iterator(TheMap.begin()); }
161-
const_iterator end() const { return const_iterator(TheMap.end()); }
160+
[[nodiscard]] const_iterator begin() const {
161+
return const_iterator(TheMap.begin());
162+
}
163+
[[nodiscard]] const_iterator end() const {
164+
return const_iterator(TheMap.end());
165+
}
162166

163-
iterator find(const_arg_type_t<ValueT> V) { return iterator(TheMap.find(V)); }
164-
const_iterator find(const_arg_type_t<ValueT> V) const {
167+
[[nodiscard]] iterator find(const_arg_type_t<ValueT> V) {
168+
return iterator(TheMap.find(V));
169+
}
170+
[[nodiscard]] const_iterator find(const_arg_type_t<ValueT> V) const {
165171
return const_iterator(TheMap.find(V));
166172
}
167173

@@ -180,10 +186,12 @@ class DenseSetImpl {
180186
/// The DenseMapInfo is responsible for supplying methods
181187
/// getHashValue(LookupKeyT) and isEqual(LookupKeyT, KeyT) for each key type
182188
/// used.
183-
template <class LookupKeyT> iterator find_as(const LookupKeyT &Val) {
189+
template <class LookupKeyT>
190+
[[nodiscard]] iterator find_as(const LookupKeyT &Val) {
184191
return iterator(TheMap.find_as(Val));
185192
}
186193
template <class LookupKeyT>
194+
[[nodiscard]]
187195
const_iterator find_as(const LookupKeyT &Val) const {
188196
return const_iterator(TheMap.find_as(Val));
189197
}
@@ -229,8 +237,9 @@ class DenseSetImpl {
229237
/// Equivalent to N calls to RHS.count. Amortized complexity is linear, worst
230238
/// case is O(N^2) (if every hash collides).
231239
template <typename ValueT, typename MapTy, typename ValueInfoT>
232-
bool operator==(const DenseSetImpl<ValueT, MapTy, ValueInfoT> &LHS,
233-
const DenseSetImpl<ValueT, MapTy, ValueInfoT> &RHS) {
240+
[[nodiscard]] bool
241+
operator==(const DenseSetImpl<ValueT, MapTy, ValueInfoT> &LHS,
242+
const DenseSetImpl<ValueT, MapTy, ValueInfoT> &RHS) {
234243
if (LHS.size() != RHS.size())
235244
return false;
236245

@@ -245,8 +254,9 @@ bool operator==(const DenseSetImpl<ValueT, MapTy, ValueInfoT> &LHS,
245254
///
246255
/// Equivalent to !(LHS == RHS). See operator== for performance notes.
247256
template <typename ValueT, typename MapTy, typename ValueInfoT>
248-
bool operator!=(const DenseSetImpl<ValueT, MapTy, ValueInfoT> &LHS,
249-
const DenseSetImpl<ValueT, MapTy, ValueInfoT> &RHS) {
257+
[[nodiscard]] bool
258+
operator!=(const DenseSetImpl<ValueT, MapTy, ValueInfoT> &LHS,
259+
const DenseSetImpl<ValueT, MapTy, ValueInfoT> &RHS) {
250260
return !(LHS == RHS);
251261
}
252262

‎llvm/include/llvm/ADT/MapVector.h‎

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,15 @@ class MapVector {
4545
using const_reverse_iterator = typename VectorType::const_reverse_iterator;
4646

4747
/// Clear the MapVector and return the underlying vector.
48-
VectorType takeVector() {
48+
[[nodiscard]] VectorType takeVector() {
4949
Map.clear();
5050
return std::move(Vector);
5151
}
5252

5353
/// Returns an array reference of the underlying vector.
54-
ArrayRef<value_type> getArrayRef() const { return Vector; }
54+
[[nodiscard]] ArrayRef<value_type> getArrayRef() const { return Vector; }
5555

56-
size_type size() const { return Vector.size(); }
56+
[[nodiscard]] size_type size() const { return Vector.size(); }
5757

5858
/// Grow the MapVector so that it can contain at least \p NumEntries items
5959
/// before resizing again.
@@ -62,24 +62,28 @@ class MapVector {
6262
Vector.reserve(NumEntries);
6363
}
6464

65-
iterator begin() { return Vector.begin(); }
66-
const_iterator begin() const { return Vector.begin(); }
67-
iterator end() { return Vector.end(); }
68-
const_iterator end() const { return Vector.end(); }
65+
[[nodiscard]] iterator begin() { return Vector.begin(); }
66+
[[nodiscard]] const_iterator begin() const { return Vector.begin(); }
67+
[[nodiscard]] iterator end() { return Vector.end(); }
68+
[[nodiscard]] const_iterator end() const { return Vector.end(); }
6969

70-
reverse_iterator rbegin() { return Vector.rbegin(); }
71-
const_reverse_iterator rbegin() const { return Vector.rbegin(); }
72-
reverse_iterator rend() { return Vector.rend(); }
73-
const_reverse_iterator rend() const { return Vector.rend(); }
74-
75-
bool empty() const {
76-
return Vector.empty();
70+
[[nodiscard]] reverse_iterator rbegin() { return Vector.rbegin(); }
71+
[[nodiscard]] const_reverse_iterator rbegin() const {
72+
return Vector.rbegin();
7773
}
74+
[[nodiscard]] reverse_iterator rend() { return Vector.rend(); }
75+
[[nodiscard]] const_reverse_iterator rend() const { return Vector.rend(); }
76+
77+
[[nodiscard]] bool empty() const { return Vector.empty(); }
7878

79-
std::pair<KeyT, ValueT> &front() { return Vector.front(); }
80-
const std::pair<KeyT, ValueT> &front() const { return Vector.front(); }
81-
std::pair<KeyT, ValueT> &back() { return Vector.back(); }
82-
const std::pair<KeyT, ValueT> &back() const { return Vector.back(); }
79+
[[nodiscard]] std::pair<KeyT, ValueT> &front() { return Vector.front(); }
80+
[[nodiscard]] const std::pair<KeyT, ValueT> &front() const {
81+
return Vector.front();
82+
}
83+
[[nodiscard]] std::pair<KeyT, ValueT> &back() { return Vector.back(); }
84+
[[nodiscard]] const std::pair<KeyT, ValueT> &back() const {
85+
return Vector.back();
86+
}
8387

8488
void clear() {
8589
Map.clear();
@@ -96,7 +100,7 @@ class MapVector {
96100
}
97101

98102
// Returns a copy of the value. Only allowed if ValueT is copyable.
99-
ValueT lookup(const KeyT &Key) const {
103+
[[nodiscard]] ValueT lookup(const KeyT &Key) const {
100104
static_assert(std::is_copy_constructible_v<ValueT>,
101105
"Cannot call lookup() if ValueT is not copyable.");
102106
typename MapType::const_iterator Pos = Map.find(Key);
@@ -134,17 +138,21 @@ class MapVector {
134138
return Ret;
135139
}
136140

137-
bool contains(const KeyT &Key) const { return Map.find(Key) != Map.end(); }
141+
[[nodiscard]] bool contains(const KeyT &Key) const {
142+
return Map.find(Key) != Map.end();
143+
}
138144

139-
size_type count(const KeyT &Key) const { return contains(Key) ? 1 : 0; }
145+
[[nodiscard]] size_type count(const KeyT &Key) const {
146+
return contains(Key) ? 1 : 0;
147+
}
140148

141-
iterator find(const KeyT &Key) {
149+
[[nodiscard]] iterator find(const KeyT &Key) {
142150
typename MapType::const_iterator Pos = Map.find(Key);
143151
return Pos == Map.end()? Vector.end() :
144152
(Vector.begin() + Pos->second);
145153
}
146154

147-
const_iterator find(const KeyT &Key) const {
155+
[[nodiscard]] const_iterator find(const KeyT &Key) const {
148156
typename MapType::const_iterator Pos = Map.find(Key);
149157
return Pos == Map.end()? Vector.end() :
150158
(Vector.begin() + Pos->second);

0 commit comments

Comments
 (0)