[libcxx-commits] [PATCH] D103171: [libc++] Deprecate std::iterator and remove it as a base class

Louis Dionne via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu May 27 08:31:47 PDT 2021

ldionne added inline comments.

Comment at: libcxx/include/__memory/raw_storage_iterator.h:45
+    typedef void                pointer;
+    typedef void                reference;
Quuxplusone wrote:
> ldionne wrote:
> > Mordante wrote:
> > > Is it intended to change the types of these typedefs and the base class?
> > Yes, I'm making it standards conforming. I'll add a test too, it seems like we didn't have any.
> This is also, technically, an ABI break ;) but you're right, it fixes a bug that's apparently been undetected forever.
I agree, but I don't care about this one. I think literally nobody is ever going to be affected.

Comment at: libcxx/include/__memory/raw_storage_iterator.h:33-35
+    : public iterator<output_iterator_tag, void, void, void, void>
Quuxplusone wrote:
> Actually, one last thing I don't understand: What's the purpose of `_LIBCPP_SUPPRESS_DEPRECATED_PUSH` here, given that this entire file is already controlled by `#pragma GCC system_header`? Do we hit situations where we're actually seeing deprecation diagnostics from this and other system headers?
> I know you didn't invent `_LIBCPP_SUPPRESS_DEPRECATED_PUSH`, so it must be solving //some// problem, but I don't get it.
We disable `#pragma GCC system_header` when we run the test suite to avoid missing some potentially important warnings/errors, so we need to disable this warning locally to avoid failing the test suite.

Comment at: libcxx/test/std/iterators/stream.iterators/istream.iterator/types.pass.cpp:35
Quuxplusone wrote:
> I think you don't need this because `std::iterator` wasn't deprecated <=14 and now you're mentioning it only under `#if TEST_STD_VER <= 14`.
> Presumably ditto in some other files as well.
Good catch, thanks.

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list