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

[Louis Dionne via Phabricator via llvm-commits]

ldionne added a comment.

Regarding whether we want to enable or disable those warnings by default: it seems like we've reached consensus (on the LLVM IRC) for enabling the deprecation warnings by default. However, I suggest we keep them disabled by default in this commit, and then change the default in a subsequent change. This will make things easier from an integration perspective, and also in case we need to roll back the defaults because of some unforeseen circumstances.



================
Comment at: libcxx/test/libcxx/algorithms/alg.modifying.operations/alg.random.shuffle/random_shuffle.cxx1z.pass.cpp:26
 
+#ifdef __clang__
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
----------------
EricWF wrote:
> ldionne wrote:
> > EricWF wrote:
> > > How do these tests work with compilers other than Clang?
> > > 
> > > Perhaps you can add a `disable_deprecated_declarations.h` header, and include that instead? That way the one header can be made to handle most compilers.
> > They must always break because we compile with `-Wall -Werror`. I thought the tests were only supposed to work with Clang -- I copied this pattern from another test (`test/libcxx/containers/gnu_cxx/hash_map.pass.cpp`).
> > 
> > Per Marshall's comment, those diagnostics will be disabled by default, so I won't need this header (or this pragma) anymore. I'll see if it makes sense to add the header for the other tests that depend on this pragma, and if so, I'll add it in another Phab review.
> > 
> The tests are actually expected to work with GCC, Clang, and MSVC (to varying degrees).
> 
> The MSVC team actually uses our test suite to test their STL, and so all tests under `test/std` should be written with that in mind.
> However, tests under `test/libcxx`, as these are, only need to work against libc++ (but with both GCC and Clang except where it's not possible).
> 
> After investigating `hash_map.pass.cpp` it seems to be by accident that the test doesn't fail under GCC. GCC doesn't promote `#warning` to an error even when `-Werror` is specified. Additionally `-W#warnings` is a Clang specific flag. GCC's version is `-Wcpp`.
> 
> However, these tests use `[[deprecated]]` not `#warning` to produce the diagnostic, and so must be made to work with GCC.
GCC does not accept `[[deprecated]]` when mixed with another `__attribute__` specifier, which is problematic because most types/functions already have such attributes applied to them for visibility purposes. However, note that applying multiple `__attribute__` specifiers seems to work. I'm not sure what to do here -- it seems like the easiest way forward would be to detect when we support `__attribute__((deprecated))` on both Clang and GCC, and use that when available. Does that sound like an acceptable way forward?


Repository:
  rL LLVM

https://reviews.llvm.org/D48912