[libcxx-commits] [PATCH] D146675: [libc++] Warn on including headers that are deprecated in C++17

[Alan Zhao via Phabricator via libcxx-commits]

ayzhao added a comment.

In D146675#4215557 <https://reviews.llvm.org/D146675#4215557>, @thakis wrote:

> Ms stl uses this construction to make the warning grouped under -Wdeprecated-declarations: https://github.com/search?q=repo%3Amicrosoft%2FSTL%20_CXX17_DEPRECATE_C_HEADER&type=code
>
> Many worth doing?

The problem with this approach (and with `#pragma message`, as I discovered earlier), is that the warning doesn't show up if the path to libc++ is passed via `-isystem`:

  $ cat /tmp/llvm/include/c++/v1/cstdbool
  // -*- C++ -*-
  //===----------------------------------------------------------------------===//
  //
  // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
  // See https://llvm.org/LICENSE.txt for license information.
  // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
  //
  //===----------------------------------------------------------------------===//
  
  #ifndef _LIBCPP_CSTDBOOL
  #define _LIBCPP_CSTDBOOL
  
  /*
      cstdbool synopsis
  
  Macros:
  
      __bool_true_false_are_defined
  
  */
  
  #include <__assert> // all public C++ headers provide the assertion handler
  #include <__config>
  
  using _HEADER_cstdbool [[deprecated("hello, world")]] = int;
  using _Hdr_cstdbool = _HEADER_cstdbool ;
  
  #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
  #  pragma GCC system_header
  #endif
  
  #undef __bool_true_false_are_defined
  #define __bool_true_false_are_defined 1
  
  #endif // _LIBCPP_CSTDBOOL
  
  $ cat include-deprecated.cc
  #include <cstdbool>
  
  $ clang++ -nostdinc++ -isystem/tmp/llvm/include/c++/v1 -c -o /dev/null include-deprecated.cc -std=c++20 -Wdeprecated-declarations
  (nothing shows up)

@cjdb suggested implementing this with `#warning`, but I'll leave it up to the libc++ maintainers to decide on the best way forward. Any thoughts @philnik ?



================
Comment at: libcxx/include/complex.h:27
 #ifdef __cplusplus
-#  include <ccomplex>
+#  include <__assert>
+#  include <complex>
----------------
philnik wrote:
> Why do we need this include now?
Removed. I included it here since `<ccomplex>` originally included it, but now that I look at it, it seems like it doesn't matter.


================
Comment at: libcxx/test/libcxx/language.support/support.runtime/version_cstdbool.pass.cpp:11
 
+// REQUIRES: c++11 || c++0x || c++14 || c++1y
+// XFAIL: c++17
----------------
philnik wrote:
> Where did you get c++0x and c++1y from? I can't find anything in the code base.
These are mentioned in `params.py`: https://github.com/llvm/llvm-project/blob/de939c6cd80c1e88913f1ef12be11aea501eb89b/libcxx/utils/libcxx/test/params.py#L60-L61


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D146675/new/

https://reviews.llvm.org/D146675