Skip to content

Commit 372f786

Browse files
kazutakahiratakazutakahirata
authored
[ADT] Add [[nodiscard]] to more classes (NFC) (#161044)
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.
1 parent 0de265c commit 372f786

File tree

4 files changed

+81
-54
lines changed

4 files changed

+81
-54
lines changed

‎llvm/include/llvm/ADT/ImmutableMap.h‎

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -111,25 +111,25 @@ class ImmutableMap {
111111
}
112112
};
113113

114-
bool contains(key_type_ref K) const {
114+
[[nodiscard]] bool contains(key_type_ref K) const {
115115
return Root ? Root->contains(K) : false;
116116
}
117117

118-
bool operator==(const ImmutableMap &RHS) const {
118+
[[nodiscard]] bool operator==(const ImmutableMap &RHS) const {
119119
return Root && RHS.Root ? Root->isEqual(*RHS.Root.get()) : Root == RHS.Root;
120120
}
121121

122-
bool operator!=(const ImmutableMap &RHS) const {
122+
[[nodiscard]] bool operator!=(const ImmutableMap &RHS) const {
123123
return Root && RHS.Root ? Root->isNotEqual(*RHS.Root.get())
124124
: Root != RHS.Root;
125125
}
126126

127-
TreeTy *getRoot() const {
127+
[[nodiscard]] TreeTy *getRoot() const {
128128
if (Root) { Root->retain(); }
129129
return Root.get();
130130
}
131131

132-
TreeTy *getRootWithoutRetain() const { return Root.get(); }
132+
[[nodiscard]] TreeTy *getRootWithoutRetain() const { return Root.get(); }
133133

134134
void manualRetain() {
135135
if (Root) Root->retain();
@@ -139,7 +139,7 @@ class ImmutableMap {
139139
if (Root) Root->release();
140140
}
141141

142-
bool isEmpty() const { return !Root; }
142+
[[nodiscard]] bool isEmpty() const { return !Root; }
143143

144144
public:
145145
//===--------------------------------------------------===//
@@ -163,10 +163,10 @@ class ImmutableMap {
163163
data_type_ref getData() const { return (*this)->second; }
164164
};
165165

166-
iterator begin() const { return iterator(Root.get()); }
167-
iterator end() const { return iterator(); }
166+
[[nodiscard]] iterator begin() const { return iterator(Root.get()); }
167+
[[nodiscard]] iterator end() const { return iterator(); }
168168

169-
data_type* lookup(key_type_ref K) const {
169+
[[nodiscard]] data_type *lookup(key_type_ref K) const {
170170
if (Root) {
171171
TreeTy* T = Root->find(K);
172172
if (T) return &T->getValue().second;
@@ -178,15 +178,17 @@ class ImmutableMap {
178178
/// getMaxElement - Returns the <key,value> pair in the ImmutableMap for
179179
/// which key is the highest in the ordering of keys in the map. This
180180
/// method returns NULL if the map is empty.
181-
value_type* getMaxElement() const {
181+
[[nodiscard]] value_type *getMaxElement() const {
182182
return Root ? &(Root->getMaxElement()->getValue()) : nullptr;
183183
}
184184

185185
//===--------------------------------------------------===//
186186
// Utility methods.
187187
//===--------------------------------------------------===//
188188

189-
unsigned getHeight() const { return Root ? Root->getHeight() : 0; }
189+
[[nodiscard]] unsigned getHeight() const {
190+
return Root ? Root->getHeight() : 0;
191+
}
190192

191193
static inline void Profile(FoldingSetNodeID& ID, const ImmutableMap& M) {
192194
ID.AddPointer(M.Root.get());
@@ -250,24 +252,24 @@ class ImmutableMapRef {
250252
return ImmutableMapRef(NewT, Factory);
251253
}
252254

253-
bool contains(key_type_ref K) const {
255+
[[nodiscard]] bool contains(key_type_ref K) const {
254256
return Root ? Root->contains(K) : false;
255257
}
256258

257259
ImmutableMap<KeyT, ValT> asImmutableMap() const {
258260
return ImmutableMap<KeyT, ValT>(Factory->getCanonicalTree(Root.get()));
259261
}
260262

261-
bool operator==(const ImmutableMapRef &RHS) const {
263+
[[nodiscard]] bool operator==(const ImmutableMapRef &RHS) const {
262264
return Root && RHS.Root ? Root->isEqual(*RHS.Root.get()) : Root == RHS.Root;
263265
}
264266

265-
bool operator!=(const ImmutableMapRef &RHS) const {
267+
[[nodiscard]] bool operator!=(const ImmutableMapRef &RHS) const {
266268
return Root && RHS.Root ? Root->isNotEqual(*RHS.Root.get())
267269
: Root != RHS.Root;
268270
}
269271

270-
bool isEmpty() const { return !Root; }
272+
[[nodiscard]] bool isEmpty() const { return !Root; }
271273

272274
//===--------------------------------------------------===//
273275
// For testing.
@@ -293,10 +295,10 @@ class ImmutableMapRef {
293295
data_type_ref getData() const { return (*this)->second; }
294296
};
295297

296-
iterator begin() const { return iterator(Root.get()); }
297-
iterator end() const { return iterator(); }
298+
[[nodiscard]] iterator begin() const { return iterator(Root.get()); }
299+
[[nodiscard]] iterator end() const { return iterator(); }
298300

299-
data_type *lookup(key_type_ref K) const {
301+
[[nodiscard]] data_type *lookup(key_type_ref K) const {
300302
if (Root) {
301303
TreeTy* T = Root->find(K);
302304
if (T) return &T->getValue().second;
@@ -308,15 +310,17 @@ class ImmutableMapRef {
308310
/// getMaxElement - Returns the <key,value> pair in the ImmutableMap for
309311
/// which key is the highest in the ordering of keys in the map. This
310312
/// method returns NULL if the map is empty.
311-
value_type* getMaxElement() const {
313+
[[nodiscard]] value_type *getMaxElement() const {
312314
return Root ? &(Root->getMaxElement()->getValue()) : nullptr;
313315
}
314316

315317
//===--------------------------------------------------===//
316318
// Utility methods.
317319
//===--------------------------------------------------===//
318320

319-
unsigned getHeight() const { return Root ? Root->getHeight() : 0; }
321+
[[nodiscard]] unsigned getHeight() const {
322+
return Root ? Root->getHeight() : 0;
323+
}
320324

321325
static inline void Profile(FoldingSetNodeID &ID, const ImmutableMapRef &M) {
322326
ID.AddPointer(M.Root.get());

‎llvm/include/llvm/ADT/SparseSet.h‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -171,23 +171,23 @@ class SparseSet {
171171
using iterator = typename DenseT::iterator;
172172
using const_iterator = typename DenseT::const_iterator;
173173

174-
const_iterator begin() const { return Dense.begin(); }
175-
const_iterator end() const { return Dense.end(); }
176-
iterator begin() { return Dense.begin(); }
177-
iterator end() { return Dense.end(); }
174+
[[nodiscard]] const_iterator begin() const { return Dense.begin(); }
175+
[[nodiscard]] const_iterator end() const { return Dense.end(); }
176+
[[nodiscard]] iterator begin() { return Dense.begin(); }
177+
[[nodiscard]] iterator end() { return Dense.end(); }
178178

179179
/// empty - Returns true if the set is empty.
180180
///
181181
/// This is not the same as BitVector::empty().
182182
///
183-
bool empty() const { return Dense.empty(); }
183+
[[nodiscard]] bool empty() const { return Dense.empty(); }
184184

185185
/// size - Returns the number of elements in the set.
186186
///
187187
/// This is not the same as BitVector::size() which returns the size of the
188188
/// universe.
189189
///
190-
size_type size() const { return Dense.size(); }
190+
[[nodiscard]] size_type size() const { return Dense.size(); }
191191

192192
/// clear - Clears the set. This is a very fast constant time operation.
193193
///
@@ -222,21 +222,27 @@ class SparseSet {
222222
/// @param Key A valid key to find.
223223
/// @returns An iterator to the element identified by key, or end().
224224
///
225-
iterator find(const KeyT &Key) { return findIndex(KeyIndexOf(Key)); }
225+
[[nodiscard]] iterator find(const KeyT &Key) {
226+
return findIndex(KeyIndexOf(Key));
227+
}
226228

227-
const_iterator find(const KeyT &Key) const {
229+
[[nodiscard]] const_iterator find(const KeyT &Key) const {
228230
return const_cast<SparseSet *>(this)->findIndex(KeyIndexOf(Key));
229231
}
230232

231233
/// Check if the set contains the given \c Key.
232234
///
233235
/// @param Key A valid key to find.
234-
bool contains(const KeyT &Key) const { return find(Key) != end(); }
236+
[[nodiscard]] bool contains(const KeyT &Key) const {
237+
return find(Key) != end();
238+
}
235239

236240
/// count - Returns 1 if this set contains an element identified by Key,
237241
/// 0 otherwise.
238242
///
239-
size_type count(const KeyT &Key) const { return contains(Key) ? 1 : 0; }
243+
[[nodiscard]] size_type count(const KeyT &Key) const {
244+
return contains(Key) ? 1 : 0;
245+
}
240246

241247
/// insert - Attempts to insert a new element.
242248
///

‎llvm/include/llvm/ADT/StringMap.h‎

Lines changed: 38 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -102,18 +102,18 @@ class StringMapImpl {
102102
return reinterpret_cast<StringMapEntryBase *>(TombstoneIntVal);
103103
}
104104

105-
unsigned getNumBuckets() const { return NumBuckets; }
106-
unsigned getNumItems() const { return NumItems; }
105+
[[nodiscard]] unsigned getNumBuckets() const { return NumBuckets; }
106+
[[nodiscard]] unsigned getNumItems() const { return NumItems; }
107107

108-
bool empty() const { return NumItems == 0; }
109-
unsigned size() const { return NumItems; }
108+
[[nodiscard]] bool empty() const { return NumItems == 0; }
109+
[[nodiscard]] unsigned size() const { return NumItems; }
110110

111111
/// Returns the hash value that will be used for the given string.
112112
/// This allows precomputing the value and passing it explicitly
113113
/// to some of the functions.
114114
/// The implementation of this function is not guaranteed to be stable
115115
/// and may change.
116-
LLVM_ABI static uint32_t hash(StringRef Key);
116+
[[nodiscard]] LLVM_ABI static uint32_t hash(StringRef Key);
117117

118118
void swap(StringMapImpl &Other) {
119119
std::swap(TheTable, Other.TheTable);
@@ -220,30 +220,35 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap
220220
using const_iterator = StringMapIterBase<ValueTy, true>;
221221
using iterator = StringMapIterBase<ValueTy, false>;
222222

223-
iterator begin() { return iterator(TheTable, NumBuckets != 0); }
224-
iterator end() { return iterator(TheTable + NumBuckets); }
225-
const_iterator begin() const {
223+
[[nodiscard]] iterator begin() { return iterator(TheTable, NumBuckets != 0); }
224+
[[nodiscard]] iterator end() { return iterator(TheTable + NumBuckets); }
225+
[[nodiscard]] const_iterator begin() const {
226226
return const_iterator(TheTable, NumBuckets != 0);
227227
}
228-
const_iterator end() const { return const_iterator(TheTable + NumBuckets); }
228+
[[nodiscard]] const_iterator end() const {
229+
return const_iterator(TheTable + NumBuckets);
230+
}
229231

230-
iterator_range<StringMapKeyIterator<ValueTy>> keys() const {
232+
[[nodiscard]] iterator_range<StringMapKeyIterator<ValueTy>> keys() const {
231233
return make_range(StringMapKeyIterator<ValueTy>(begin()),
232234
StringMapKeyIterator<ValueTy>(end()));
233235
}
234236

235-
iterator find(StringRef Key) { return find(Key, hash(Key)); }
237+
[[nodiscard]] iterator find(StringRef Key) { return find(Key, hash(Key)); }
236238

237-
iterator find(StringRef Key, uint32_t FullHashValue) {
239+
[[nodiscard]] iterator find(StringRef Key, uint32_t FullHashValue) {
238240
int Bucket = FindKey(Key, FullHashValue);
239241
if (Bucket == -1)
240242
return end();
241243
return iterator(TheTable + Bucket);
242244
}
243245

244-
const_iterator find(StringRef Key) const { return find(Key, hash(Key)); }
246+
[[nodiscard]] const_iterator find(StringRef Key) const {
247+
return find(Key, hash(Key));
248+
}
245249

246-
const_iterator find(StringRef Key, uint32_t FullHashValue) const {
250+
[[nodiscard]] const_iterator find(StringRef Key,
251+
uint32_t FullHashValue) const {
247252
int Bucket = FindKey(Key, FullHashValue);
248253
if (Bucket == -1)
249254
return end();
@@ -252,7 +257,7 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap
252257

253258
/// lookup - Return the entry for the specified key, or a default
254259
/// constructed value if no such entry exists.
255-
ValueTy lookup(StringRef Key) const {
260+
[[nodiscard]] ValueTy lookup(StringRef Key) const {
256261
const_iterator Iter = find(Key);
257262
if (Iter != end())
258263
return Iter->second;
@@ -261,7 +266,7 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap
261266

262267
/// at - Return the entry for the specified key, or abort if no such
263268
/// entry exists.
264-
const ValueTy &at(StringRef Val) const {
269+
[[nodiscard]] const ValueTy &at(StringRef Val) const {
265270
auto Iter = this->find(Val);
266271
assert(Iter != this->end() && "StringMap::at failed due to a missing key");
267272
return Iter->second;
@@ -272,18 +277,22 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap
272277
ValueTy &operator[](StringRef Key) { return try_emplace(Key).first->second; }
273278

274279
/// contains - Return true if the element is in the map, false otherwise.
275-
bool contains(StringRef Key) const { return find(Key) != end(); }
280+
[[nodiscard]] bool contains(StringRef Key) const {
281+
return find(Key) != end();
282+
}
276283

277284
/// count - Return 1 if the element is in the map, 0 otherwise.
278-
size_type count(StringRef Key) const { return contains(Key) ? 1 : 0; }
285+
[[nodiscard]] size_type count(StringRef Key) const {
286+
return contains(Key) ? 1 : 0;
287+
}
279288

280289
template <typename InputTy>
281-
size_type count(const StringMapEntry<InputTy> &MapEntry) const {
290+
[[nodiscard]] size_type count(const StringMapEntry<InputTy> &MapEntry) const {
282291
return count(MapEntry.getKey());
283292
}
284293

285294
/// equal - check whether both of the containers are equal.
286-
bool operator==(const StringMap &RHS) const {
295+
[[nodiscard]] bool operator==(const StringMap &RHS) const {
287296
if (size() != RHS.size())
288297
return false;
289298

@@ -302,7 +311,9 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap
302311
return true;
303312
}
304313

305-
bool operator!=(const StringMap &RHS) const { return !(*this == RHS); }
314+
[[nodiscard]] bool operator!=(const StringMap &RHS) const {
315+
return !(*this == RHS);
316+
}
306317

307318
/// insert - Insert the specified key/value pair into the map. If the key
308319
/// already exists in the map, return false and ignore the request, otherwise
@@ -447,8 +458,12 @@ template <typename ValueTy, bool IsConst> class StringMapIterBase {
447458
AdvancePastEmptyBuckets();
448459
}
449460

450-
reference operator*() const { return *static_cast<value_type *>(*Ptr); }
451-
pointer operator->() const { return static_cast<value_type *>(*Ptr); }
461+
[[nodiscard]] reference operator*() const {
462+
return *static_cast<value_type *>(*Ptr);
463+
}
464+
[[nodiscard]] pointer operator->() const {
465+
return static_cast<value_type *>(*Ptr);
466+
}
452467

453468
StringMapIterBase &operator++() { // Preincrement
454469
++Ptr;

‎llvm/include/llvm/ADT/StringSet.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,9 @@ class StringSet : public StringMap<std::nullopt_t, AllocatorTy> {
5757
}
5858

5959
/// Check if the set contains the given \c key.
60-
bool contains(StringRef key) const { return Base::contains(key); }
60+
[[nodiscard]] bool contains(StringRef key) const {
61+
return Base::contains(key);
62+
}
6163
};
6264

6365
} // end namespace llvm

0 commit comments

Comments
 (0)