[libc] Implement forward iterators for libc fixed_vector

[jameshu15869]

• Implements forward iterators for cpp::fixed_vector to use in [libc] NVPTX Profiling #92009

[llvmbot]

@llvm/pr-subscribers-libc

Author: None (jameshu15869)

Changes

• Implements forward iterators to use in [libc] NVPTX Profiling #92009

---

Full diff: https://github.com/llvm/llvm-project/pull/93916.diff

2 Files Affected:

• (modified) libc/src/__support/fixedvector.h (+4)
• (modified) libc/test/src/__support/CPP/array_test.cpp (+5)

diff --git a/libc/src/__support/fixedvector.h b/libc/src/__support/fixedvector.h
index 81747ee10067c..6aeb4d56363e9 100644
--- a/libc/src/__support/fixedvector.h
+++ b/libc/src/__support/fixedvector.h
@@ -63,6 +63,10 @@ template <typename T, size_t CAPACITY> class FixedVector {
     return reverse_iterator{&store[item_count]};
   }
   LIBC_INLINE constexpr reverse_iterator rend() { return store.rend(); }
+
+  using iterator = typename cpp::array<T, CAPACITY>::iterator;
+  LIBC_INLINE constexpr iterator begin() { return store.begin(); }
+  LIBC_INLINE constexpr iterator end() { return iterator{&store[item_count]}; }
 };
 
 } // namespace LIBC_NAMESPACE
diff --git a/libc/test/src/__support/CPP/array_test.cpp b/libc/test/src/__support/CPP/array_test.cpp
index f2d7bff636e42..06991871f6172 100644
--- a/libc/test/src/__support/CPP/array_test.cpp
+++ b/libc/test/src/__support/CPP/array_test.cpp
@@ -28,6 +28,11 @@ TEST(LlvmLibcArrayTest, Basic) {
   ASSERT_EQ(*(++it), 1);
   ASSERT_EQ(*(++it), 0);
 
+  auto forward_it = a.begin();
+  ASSERT_EQ(*forward_it, 0);
+  ASSERT_EQ(*(++forward_it), 1);
+  ASSERT_EQ(*(++forward_it), 2);
+
   for (int &x : a)
     ASSERT_GE(x, 0);
 }

[jhuber6]

Does returning store.end() not work?

Suggested change

	LIBC_INLINE constexpr iterator end() { return iterator{&store[item_count]}; }
	LIBC_INLINE constexpr iterator end() { return store.end(); }

[jameshu15869]

I think store.end() ends up using the entire space allocated to the array instead of the actual number of elements inside it. I was getting a memory error with store.end()but I'm not 100% sure that is the cause

[jhuber6]

Ah, that makes sense. I'm wondering if something like
return store.begin() + item_count would work better.

[jameshu15869]

Would that account for the size of the objects themselves when doing the pointer arithmetic? i.e. if an int is 4 bytes and you have 4 elements would you want end() to be at store.begin() + 4*4?

[jhuber6]

The iterator type is T * from the template, so it should know how to do pointer arithmetic on it, but it should be caught in your tests.

[jameshu15869]

Gotcha, I was waiting on rebuilding LLVM this morning but I can try it when I get home later today

[lntue]

I actually prefer &store[item_count] and let the compiler do the pointer arithmetic for you.

[jhuber6]

Just using the iterator directly makes sure it's the correct iterator type though.

[michaelrj-google]

LGTM

[nickdesaulniers]

Consider updating the commit message and PR description to note that this is only for fixedvector.

[jameshu15869]

Changed!