[PATCH] D48912: [libc++] Add deprecated attributes to many deprecated components

[Louis Dionne via Phabricator via llvm-commits]

ldionne marked an inline comment as done.
ldionne added a comment.

I still think this is good to go as-is, given my answer to JF's comments.



================
Comment at: libcxx/docs/UsingLibcxx.rst:212
+  will trigger a warning saying that `std::auto_ptr` is deprecated. By default,
+  this macro is not defined.
+
----------------
jfb wrote:
> I thought you'd agreed on IRC to have it defined by default (i.e. enable deprecation warnings by default)?
> 
> On-by-default sounds right to me, since that's what the Standard is effectively asking for. I agree we'll make some developers sad, but they'll be even more sad when deprecated features get removed entirely.
The plan was to do this in a follow-up patch. This patch is adding the attributes but not changing behavior, and the followup would only change the default behavior. This is because enabling this by default will require more changes (and thus a larger diff), since some tests will need fixing up, etc.


================
Comment at: libcxx/include/__config:1006
+#  define _LIBCPP_DEPRECATED_IN_CXX17
+#endif
+
----------------
jfb wrote:
> I assume all of the above will work with deleted things because they'll be behind a macro that `#if 0`'s them when deleted (and then a user can turn the deleted feature back on with another macro?). Or do we just delete things outright?
By deleted, you mean removed from the standard? If so, then yes, those attributes are applied to a type/function directly. When a type/function is not declared/defined because it's been removed from the standard, the whole thing is behind a `#if 0`.


================
Comment at: libcxx/include/memory:2059
 template <class _Tp>
-struct auto_ptr_ref
+struct _LIBCPP_DEPRECATED_IN_CXX11 auto_ptr_ref
 {
----------------
jfb wrote:
> So here's an example where, if you define `_LIBCPP_ENABLE_CXX17_REMOVED_AUTO_PTR` you can come back and get `auto_ptr` and then you get the deprecated warning. Should we have another macro that says (for this one) "deprecated in 11, removed in 17 but you asked to get it back, srlys why?"
Ah, I see what you might have meant above. In the current state of things, you'll get the deprecated warning even if you define `_LIBCPP_ENABLE_CXX17_REMOVED_AUTO_PTR`. I think that's the correct thing to do. If you want to get rid of the warnings, just define `_LIBCPP_DISABLE_DEPRECATION_WARNINGS` (in the future, after the default has been changed). We could do otherwise, but I don't see a strong case for it, and it will complicate things quite a bit because each macro like `_LIBCPP_ENABLE_CXX17_REMOVED_AUTO_PTR` will need its own deprecation macro.


Repository:
  rL LLVM

https://reviews.llvm.org/D48912