[ADT] Reduce boilerplate in DenseSet (NFC) #158456
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The class definitions of DenseSet and SmallDenseSet contain a lot of
boilerplate code, repeating the lengthy base class name twice in each
definition.
This patch simplifies the two definitions by making them type aliases.
The patch adds an implicit default constructor. Without the implicit
default constructor, the following wouldn't work:
Index.exportToDot(OSDot, {});
One caveat is that with the type alias-based solution, forward
declarations like "class DenseSet;" are no longer valid. This patch
updates the forward declaration in MLIR.
|
@llvm/pr-subscribers-mlir @llvm/pr-subscribers-llvm-adt Author: Kazu Hirata (kazutakahirata) ChangesThe class definitions of DenseSet and SmallDenseSet contain a lot of This patch simplifies the two definitions by making them type aliases. Index.exportToDot(OSDot, {}); One caveat is that with the type alias-based solution, forward Full diff: https://github.com/llvm/llvm-project/pull/158456.diff 2 Files Affected:
diff --git a/llvm/include/llvm/ADT/DenseSet.h b/llvm/include/llvm/ADT/DenseSet.h
index 281d4d1c78cc0..9668eb6640d1f 100644
--- a/llvm/include/llvm/ADT/DenseSet.h
+++ b/llvm/include/llvm/ADT/DenseSet.h
@@ -66,7 +66,8 @@ class DenseSetImpl {
using value_type = ValueT;
using size_type = unsigned;
- explicit DenseSetImpl(unsigned InitialReserve = 0) : TheMap(InitialReserve) {}
+ DenseSetImpl() = default;
+ explicit DenseSetImpl(unsigned InitialReserve) : TheMap(InitialReserve) {}
template <typename InputIt>
DenseSetImpl(const InputIt &I, const InputIt &E)
@@ -254,40 +255,21 @@ bool operator!=(const DenseSetImpl<ValueT, MapTy, ValueInfoT> &LHS,
/// Implements a dense probed hash-table based set.
template <typename ValueT, typename ValueInfoT = DenseMapInfo<ValueT>>
-class DenseSet : public detail::DenseSetImpl<
- ValueT,
- DenseMap<ValueT, detail::DenseSetEmpty, ValueInfoT,
- detail::DenseSetPair<ValueT>>,
- ValueInfoT> {
- using BaseT =
- detail::DenseSetImpl<ValueT,
- DenseMap<ValueT, detail::DenseSetEmpty, ValueInfoT,
- detail::DenseSetPair<ValueT>>,
- ValueInfoT>;
-
-public:
- using BaseT::BaseT;
-};
+using DenseSet =
+ detail::DenseSetImpl<ValueT,
+ DenseMap<ValueT, detail::DenseSetEmpty, ValueInfoT,
+ detail::DenseSetPair<ValueT>>,
+ ValueInfoT>;
/// Implements a dense probed hash-table based set with some number of buckets
/// stored inline.
template <typename ValueT, unsigned InlineBuckets = 4,
typename ValueInfoT = DenseMapInfo<ValueT>>
-class SmallDenseSet
- : public detail::DenseSetImpl<
- ValueT,
- SmallDenseMap<ValueT, detail::DenseSetEmpty, InlineBuckets,
- ValueInfoT, detail::DenseSetPair<ValueT>>,
- ValueInfoT> {
- using BaseT = detail::DenseSetImpl<
- ValueT,
- SmallDenseMap<ValueT, detail::DenseSetEmpty, InlineBuckets, ValueInfoT,
- detail::DenseSetPair<ValueT>>,
- ValueInfoT>;
-
-public:
- using BaseT::BaseT;
-};
+using SmallDenseSet = detail::DenseSetImpl<
+ ValueT,
+ SmallDenseMap<ValueT, detail::DenseSetEmpty, InlineBuckets, ValueInfoT,
+ detail::DenseSetPair<ValueT>>,
+ ValueInfoT>;
} // end namespace llvm
diff --git a/mlir/include/mlir/Support/LLVM.h b/mlir/include/mlir/Support/LLVM.h
index 020c0fba726c8..8fd3b7c2f40f2 100644
--- a/mlir/include/mlir/Support/LLVM.h
+++ b/mlir/include/mlir/Support/LLVM.h
@@ -55,8 +55,13 @@ template <typename KeyT, typename ValueT, typename KeyInfoT, typename BucketT>
class DenseMap;
template <typename T, typename Enable>
struct DenseMapInfo;
-template <typename ValueT, typename ValueInfoT>
-class DenseSet;
+namespace detail {
+template <typename ValueT, typename MapTy, typename ValueInfoT>
+class DenseSetImpl;
+struct DenseSetEmpty;
+template <typename KeyT>
+class DenseSetPair;
+} // namespace detail
class MallocAllocator;
template <typename T>
class MutableArrayRef;
@@ -125,7 +130,11 @@ template <typename KeyT, typename ValueT,
typename BucketT = llvm::detail::DenseMapPair<KeyT, ValueT>>
using DenseMap = llvm::DenseMap<KeyT, ValueT, KeyInfoT, BucketT>;
template <typename ValueT, typename ValueInfoT = DenseMapInfo<ValueT>>
-using DenseSet = llvm::DenseSet<ValueT, ValueInfoT>;
+using DenseSet = llvm::detail::DenseSetImpl<
+ ValueT,
+ llvm::DenseMap<ValueT, llvm::detail::DenseSetEmpty, ValueInfoT,
+ llvm::detail::DenseSetPair<ValueT>>,
+ ValueInfoT>;
template <typename T, typename Vector = llvm::SmallVector<T, 0>,
typename Set = DenseSet<T>, unsigned N = 0>
using SetVector = llvm::SetVector<T, Vector, Set, N>;
|
kuhar
reviewed
Sep 14, 2025
@kuhar I've put in one more layer. A type alias eliminates the boiler plate. The user facing "forwarding" class allows us to keep the forward declaration in mlir as is. We still manage to reduce the total line count. |
kuhar
approved these changes
Sep 14, 2025
kazutakahirata
deleted the
cleanup_20250913_ADT_DenseSet_boilerplate
branch
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/31069 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/27391 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/16034 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/154/builds/21474 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The class definitions of DenseSet and SmallDenseSet contain a lot of
boilerplate code, repeating the lengthy base class name twice in each
definition.
This patch simplifies the two definitions by making them type aliases.