[libc++] P3168R2 Give std::optional Range Support #146491

dywoq · 2025-07-01T09:04:08Z

Link to P3168R2: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2024/p3168r2.html

…<optional<_T>> = range_format::disabled

…t_macro_components.py

github-actions · 2025-07-01T09:04:27Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

libcxx/include/optional

frederick-vs-ja · 2025-07-01T11:16:07Z

libcxx/include/optional

+  _LIBCPP_HIDE_FROM_ABI iterator begin() {
+    if (!this->has_value())
+      return this->end();
+    return iterator(reinterpret_cast<_Tp*>(this->__get()));
+  }


constexpr is missing, and reinterpret_cast is incompatible with constant evaluation. Ditto below.

Will use static_cast then

frederick-vs-ja · 2025-07-01T11:17:22Z

libcxx/test/std/utilities/optional/optional.range/iteration.pass.cpp

+int main() {
+  std::optional<int> val = 2;
+  for (const auto& elem : val) {
+    std::cout << elem << std::endl;


We should assert here.

Also, there should be a file testing the properties of the iterator types.

libcxx/include/optional

…onal> Co-authored-by: A. Jiang <[email protected]>

Co-authored-by: A. Jiang <[email protected]>

dywoq · 2025-07-01T11:51:36Z

 _LIBCPP_HIDE_FROM_ABI constexpr reference operator*() const {
    if (__ptr_ == nullptr)
      std::__throw_runtime_error("deferencering end iterator");
    return *__ptr_;
  }

  _LIBCPP_HIDE_FROM_ABI constexpr pointer operator->() const {
    if (__ptr_ == nullptr)
      std::__throw_runtime_error("deferencering end iterator");
    return *__ptr_;
  }

Should these functions return __ptr_ instead of throwing exceptions if __ptr_ is nullptr? Throwing exceptions is not allowed in constexpr functions

zwuis · 2025-07-01T12:09:53Z

 _LIBCPP_HIDE_FROM_ABI constexpr reference operator*() const {
    if (__ptr_ == nullptr)
      std::__throw_runtime_error("deferencering end iterator");
    return *__ptr_;
  }

  _LIBCPP_HIDE_FROM_ABI constexpr pointer operator->() const {
    if (__ptr_ == nullptr)
      std::__throw_runtime_error("deferencering end iterator");
    return *__ptr_;
  }

Should these functions return __ptr_ instead of throwing exceptions if __ptr_ is nullptr? Throwing exceptions is not allowed in constexpr functions

*end() is UB so these if statements should be removed.

… optional<T>::end

zwuis · 2025-07-01T12:20:56Z

libcxx/include/optional

+  _LIBCPP_HIDE_FROM_ABI constexpr pointer operator->() const { return *__ptr_; }
+
+  _LIBCPP_HIDE_FROM_ABI constexpr __optional_iterator& operator++() {
+    __ptr_ = nullptr;


This doesn't meet the Cpp17RandomAccessIterator requirements.

libcxx/test/std/utilities/optional/optional.range/iteration.pass.cpp

libcxx/test/std/utilities/optional/optional.range/satisfies_range_concept.pass.cpp

…pass test

philnik777 · 2025-07-01T13:29:53Z

libcxx/include/optional

+constexpr auto format_kind<optional<_T>> = range_format::disabled;
+
+template <typename _Tp>
+class __optional_iterator {


Couldn't we just use __wrap_iter?

philnik777 · 2025-07-01T13:30:10Z

libcxx/test/std/utilities/optional/optional.range/runtime_error.fail.cpp

+  auto end               = val.end();
+  TEST_DOES_NOT_THROW(*end);
+  return 0;
+}


Missing newline.

philnik777 · 2025-07-01T13:30:46Z

libcxx/test/std/utilities/optional/optional.range/iteration.pass.cpp

+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+


Please add comments in all tests what they are supported to do.

philnik777 · 2025-07-01T13:31:08Z

libcxx/test/std/utilities/optional/optional.range/iteration.pass.cpp

+#include <cassert>
+#include <optional>
+
+int main() {


All tests should also be constant evaluated.

philnik777 · 2025-07-01T13:31:21Z

libcxx/test/std/utilities/optional/optional.range/runtime_error.fail.cpp

Please don't use .fail.cpp.

philnik777 · 2025-07-01T13:31:56Z

libcxx/test/std/utilities/optional/optional.range/satisfies_range_concept.pass.cpp

+#include <ranges>
+
+int main() {
+  static_assert(std::ranges::range<std::optional<int>>, "std::ranges::range<std::optional<int>> is false");


You don't have to state the obvious. If there is not much to add, just don't add a message.

philnik777 · 2025-07-01T13:33:40Z

libcxx/include/optional

+template <class _T>
+constexpr auto format_kind<optional<_T>> = range_format::disabled;
+
+template <typename _Tp>


Please add tests to make sure that the iterator fulfills all the iterator requirements.

philnik777 · 2025-07-01T13:33:56Z

libcxx/include/optional

+template <class _T>
+constexpr bool ranges::enable_view<optional<_T>> = true;
+
+template <class _T>
+constexpr auto format_kind<optional<_T>> = range_format::disabled;


These don't have tests AFAICT.

philnik777 · 2025-07-01T13:34:06Z

libcxx/include/optional

+#    if _LIBCPP_STD_VER >= 26
+
+template <typename _Tp>
+class __optional_iterator;
+
+#    endif // _LIBCPP_STD_VER >= 26


Why is this needed?

philnik777 · 2025-07-01T13:34:47Z

libcxx/include/optional

 #  include <__concepts/invocable.h>
 #  include <__config>
 #  include <__exception/exception.h>
+#  include <__format/format_context.h>


Why is this include needed?

…tional.range

zwuis · 2025-07-01T14:57:34Z

libcxx/include/optional

+#    if _LIBCPP_STD_VER >= 26
+
+  using iterator       = __optional_iterator<_Tp>;
+  using const_iterator = __optional_iterator<_Tp>;


The reference type for const_iterator is const _Tp&.

zwuis · 2025-07-01T15:03:20Z

libcxx/include/optional

+  _LIBCPP_HIDE_FROM_ABI constexpr pointer operator->() const { return *__it_.operator->(); }
+
+  _LIBCPP_HIDE_FROM_ABI constexpr __optional_iterator& operator++() noexcept {
+    __it_ = _Base();


Note that std::contiguous_iterator requires std::to_address(iter2) == std::to_address(iter1) + std::iter_difference_t<Iter>(iter2 - iter1) even if iter1 is dereferenceable and iter2 is NON-dereferenceable.

…tility

dywoq · 2025-07-02T06:33:49Z

is it ready to merge?

zwuis · 2025-07-02T07:20:06Z

is it ready to merge?

~~No. Some review comments are not addressed. Note that some review comments are marked "Outdated" by GitHub, which should be addressed as well.~~

My fault. It seems that all review comments are addressed currently.

dywoq · 2025-07-02T07:45:08Z

is it ready to merge?

~~No. Some review comments are not addressed. Note that some review comments are marked "Outdated" by GitHub, which should be addressed as well.~~

My fault. It seems that all review comments are addressed currently.

It's nothing

zwuis · 2025-07-02T07:36:37Z

libcxx/include/optional

+
+template <typename _Tp>
+class __optional_iterator {
+  using _Base = __wrap_iter<_Tp*>;


Missing _LIBCPP_NODEBUG.

I meant that we don't need to invent a new iterator at all. We can just use __wrap_iter instead.

oh, I misunderstood you. I'll use __wrap-iter then

zwuis · 2025-07-02T07:38:59Z

libcxx/include/optional

+#    if _LIBCPP_STD_VER >= 26
+
+  using iterator       = __optional_iterator<_Tp>;
+  using const_iterator = __optional_iterator<const _Tp&>;


Did you mean __optional_iterator<const _Tp>?

This needs tests as well.

zwuis · 2025-07-02T07:41:16Z

libcxx/include/optional

+template <typename _Tp>
+_LIBCPP_HIDE_FROM_ABI constexpr _Tp* to_address(const __optional_iterator<_Tp>& __it) noexcept {
+  return std::to_address(__it.__it_);
+}


Can you explain why adding this function? P3168 doesn't add this this function.

Here

Note that std::contiguous_iterator requires std::to_address(iter2) == std::to_address(iter1) + std::iter_difference_t(iter2 - iter1) even if iter1 is dereferenceable and iter2 is NON-dereferenceable.

I thought I would need to add std::to_address for __optional_iterator

zwuis · 2025-07-02T07:43:31Z

libcxx/include/optional

+  _LIBCPP_HIDE_FROM_ABI constexpr iterator end() { return iterator(__wrap_iter(static_cast<_Tp*>(nullptr))); }
+
+  _LIBCPP_HIDE_FROM_ABI constexpr const_iterator end() const {
+    return const_iterator(__wrap_iter(static_cast<const _Tp*>(nullptr)));
+  }


Seems wrong after updating implementation of operator++.

libcxx/test/std/utilities/optional/optional.range/enable_ranges_view.pass.cpp

…pile.pass.cpp

github-actions · 2025-07-02T09:41:33Z

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff HEAD~1 HEAD --extensions ,cpp -- libcxx/test/std/utilities/optional/optional.range/enable_ranges_view.compile.pass.cpp libcxx/test/std/utilities/optional/optional.range/format_kind.pass.cpp libcxx/test/std/utilities/optional/optional.range/iteration.pass.cpp libcxx/test/std/utilities/optional/optional.range/iterator_types.verify.cpp libcxx/test/std/utilities/optional/optional.range/runtime_error.verify.cpp libcxx/test/std/utilities/optional/optional.range/satisfies_range_concept.verify.cpp libcxx/include/optional libcxx/include/version libcxx/test/std/language.support/support.limits/support.limits.general/optional.version.compile.pass.cpp libcxx/test/std/language.support/support.limits/support.limits.general/version.version.compile.pass.cpp

View the diff from clang-format here.

diff --git a/libcxx/include/optional b/libcxx/include/optional
index 58a0d080a..571fe49e2 100644
--- a/libcxx/include/optional
+++ b/libcxx/include/optional
@@ -630,7 +630,7 @@ public:
   _LIBCPP_HIDE_FROM_ABI constexpr __optional_iterator& operator=(const __optional_iterator&) = default;
 
   _LIBCPP_HIDE_FROM_ABI constexpr reference operator*() const { return *__it_; }
-  _LIBCPP_HIDE_FROM_ABI constexpr pointer operator->() const { return __it_.operator->(); } 
+  _LIBCPP_HIDE_FROM_ABI constexpr pointer operator->() const { return __it_.operator->(); }
 
   _LIBCPP_HIDE_FROM_ABI constexpr __optional_iterator& operator++() noexcept {
     ++__it_;

dywoq · 2025-07-02T09:44:29Z

I'm going to fix issues later, but not tomorrow

libcxx/test/std/utilities/optional/optional.range/format_kind.pass.cpp

libcxx/test/std/utilities/optional/optional.range/iteration.pass.cpp

libcxx/test/std/utilities/optional/optional.range/iterator_types.verify.cpp

libcxx/test/std/utilities/optional/optional.range/satisfies_range_concept.verify.cpp

… tests

…common_range, and contiguous_range assertions for std::optional

…e.iteration.pass Co-authored-by: A. Jiang <[email protected]>

dywoq added 14 commits June 30, 2025 22:27

[libcxx] add ranges::enable_view<optional<_T>> = true and format_kind…

4b9c76b

…<optional<_T>> = range_format::disabled

[libcxx] add forward declaration of __optional_iterator in <optional>

ebda4e4

[libcxx] add __optional_iterator

6e97c34

[libcxxx] add begin() and end() to std::optional class

ef52562

[libcxx] update includes in <optional> header

820d96c

[libcxx] add comment

5fc696b

[libcxx] remove unimplemented: True parameter in generate_feature_tes…

162170c

…t_macro_components.py

[libcxx] update <version> header

f968252

[libcxx] write optional.range tests

06109ed

[libcxx] add optional.range.runtime_error.fail test

fc75fe0

[libcxx] remove unused include

f939528

[libcxx] change path to assert_macros.h

fdc917b

[libcxx] remove .clangd

e704aba

[libcxx] rename test

f294f98

dywoq requested a review from a team as a code owner July 1, 2025 09:04

frederick-vs-ja requested changes Jul 1, 2025

View reviewed changes

dywoq and others added 2 commits July 1, 2025 14:39

[libcxx] change iterator_category to random_access_iterator_tag <opti…

a720391

…onal> Co-authored-by: A. Jiang <[email protected]>

[libcxx] remove operator!= for __optional_iterator in <optional>

4bfc9bb

Co-authored-by: A. Jiang <[email protected]>

dywoq added 2 commits July 1, 2025 15:11

[libcxx] add constexpr to __optional_iterator, optional<T>::begin and…

26974eb

… optional<T>::end

[libcxx] use assert in optional.range.iteration.pass test

b61f90b

zwuis reviewed Jul 1, 2025

View reviewed changes

dywoq added 4 commits July 1, 2025 15:33

[libcxx] use static_assert in optional.range.satisfies_range_concept.…

6993061

…pass test

[libcxx] add additional operators to __optional_iterator

a3ef155

[libcxx] remove .clangd

6d4be21

[libcxx] add iterator_types.verify test to optional.ranges

47ba868

dywoq requested a review from frederick-vs-ja July 1, 2025 13:33

philnik777 requested changes Jul 1, 2025

View reviewed changes

dywoq added 7 commits July 1, 2025 16:37

[libcxx] add missing new character line at EOF

3f69eda

[libcxx] change optional.range.iteration.pass test to constant eveluated

ef8af0a

[libcxx] use __wrap_iter in __optional_iterator

04b5511

[libcxx] add missing comment

3f6f457

[libcxx] add tests enable_ranges_view.pass and format_kind.pass in op…

132bc39

…tional.range

[libcxx] remove .clangd

8c4d4df

[libcxx] add friend declarations for __wrap_iter in __optional_iterator

ef032f1

zwuis reviewed Jul 1, 2025

View reviewed changes

dywoq added 2 commits July 1, 2025 18:11

[libcxx] update const_iterator type to use const reference in optional

149ff9e

[libcxx] simplify __optional_iterator operations and add to_address u…

0510945

…tility

zwuis reviewed Jul 2, 2025

View reviewed changes

[libcxx] rename enable_ranges_view.pass.cpp to enable_ranges_view.com…

5be2e9d

…pile.pass.cpp

frederick-vs-ja requested changes Jul 2, 2025

View reviewed changes

dywoq and others added 7 commits July 2, 2025 18:38

[libcxx] rename optional.range tests

e34c3bc

[libcxx] remove main function blocks in optional.range.*.compile.pass…

fc0219e

… tests

[libcxx] update satisfies_range_concept test to include sized_range, …

b0393d6

…common_range, and contiguous_range assertions for std::optional

[libcxx] remove __optional_iterator class

d66ea58

[libcxx] refactor iterator type assertions in optional.range

42de74b

[libcxx] use test() along with static_assert(test()) in optional.rang…

99fb189

…e.iteration.pass Co-authored-by: A. Jiang <[email protected]>

[libcxx] correct iterator type to use pointer types in optional

02cd53f

dywoq closed this by deleting the head repository Oct 11, 2025

[libc++] P3168R2 Give std::optional Range Support #146491

[libc++] P3168R2 Give std::optional Range Support #146491

Uh oh!

Conversation

dywoq commented Jul 1, 2025

Uh oh!

github-actions bot commented Jul 1, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dywoq commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zwuis commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dywoq commented Jul 2, 2025

Uh oh!

zwuis commented Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dywoq commented Jul 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Jul 2, 2025

Uh oh!

dywoq commented Jul 2, 2025

Uh oh!

Uh oh!

Uh oh!

dywoq commented Jul 1, 2025 •

edited

Loading

zwuis commented Jul 1, 2025 •

edited

Loading

zwuis commented Jul 2, 2025 •

edited

Loading