[clang] [llvm] [ADT] Simplify SmallSet (PR #109412)
Victor Campos via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 20 06:56:13 PDT 2024
https://github.com/vhscampos updated https://github.com/llvm/llvm-project/pull/109412
>From 58c88d6fee0f1aa486201189bbe67cea7da2d25e Mon Sep 17 00:00:00 2001
From: Victor Campos <victor.campos at arm.com>
Date: Fri, 9 Aug 2024 13:51:52 +0100
Subject: [PATCH 1/2] [ADT] Style and nit fixes in SmallSet
---
llvm/include/llvm/ADT/SmallSet.h | 43 ++++++++++++++++----------------
1 file changed, 21 insertions(+), 22 deletions(-)
diff --git a/llvm/include/llvm/ADT/SmallSet.h b/llvm/include/llvm/ADT/SmallSet.h
index a16e8ac6f07552..630c98504261aa 100644
--- a/llvm/include/llvm/ADT/SmallSet.h
+++ b/llvm/include/llvm/ADT/SmallSet.h
@@ -46,24 +46,24 @@ class SmallSetIterator
VecIterTy VecIter;
};
- bool isSmall;
+ bool IsSmall;
public:
- SmallSetIterator(SetIterTy SetIter) : SetIter(SetIter), isSmall(false) {}
+ SmallSetIterator(SetIterTy SetIter) : SetIter(SetIter), IsSmall(false) {}
- SmallSetIterator(VecIterTy VecIter) : VecIter(VecIter), isSmall(true) {}
+ SmallSetIterator(VecIterTy VecIter) : VecIter(VecIter), IsSmall(true) {}
// Spell out destructor, copy/move constructor and assignment operators for
// MSVC STL, where set<T>::const_iterator is not trivially copy constructible.
~SmallSetIterator() {
- if (isSmall)
+ if (IsSmall)
VecIter.~VecIterTy();
else
SetIter.~SetIterTy();
}
- SmallSetIterator(const SmallSetIterator &Other) : isSmall(Other.isSmall) {
- if (isSmall)
+ SmallSetIterator(const SmallSetIterator &Other) : IsSmall(Other.IsSmall) {
+ if (IsSmall)
VecIter = Other.VecIter;
else
// Use placement new, to make sure SetIter is properly constructed, even
@@ -71,8 +71,8 @@ class SmallSetIterator
new (&SetIter) SetIterTy(Other.SetIter);
}
- SmallSetIterator(SmallSetIterator &&Other) : isSmall(Other.isSmall) {
- if (isSmall)
+ SmallSetIterator(SmallSetIterator &&Other) : IsSmall(Other.IsSmall) {
+ if (IsSmall)
VecIter = std::move(Other.VecIter);
else
// Use placement new, to make sure SetIter is properly constructed, even
@@ -83,11 +83,11 @@ class SmallSetIterator
SmallSetIterator& operator=(const SmallSetIterator& Other) {
// Call destructor for SetIter, so it gets properly destroyed if it is
// not trivially destructible in case we are setting VecIter.
- if (!isSmall)
+ if (!IsSmall)
SetIter.~SetIterTy();
- isSmall = Other.isSmall;
- if (isSmall)
+ IsSmall = Other.IsSmall;
+ if (IsSmall)
VecIter = Other.VecIter;
else
new (&SetIter) SetIterTy(Other.SetIter);
@@ -97,11 +97,11 @@ class SmallSetIterator
SmallSetIterator& operator=(SmallSetIterator&& Other) {
// Call destructor for SetIter, so it gets properly destroyed if it is
// not trivially destructible in case we are setting VecIter.
- if (!isSmall)
+ if (!IsSmall)
SetIter.~SetIterTy();
- isSmall = Other.isSmall;
- if (isSmall)
+ IsSmall = Other.IsSmall;
+ if (IsSmall)
VecIter = std::move(Other.VecIter);
else
new (&SetIter) SetIterTy(std::move(Other.SetIter));
@@ -109,22 +109,22 @@ class SmallSetIterator
}
bool operator==(const SmallSetIterator &RHS) const {
- if (isSmall != RHS.isSmall)
+ if (IsSmall != RHS.IsSmall)
return false;
- if (isSmall)
+ if (IsSmall)
return VecIter == RHS.VecIter;
return SetIter == RHS.SetIter;
}
SmallSetIterator &operator++() { // Preincrement
- if (isSmall)
- VecIter++;
+ if (IsSmall)
+ ++VecIter;
else
- SetIter++;
+ ++SetIter;
return *this;
}
- const T &operator*() const { return isSmall ? *VecIter : *SetIter; }
+ const T &operator*() const { return IsSmall ? *VecIter : *SetIter; }
};
/// SmallSet - This maintains a set of unique values, optimizing for the case
@@ -167,9 +167,8 @@ class SmallSet {
if (isSmall()) {
// Since the collection is small, just do a linear search.
return vfind(V) == Vector.end() ? 0 : 1;
- } else {
- return Set.count(V);
}
+ return Set.count(V);
}
/// insert - Insert an element into the set if it isn't already there.
>From 3d83c5456c35f891aefa65f7cc6b37795af99c32 Mon Sep 17 00:00:00 2001
From: Victor Campos <victor.campos at arm.com>
Date: Fri, 20 Sep 2024 11:43:18 +0100
Subject: [PATCH 2/2] [ADT] Simplify SmallSet
- Remove dependence on `STLExtras.h`.
- Remove unused header inclusions.
- Make `count` use `contains` for deduplication.
- Replace hand-written linear scans on Vector by `std::find`.
---
clang/lib/Basic/TargetID.cpp | 1 +
llvm/include/llvm/ADT/SmallSet.h | 37 +++++++-------------------------
2 files changed, 9 insertions(+), 29 deletions(-)
diff --git a/clang/lib/Basic/TargetID.cpp b/clang/lib/Basic/TargetID.cpp
index 3c06d9bad1dc0d..fa1bfec2aacb9c 100644
--- a/clang/lib/Basic/TargetID.cpp
+++ b/clang/lib/Basic/TargetID.cpp
@@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//
#include "clang/Basic/TargetID.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/TargetParser/TargetParser.h"
diff --git a/llvm/include/llvm/ADT/SmallSet.h b/llvm/include/llvm/ADT/SmallSet.h
index 630c98504261aa..8d7511bf0bc8d9 100644
--- a/llvm/include/llvm/ADT/SmallSet.h
+++ b/llvm/include/llvm/ADT/SmallSet.h
@@ -16,14 +16,10 @@
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/iterator.h"
-#include "llvm/Support/Compiler.h"
-#include "llvm/Support/type_traits.h"
#include <cstddef>
#include <functional>
#include <set>
-#include <type_traits>
#include <utility>
namespace llvm {
@@ -139,10 +135,6 @@ class SmallSet {
SmallVector<T, N> Vector;
std::set<T, C> Set;
- using VIterator = typename SmallVector<T, N>::const_iterator;
- using SIterator = typename std::set<T, C>::const_iterator;
- using mutable_iterator = typename SmallVector<T, N>::iterator;
-
// In small mode SmallPtrSet uses linear search for the elements, so it is
// not a good idea to choose this value too high. You may consider using a
// DenseSet<> instead if you expect many elements in the set.
@@ -163,13 +155,7 @@ class SmallSet {
}
/// count - Return 1 if the element is in the set, 0 otherwise.
- size_type count(const T &V) const {
- if (isSmall()) {
- // Since the collection is small, just do a linear search.
- return vfind(V) == Vector.end() ? 0 : 1;
- }
- return Set.count(V);
- }
+ size_type count(const T &V) const { return contains(V) ? 1 : 0; }
/// insert - Insert an element into the set if it isn't already there.
/// Returns a pair. The first value of it is an iterator to the inserted
@@ -181,7 +167,7 @@ class SmallSet {
return std::make_pair(const_iterator(I), Inserted);
}
- VIterator I = vfind(V);
+ auto I = std::find(Vector.begin(), Vector.end(), V);
if (I != Vector.end()) // Don't reinsert if it already exists.
return std::make_pair(const_iterator(I), false);
if (Vector.size() < N) {
@@ -206,11 +192,11 @@ class SmallSet {
bool erase(const T &V) {
if (!isSmall())
return Set.erase(V);
- for (mutable_iterator I = Vector.begin(), E = Vector.end(); I != E; ++I)
- if (*I == V) {
- Vector.erase(I);
- return true;
- }
+ auto I = std::find(Vector.begin(), Vector.end(), V);
+ if (I != Vector.end()) {
+ Vector.erase(I);
+ return true;
+ }
return false;
}
@@ -234,19 +220,12 @@ class SmallSet {
/// Check if the SmallSet contains the given element.
bool contains(const T &V) const {
if (isSmall())
- return vfind(V) != Vector.end();
+ return std::find(Vector.begin(), Vector.end(), V) != Vector.end();
return Set.find(V) != Set.end();
}
private:
bool isSmall() const { return Set.empty(); }
-
- VIterator vfind(const T &V) const {
- for (VIterator I = Vector.begin(), E = Vector.end(); I != E; ++I)
- if (*I == V)
- return I;
- return Vector.end();
- }
};
/// If this set is of pointer values, transparently switch over to using
More information about the cfe-commits
mailing list