[llvm] [ArrayRef] Add constructor from iterator_range<U*> (NFC). (PR #137796)
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 30 02:15:30 PDT 2025
https://github.com/fhahn updated https://github.com/llvm/llvm-project/pull/137796
>From 93a376047ecd49637fff2f21249f7e2d1593a365 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/6] [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 4105fd1387931bc71fa4213f612a1caea28fb493 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/6] !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 da7f27522c920551140599c833f421dde53d201c 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/6] !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 503b6022bece8bcb948035ef08d306180858b723 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/6] !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());
>From d6e3f8d8f827ac8d2c03ca6ae4b53c314b4b6486 Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Wed, 30 Apr 2025 10:09:31 +0100
Subject: [PATCH 5/6] !fixup is_same_v -> is_convertible_v, is_constructible_v
---
llvm/include/llvm/ADT/ArrayRef.h | 3 ++-
llvm/unittests/ADT/ArrayRefTest.cpp | 16 ++++++++++------
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/llvm/include/llvm/ADT/ArrayRef.h b/llvm/include/llvm/ADT/ArrayRef.h
index 4f5567ecc55b7..1bb44806d223c 100644
--- a/llvm/include/llvm/ADT/ArrayRef.h
+++ b/llvm/include/llvm/ADT/ArrayRef.h
@@ -152,7 +152,8 @@ namespace llvm {
/// Construct an ArrayRef<T> from iterator_range<U*>. This uses SFINAE
/// 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 *>>>
+ template <typename U,
+ typename = std::enable_if_t<std::is_convertible_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 f6f36e5a881e1..017193d5eaa4b 100644
--- a/llvm/unittests/ADT/ArrayRefTest.cpp
+++ b/llvm/unittests/ADT/ArrayRefTest.cpp
@@ -259,14 +259,13 @@ 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,
+static_assert(!std::is_constructible_v<
+ ArrayRef<int>, iterator_range<TestRandomAccessIterator>>,
+ "cannot construct from iterator range with non-pointer iterator");
+static_assert(!std::is_constructible_v<ArrayRef<int>, iterator_range<int>>,
"cannot construct from iterator range with non-pointer iterator");
static_assert(
- std::is_constructible<ArrayRef<char *>, iterator_range<char **>>::value,
+ std::is_constructible_v<ArrayRef<char *>, iterator_range<char **>>,
"should be able to construct ArrayRef from iterator_range over pointers");
TEST(ArrayRefTest, ArrayRefFromIteratorRange) {
@@ -276,6 +275,11 @@ TEST(ArrayRefTest, ArrayRefFromIteratorRange) {
EXPECT_EQ(A1.size(), A2.size());
for (std::size_t i = 0; i < A1.size(); ++i)
EXPECT_EQ(A1[i], A2[i]);
+
+ ArrayRef<const int> A3 = make_range(A1.begin(), A1.end());
+ EXPECT_EQ(A1.size(), A3.size());
+ for (std::size_t i = 0; i < A1.size(); ++i)
+ EXPECT_EQ(A1[i], A3[i]);
}
TEST(ArrayRefTest, ArrayRefFromIteratorConstRange) {
>From 9d4410a1f2ab994b77674e096de096362eb4401c Mon Sep 17 00:00:00 2001
From: Florian Hahn <flo at fhahn.com>
Date: Wed, 30 Apr 2025 10:14:51 +0100
Subject: [PATCH 6/6] !fixup add astatic assert ArrayRef<int> from
ArrayRef<const int *>.
---
llvm/unittests/ADT/ArrayRefTest.cpp | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/llvm/unittests/ADT/ArrayRefTest.cpp b/llvm/unittests/ADT/ArrayRefTest.cpp
index 017193d5eaa4b..8de75beeef19c 100644
--- a/llvm/unittests/ADT/ArrayRefTest.cpp
+++ b/llvm/unittests/ADT/ArrayRefTest.cpp
@@ -264,6 +264,11 @@ static_assert(!std::is_constructible_v<
"cannot construct from iterator range with non-pointer iterator");
static_assert(!std::is_constructible_v<ArrayRef<int>, iterator_range<int>>,
"cannot construct from iterator range with non-pointer iterator");
+static_assert(
+ !std::is_constructible_v<ArrayRef<int>, iterator_range<const int *>>,
+ "cannot construct ArrayRef with non-const elements from const iterator "
+ "range");
+
static_assert(
std::is_constructible_v<ArrayRef<char *>, iterator_range<char **>>,
"should be able to construct ArrayRef from iterator_range over pointers");
More information about the llvm-commits
mailing list