[llvm] [ArrayRef] Add constructor from iterator_range<U*> (NFC). (PR #137796)
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 29 11:19:36 PDT 2025
https://github.com/fhahn updated https://github.com/llvm/llvm-project/pull/137796
>From 0d3dc6c77748433a2c85dad3c40fd5d619c7192e Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Tue, 29 Apr 2025 13:19:16 +0100
Subject: [PATCH 1/4] [ArrayRef] Add constructor from iterator_range<U*> (NFC).
Add a new constructor to ArrayRef that takes an iterator_range with a
random access iterator that can be converted.
This can help to avoid creating unnecessary iterator_ranges for types
where an ArrayRef can already be constructed. I will share a follow-up
soon.
---
llvm/include/llvm/ADT/ArrayRef.h | 13 +++++++++++++
llvm/unittests/ADT/ArrayRefTest.cpp | 20 ++++++++++++++++++++
2 files changed, 33 insertions(+)
diff --git a/llvm/include/llvm/ADT/ArrayRef.h b/llvm/include/llvm/ADT/ArrayRef.h
index a1317423cdd1a..f2fc7b636a8f5 100644
--- a/llvm/include/llvm/ADT/ArrayRef.h
+++ b/llvm/include/llvm/ADT/ArrayRef.h
@@ -149,6 +149,19 @@ namespace llvm {
* = nullptr)
: Data(Vec.data()), Length(Vec.size()) {}
+ /// Construct an ArrayRef<T> from iterator_range<U*>. This uses SFINAE
+ /// to ensure that this is only used for iterator ranges of random access
+ /// iterators that can be converted.
+ template <typename U>
+ ArrayRef(const iterator_range<U *> &Range,
+ std::enable_if_t<std::is_base_of<std::random_access_iterator_tag,
+ typename std::iterator_traits<
+ decltype(Range.begin())>::
+ iterator_category>::value &&
+ std::is_convertible<U *, T const *>::value,
+ void> * = nullptr)
+ : Data(Range.begin()), Length(llvm::size(Range)) {}
+
/// @}
/// @name Simple Operations
/// @{
diff --git a/llvm/unittests/ADT/ArrayRefTest.cpp b/llvm/unittests/ADT/ArrayRefTest.cpp
index fb25ee19c0b20..128efbea813d2 100644
--- a/llvm/unittests/ADT/ArrayRefTest.cpp
+++ b/llvm/unittests/ADT/ArrayRefTest.cpp
@@ -255,6 +255,26 @@ TEST(ArrayRefTest, ArrayRefFromStdArray) {
}
}
+TEST(ArrayRefTest, ArrayRefFromIteratorRange) {
+ std::array<int, 5> A1{{42, -5, 0, 1000000, -1000000}};
+ ArrayRef<int> A2 = make_range(A1.begin(), A1.end());
+
+ EXPECT_EQ(A1.size(), A2.size());
+ for (std::size_t i = 0; i < A1.size(); ++i) {
+ EXPECT_EQ(A1[i], A2[i]);
+ }
+}
+
+TEST(ArrayRefTest, ArrayRefFromIteratorConstRange) {
+ std::array<const int, 5> A1{{42, -5, 0, 1000000, -1000000}};
+ ArrayRef<const int> A2 = make_range(A1.begin(), A1.end());
+
+ EXPECT_EQ(A1.size(), A2.size());
+ for (std::size_t i = 0; i < A1.size(); ++i) {
+ EXPECT_EQ(A1[i], A2[i]);
+ }
+}
+
static_assert(std::is_trivially_copyable_v<ArrayRef<int>>,
"trivially copyable");
>From ce3bc7cfb83093d879eebdc6280842b9042e58dc Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Tue, 29 Apr 2025 17:40:23 +0100
Subject: [PATCH 2/4] !fixup address latest comments, thanks!
---
llvm/include/llvm/ADT/ArrayRef.h | 17 +++++++++--------
llvm/unittests/ADT/ArrayRefTest.cpp | 6 ++----
2 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/llvm/include/llvm/ADT/ArrayRef.h b/llvm/include/llvm/ADT/ArrayRef.h
index f2fc7b636a8f5..47ba9fe3a23e3 100644
--- a/llvm/include/llvm/ADT/ArrayRef.h
+++ b/llvm/include/llvm/ADT/ArrayRef.h
@@ -152,14 +152,15 @@ namespace llvm {
/// Construct an ArrayRef<T> from iterator_range<U*>. This uses SFINAE
/// to ensure that this is only used for iterator ranges of random access
/// iterators that can be converted.
- template <typename U>
- ArrayRef(const iterator_range<U *> &Range,
- std::enable_if_t<std::is_base_of<std::random_access_iterator_tag,
- typename std::iterator_traits<
- decltype(Range.begin())>::
- iterator_category>::value &&
- std::is_convertible<U *, T const *>::value,
- void> * = nullptr)
+ template <typename U,
+ typename = std::enable_if_t<
+ std::is_base_of<
+ std::random_access_iterator_tag,
+ typename std::iterator_traits<
+ decltype(std::declval<const iterator_range<U *> &>()
+ .begin())>::iterator_category>::value &&
+ std::is_convertible_v<U *, T const *>>>
+ ArrayRef(const iterator_range<U *> &Range)
: Data(Range.begin()), Length(llvm::size(Range)) {}
/// @}
diff --git a/llvm/unittests/ADT/ArrayRefTest.cpp b/llvm/unittests/ADT/ArrayRefTest.cpp
index 128efbea813d2..098a0910b6cb8 100644
--- a/llvm/unittests/ADT/ArrayRefTest.cpp
+++ b/llvm/unittests/ADT/ArrayRefTest.cpp
@@ -260,9 +260,8 @@ TEST(ArrayRefTest, ArrayRefFromIteratorRange) {
ArrayRef<int> A2 = make_range(A1.begin(), A1.end());
EXPECT_EQ(A1.size(), A2.size());
- for (std::size_t i = 0; i < A1.size(); ++i) {
+ for (std::size_t i = 0; i < A1.size(); ++i)
EXPECT_EQ(A1[i], A2[i]);
- }
}
TEST(ArrayRefTest, ArrayRefFromIteratorConstRange) {
@@ -270,9 +269,8 @@ TEST(ArrayRefTest, ArrayRefFromIteratorConstRange) {
ArrayRef<const int> A2 = make_range(A1.begin(), A1.end());
EXPECT_EQ(A1.size(), A2.size());
- for (std::size_t i = 0; i < A1.size(); ++i) {
+ for (std::size_t i = 0; i < A1.size(); ++i)
EXPECT_EQ(A1[i], A2[i]);
- }
}
static_assert(std::is_trivially_copyable_v<ArrayRef<int>>,
>From 1a8430fb5df52f46eaab9ac3e0592c9d2af7c9d8 Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Tue, 29 Apr 2025 17:43:40 +0100
Subject: [PATCH 3/4] !fixup enable_if
---
llvm/include/llvm/ADT/ArrayRef.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/include/llvm/ADT/ArrayRef.h b/llvm/include/llvm/ADT/ArrayRef.h
index 47ba9fe3a23e3..c380f71832d13 100644
--- a/llvm/include/llvm/ADT/ArrayRef.h
+++ b/llvm/include/llvm/ADT/ArrayRef.h
@@ -153,7 +153,7 @@ namespace llvm {
/// to ensure that this is only used for iterator ranges of random access
/// iterators that can be converted.
template <typename U,
- typename = std::enable_if_t<
+ typename = std::enable_if<
std::is_base_of<
std::random_access_iterator_tag,
typename std::iterator_traits<
>From 0e21ecbee5de96f4f0e4b8d58971c98e40535592 Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Tue, 29 Apr 2025 19:18:51 +0100
Subject: [PATCH 4/4] !fixup simplify SFINAE check
---
llvm/include/llvm/ADT/ArrayRef.h | 13 +++----------
llvm/unittests/ADT/ArrayRefTest.cpp | 14 ++++++++++++++
2 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/llvm/include/llvm/ADT/ArrayRef.h b/llvm/include/llvm/ADT/ArrayRef.h
index c380f71832d13..4f5567ecc55b7 100644
--- a/llvm/include/llvm/ADT/ArrayRef.h
+++ b/llvm/include/llvm/ADT/ArrayRef.h
@@ -150,16 +150,9 @@ namespace llvm {
: Data(Vec.data()), Length(Vec.size()) {}
/// Construct an ArrayRef<T> from iterator_range<U*>. This uses SFINAE
- /// to ensure that this is only used for iterator ranges of random access
- /// iterators that can be converted.
- template <typename U,
- typename = std::enable_if<
- std::is_base_of<
- std::random_access_iterator_tag,
- typename std::iterator_traits<
- decltype(std::declval<const iterator_range<U *> &>()
- .begin())>::iterator_category>::value &&
- std::is_convertible_v<U *, T const *>>>
+ /// to ensure that this is only used for iterator ranges over plain pointer
+ /// iterators.
+ template <typename U, typename = std::enable_if_t<std::is_same_v<U *, T *>>>
ArrayRef(const iterator_range<U *> &Range)
: Data(Range.begin()), Length(llvm::size(Range)) {}
diff --git a/llvm/unittests/ADT/ArrayRefTest.cpp b/llvm/unittests/ADT/ArrayRefTest.cpp
index 098a0910b6cb8..f6f36e5a881e1 100644
--- a/llvm/unittests/ADT/ArrayRefTest.cpp
+++ b/llvm/unittests/ADT/ArrayRefTest.cpp
@@ -255,6 +255,20 @@ TEST(ArrayRefTest, ArrayRefFromStdArray) {
}
}
+struct TestRandomAccessIterator {
+ using iterator_category = std::random_access_iterator_tag;
+};
+
+static_assert(
+ !std::is_constructible<ArrayRef<int>,
+ iterator_range<TestRandomAccessIterator>>::value,
+ "cannot construct from iterator range with non-pointer iterator");
+static_assert(!std::is_constructible<ArrayRef<int>, iterator_range<int>>::value,
+ "cannot construct from iterator range with non-pointer iterator");
+static_assert(
+ std::is_constructible<ArrayRef<char *>, iterator_range<char **>>::value,
+ "should be able to construct ArrayRef from iterator_range over pointers");
+
TEST(ArrayRefTest, ArrayRefFromIteratorRange) {
std::array<int, 5> A1{{42, -5, 0, 1000000, -1000000}};
ArrayRef<int> A2 = make_range(A1.begin(), A1.end());
More information about the llvm-commits
mailing list