[libcxx-commits] [PATCH] D118616: [libc++] Use -I instead of -isystem to include headers in the test suite

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Mon Feb 28 15:53:45 PST 2022


Quuxplusone added inline comments.


================
Comment at: libcxx/include/__locale:24-25
 #if defined(_LIBCPP_MSVCRT_LIKE)
-# include <cstring>
 # include <__support/win32/locale_win32.h>
+# include <cstring>
 #elif defined(_AIX) || defined(__MVS__)
----------------
I don't think you should touch the order of these two lines (because I'm paranoid). But if your goal is to alphabetize the includes in this file, then you should also touch lines 31-32 at the same time.


================
Comment at: libcxx/include/__ranges/reverse_view.h:113-129
     template<class _Tp>
-    constexpr bool __is_reverse_view = false;
+    inline constexpr bool __is_reverse_view = false;
 
     template<class _Tp>
-    constexpr bool __is_reverse_view<reverse_view<_Tp>> = true;
+    inline constexpr bool __is_reverse_view<reverse_view<_Tp>> = true;
 
     template<class _Tp>
----------------
Quuxplusone wrote:
> These changes LGTM but should be committed separately from whatever else is going on here.
> You can just say "Reviewed as part of D118616". Or I'll do it myself if I get around to it tomorrow morning. :)
I finally remembered this comment, and landed these six lines in 6d751c410d6f5400c2c26d47e5033679fbd736d6. Please rebase. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118616



More information about the libcxx-commits mailing list