[libcxx-commits] [PATCH] D101729: [libcxx] deprecates `std::iterator` and removes it as a base class

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed May 5 10:28:30 PDT 2021

ldionne requested changes to this revision.
ldionne added a comment.

Generally looks good, thanks for doing this cleanup. I agree we should unconditionally define the various typedefs everywhere to remove some `#if`s.

Comment at: libcxx/include/__memory/raw_storage_iterator.h:41-47
+#if _LIBCPP_STD_VER > 14
+  using iterator_category = output_iterator_tag;
+  using value_type = void;
+  using difference_type = void;
+  using pointer = void;
+  using reference = void;
Quuxplusone wrote:
> Here, we //could// drop the `#if` and just unconditionally define these members... except that that would visibly change the value of `raw_storage_iterator::difference_type` (from non-conforming to conforming), and maybe we don't want that? But again I'd think we don't really care about this type and so we might as well lose the `#if`.
I think it's an acceptable change - I doubt it will break many users since `raw_storage_iterator` isn't widely used, and it's a compile-time breakage in all cases, so not some nasty runtime issue.

Comment at: libcxx/include/iterator:85
+struct iterator                                            // until C++17
+struct [[deprecated)]] iterator                            // since C++17
cjdb wrote:
> Quuxplusone wrote:
> > Stray `)` here. But please remove this diff anyway; `[[deprecated]]` doesn't appear in the Standard and even if it did I don't think it needs to appear in the synopsis comments.
> Let's take this one to Discord, I have some thoughts on this.
Please use `// deprecated in C++17` instead -- that is what we do elsewhere.

Comment at: libcxx/test/libcxx/iterators/iterator.requirements/iterator.concepts/cpp20_iter_concepts.pass.cpp:11-12
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
Why do we need this? Aren't we removing the use of those deprecated decls?

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list