[libc++] Use -I instead of -isystem to include headers in the test suite

[philnik]

Summary

Using -isystem marks the headers as system headers, which means that we
don't actually get all the warnings that we'd normally get if we included
the headers as user headers.

The goal of the test suite is normally to mirror as closely as possible
how users would use the library. Technically, this change goes against
that philosophy, since users should be using -isystem (if they ever
need to specify the libc++ path explicitly, which should be a rare
occurence). However, I believe fishing out additional warnings from
the headers provides more value, hence this change. Ideally, we'd be
able to still use -isystem, but instruct Clang to still emit warnings
from the libc++ headers (e.g. we could tell Clang to emit warnings in
any file inside <...>/usr/include/c++/v1).

[jloser]

Ideally, we'd be able to still use -isystem, but instruct Clang to still emit warnings from the libc++ headers (e.g. we could tell Clang to emit warnings in any file inside <...>/usr/include/c++/v1).

I think that this can be done with --no-system-header-prefix=path/to/usr/include/c++/v1 FWIW. Note that while Clang supports this, GCC does not for example.

In any case, I like the ideas for this patch; I'm fine with your current solution.

[ldionne]

In D118616#3284245, @jloser wrote:

Ideally, we'd be able to still use -isystem, but instruct Clang to still emit warnings from the libc++ headers (e.g. we could tell Clang to emit warnings in any file inside <...>/usr/include/c++/v1).

I think that this can be done with --no-system-header-prefix=path/to/usr/include/c++/v1 FWIW. Note that while Clang supports this, GCC does not for example.

In any case, I like the ideas for this patch; I'm fine with your current solution.

Thanks a lot for the information, I wasn't aware of that option at all. This is quite nice.

However, I just had a look into using that option and it looks like it doesn't help us at all. Indeed, the documentation says:

The –system-header-prefix= and –no-system-header-prefix= command-line arguments can be used to override whether subsets of an include path are treated as system headers. When the name in a #include directive is found within a header search path and starts with a system prefix, the header is treated as a system header.

Since we always include libc++ headers without any prefix in the #include <...> directive, this doesn't help. I would have imagined that doing something like --no-system-header-prefix=<...>/include/c++/v1 would do the trick, but trying it out confirms that it would only work if we included libc++ headers from the test suite as #include <include/c++/v1/chrono> for example.

[ldionne]

This is the alternative patch I tried, and it didn't work as one would expect:

commit 09058c0c732ea895def9f3e08b08672b2d5f1bc9
Author: Louis Dionne <[e-mail]>
Date:   Mon Jan 31 11:14:07 2022 -0500

    [libc++] Don't treat libc++ headers as system headers in the test suite
    
    This makes it possible to fish out warnings from our headers inside the
    test suite without having to use `-I` instead of `-isystem` to include
    those headers. This patch should pretty much negate the need for
    _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER, which was only used to enable
    these warnings in the test suite and when building the library.
    
    Differential Revision: https://reviews.llvm.org/D118616

diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index 429e2d82c97b..2c5837075f4a 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -42,6 +42,8 @@ DEFAULT_FEATURES = [
                                                                  sys.platform.lower().strip() == 'darwin'), # TODO: this doesn't handle cross-compiling to Apple platforms.
   Feature(name='objective-c++',                 when=lambda cfg: hasCompileFlag(cfg, '-xobjective-c++ -fobjc-arc')),
   Feature(name='verify-support',                when=lambda cfg: hasCompileFlag(cfg, '-Xclang -verify-ignore-unexpected')),
+  Feature(name='warnings-in-system-headers',    when=lambda cfg: hasCompileFlag(cfg, '--no-system-header-prefix=%{include}'),
+                                                actions=[AddCompileFlag('--no-system-header-prefix=%{include}')]),
 
   Feature(name='non-lockfree-atomics',
           when=lambda cfg: sourceBuilds(cfg, """
@@ -89,8 +91,7 @@ DEFAULT_FEATURES = [
   Feature(name=lambda cfg: 'apple-clang-{__clang_major__}.{__clang_minor__}'.format(**compilerMacros(cfg)),                        when=_isAppleClang),
   Feature(name=lambda cfg: 'apple-clang-{__clang_major__}.{__clang_minor__}.{__clang_patchlevel__}'.format(**compilerMacros(cfg)), when=_isAppleClang),
 
-  Feature(name='clang',                                                                                                            when=_isClang,
-          actions=[AddCompileFlag('-D_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER')]),
+  Feature(name='clang',                                                                                                            when=_isClang),
   Feature(name=lambda cfg: 'clang-{__clang_major__}'.format(**compilerMacros(cfg)),                                                when=_isClang),
   Feature(name=lambda cfg: 'clang-{__clang_major__}.{__clang_minor__}'.format(**compilerMacros(cfg)),                              when=_isClang),
   Feature(name=lambda cfg: 'clang-{__clang_major__}.{__clang_minor__}.{__clang_patchlevel__}'.format(**compilerMacros(cfg)),       when=_isClang),

[ldionne]

Workaround failure on AIX

[ldionne]

Try to fix the failure differently.

[philnik]

Commandeering to fix clang-tidy, and Louis is not available at the moment.

[philnik]

• Also fix clang-tidy in this patch

[philnik]

• Fix modules build

[philnik]

• Fix CI

[philnik]

• Hopefully fix Windows CI

[philnik]

• Next try to fix Windows

[philnik]

• And another try

[philnik]

• Maybe this works

[mstorsjo]

libcxx/include/__config_site.in

39 ↗

(On Diff #410084)

For the cases where this was needed, the safest fix would be to make sure to include libcxx headers before standard c headers. Or alternatively we could maybe consider if _LIBCPP_EXTRA_SITE_DEFINES should add an #undef before each define.

[EricWF]

Wait... How long have we been using -isystem in the test suite. That suppresses all warnings in the headers along that path.

[• Quuxplusone]

LGTM mod comments, but I consider this to be @ldionne's jurisdiction, and it certainly doesn't seem super urgent, so I think we should just wait for him to take a look.

Can you explain in the commit message how this relates to clang-tidy? Even looking at https://reviews.llvm.org/D118616?vs=404892&id=409993#toc I don't really understand your comment "Also fix clang-tidy in this patch."

libcxx/include/__config

94–96

Then personally I'd prefer to pass -Wno-macro-redefined in those specific tests. We shouldn't do anything in <__config> that is beneficial only for certain tests in the test suite; we should do it here iff we think it'll benefit real users. Do we want real users to be able to do, like, -D_LIBCPP_ABI_ALTERNATE_STRING_LAYOUT=1? If so, then lines 95-96 are good but the comment on line 94 should talk about that, not about tests in the test suite.

libcxx/include/__iterator/iterator_traits.h

201

Please file a bug for this (I assume a clang-tidy bug, right?) and link it from here and/or in the commit message.

libcxx/include/__ranges/reverse_view.h

113–129

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. :)

libcxx/include/ext/__hash

19–22

Here and below, this shouldn't be necessary. Remember the economist's $100 bill: if this were needed, why wouldn't we be doing it everywhere? (E.g., in <string>.) It seems likely that there's some master list of "libc++ headers" somewhere and that <ext/__hash> and <ext/hash_map> are accidentally missing from that list.

libcxx/test/std/containers/sequences/array/size_and_alignment.pass.cpp

16

I might think this also needs to be guarded by && defined(__clang__), but I suppose we never test any non-Clang compilers on AIX. Okay.

[philnik]

In D118616#3333754, @Quuxplusone wrote:

LGTM mod comments, but I consider this to be @ldionne's jurisdiction, and it certainly doesn't seem super urgent, so I think we should just wait for him to take a look.

Can you explain in the commit message how this relates to clang-tidy? Even looking at https://reviews.llvm.org/D118616?vs=404892&id=409993#toc I don't really understand your comment "Also fix clang-tidy in this patch."

Louis opened this PR before I landed the clang-tidy PR and this happens to be the fix for clang-tidy ignoring the headers. With the "Also fix clang-tidy in this patch" I meant that I'm fixing the warnings that clang-tidy produced after rebasing. I'm not sure what to add to the commit message. It's exactly the same problem, only that it's not clang producing the warnings, but clang-tidy.

libcxx/include/__config_site.in

39 ↗

(On Diff #410084)

I don't think the first option makes sense. That would mean that it makes a difference in which order the headers are included, which I'm not a fan of. The second one is an option, but I didn't have any luck with it yet.

libcxx/include/__iterator/iterator_traits.h

201

clang-tidy is kind-of right. __i - __i is a redundant expression, and I had to look at it a few times before I got why it's correct in this case. Where else would subtracting something from itself make sense?

libcxx/include/ext/__hash

19–22

I thought the <ext/*> headers weren't in module.modulemap, because they were deprecated. It's also possible they are not in there by accident.

[Mordante]

I didn't look at the patch closely.

libcxx/include/__iterator/iterator_traits.h

201

I agree with Arthur, this seems like a bug. clang-tidy uses the AST so it knows the code is in a requires clause.

I have a strong preference to at least mention the bug here, that makes validating whether it's fixed in the future easier. I don't object to adding it to the commit message.

[ldionne]

Thanks for commandeering. Can you also change the testing configurations in libcxxabi/test/configs/?

libcxx/include/__config

94–96

+1 here, we should just add those suppressions in the tests that need them

[philnik]

• Address comments

[ldionne]

libcxx/include/ext/__hash

19–22

I don't understand why this diff is necessary in this patch. What happens if you remove it? What fails?

[philnik]

libcxx/include/ext/__hash

19–22

Because we replace -isystem with -I. All warnings in headers are now part of the tests, so there must not be any warnings because of -Werror. So basically, the modules build would fail.

[• Quuxplusone]

libcxx/include/__locale

24–26

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.

libcxx/include/__ranges/reverse_view.h

113–129

I finally remembered this comment, and landed these six lines in 6d751c410d6f5400c2c26d47e5033679fbd736d6. Please rebase. :)

[EricWF]

libcxx/include/ext/__hash

19–22

But there's a #pragma system_header?

[• Quuxplusone]

libcxx/include/ext/__hash

19–22

I thought the <ext/*> headers weren't in module.modulemap, because they were deprecated. It's also possible they are not in there by accident.

I hypothesize that they're missing only by accident. I've posted D120758, and I'd be interested to see if you can eliminate these diffs by rebasing on it (or just adopting its diffs into this PR; I'll be happy to abandon it).

[philnik]

• Rebased
• Address comments
• Exclude ext/ from modules build

Adding the things in ext/ to the modulemap doesn't work, because it would ignore __DEPRECATED.

[• Quuxplusone]

libcxx/include/ext/__hash

13

I notice this is not guarded correctly by _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER. We should fix that, but probably fix it after the dust has settled on all these reviews.

libcxx/utils/libcxx/test/params.py

80 ↗

(On Diff #412460)

I think this macro should have the word TEST in it somewhere.

However, since you use it only for the generated header tests, IMHO it would be quite reasonable to just rip out the ext/ headers from those generated tests specifically (i.e. adopt a small portion of D120831's changes), and then you don't need the macro at all, right?

[ldionne]

This LGTM if CI is passing, however I would much rather rip out the ext/ headers from tests and avoid introducing the _LIBCXX_MODULES_BUILD macro.

FWIW, this is going to interact with D120684 almost certainly. The second one to land his patch is probably going to have additional work to do. If CI passes on your patch, I don't mind you shipping it as-is, and then tackling the removal of _LIBCXX_MODULES_BUILD separately, just for the sake of making progress on this (and allowing me to start fixing any remaining issues with my patch).

[philnik]

• Disable some warnings in bad_engine tests

[philnik]

• Disable warnings in trivial_abi tests

libcxx/.clang-tidy

 InheritParentConfig: true
-Checks: '-readability-identifier-naming,-llvm-header-guard,-misc-unconventional-assign-operator,-llvm-else-after-return'
+Checks: '-readability-identifier-naming,-llvm-header-guard,-misc-unconventional-assign-operator,-llvm-else-after-return,-llvm-qualified-auto'

libcxx/CMakeLists.txt

if (LIBCXX_ABI_DEFINES)
   config_define(${abi_defines} _LIBCPP_ABI_DEFINES)
 endif()
 
 if (LIBCXX_EXTRA_SITE_DEFINES)
   set(extra_site_defines)
   foreach (extra_site_define ${LIBCXX_EXTRA_SITE_DEFINES})
     # Allow defines such as DEFINE=VAL, transformed into "#define DEFINE VAL".
     string(REPLACE "=" " " extra_site_define "${extra_site_define}")
-    list(APPEND extra_site_defines "#define ${extra_site_define}")
+    list(APPEND extra_site_defines "#ifdef ${extra_site_define};#  undef ${extra_site_define};#endif;#define ${extra_site_define}")
   endforeach()
   string(REPLACE ";" "\n" extra_site_defines "${extra_site_defines}")
   config_define(${extra_site_defines} _LIBCPP_EXTRA_SITE_DEFINES)
 endif()
 
 # By default libc++ on Windows expects to use a shared library, which requires
 # the headers to use DLL import/export semantics. However when building a
 # static library only we modify the headers to disable DLL import/export.

libcxx/include/__config

 #elif defined(_WIN32)
 #  define _LIBCPP_OBJECT_FORMAT_COFF  1
 #elif defined(__wasm__)
 #  define _LIBCPP_OBJECT_FORMAT_WASM  1
 #else
    // ... add new file formats here ...
 #endif
 
+#if defined(__apple_build_version__)
+#  define _LIBCPP_COMPILER_CLANG_BASED
+#  define _LIBCPP_APPLE_CLANG_VER (__apple_build_version__ / 10000)
+#elif defined(__clang__)
+#  define _LIBCPP_COMPILER_CLANG_BASED
+#  define _LIBCPP_CLANG_VER (__clang_major__ * 100 + __clang_minor__)
+#elif defined(__GNUC__)
+#  define _LIBCPP_COMPILER_GCC
+#elif defined(_MSC_VER)
+#  define _LIBCPP_COMPILER_MSVC
+#elif defined(__IBMCPP__)
+#  define _LIBCPP_COMPILER_IBM
+#endif
+
+#define _LIBCPP_TOSTRING2(x) #x
+#define _LIBCPP_TOSTRING(x) _LIBCPP_TOSTRING2(x)
+
+#ifdef _LIBCPP_COMPILER_CLANG_BASED
+#  define _LIBCPP_DIAGNOSTIC_PUSH _Pragma("clang diagnostic push")
+#  define _LIBCPP_DIAGNOSTIC_POP _Pragma("clang diagnostic pop")
+#  define _LIBCPP_CLANG_DIAGNOSTIC_IGNORED(str) _Pragma(_LIBCPP_TOSTRING(clang diagnostic ignored str))
+#  define _LIBCPP_GCC_DIAGNOSTIC_IGNORED(str)
+#elif defined(_LIBCPP_COMPILER_GCC)
+#  define _LIBCPP_DIAGNOSTIC_PUSH _Pragma("GCC diagnostic push")
+#  define _LIBCPP_DIAGNOSTIC_POP _Pragma("GCC diagnostic pop")
+#  define _LIBCPP_CLANG_DIAGNOSTIC_IGNORED(str)
+#  define _LIBCPP_GCC_DIAGNOSTIC_IGNORED(str) _Pragma(_LIBCPP_TOSTRING(GCC diagnostic ignored str))
+#else
+#  define _LIBCPP_DIAGNOSTIC_PUSH
+#  define _LIBCPP_DIAGNOSTIC_POP
+#  define _LIBCPP_CLANG_DIAGNOSTIC_IGNORED(str)
+#  define _LIBCPP_GCC_DIAGNOSTIC_IGNORED(str)
+#endif
+
 #if _LIBCPP_ABI_VERSION >= 2
+// These macros are defined in some tests
+_LIBCPP_DIAGNOSTIC_PUSH
+_LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wmacro-redefined")
+
 // Change short string representation so that string data starts at offset 0,
 // improving its alignment in some cases.
 #  define _LIBCPP_ABI_ALTERNATE_STRING_LAYOUT
 // Fix deque iterator type in order to support incomplete types.
 #  define _LIBCPP_ABI_INCOMPLETE_TYPES_IN_DEQUE
 // Fix undefined behavior in how std::list stores its linked nodes.
 #  define _LIBCPP_ABI_LIST_REMOVE_NODE_POINTER_UB
 // Fix undefined behavior in  how __tree stores its end and parent nodes.

 // std::random_device, one needs to retain the same object layout to remain ABI
 // compatible. This switch removes these workarounds for platforms that don't care
 // about ABI compatibility.
 #  define _LIBCPP_ABI_NO_RANDOM_DEVICE_COMPATIBILITY_LAYOUT
 // Remove basic_string common base
 #  define _LIBCPP_ABI_DO_NOT_EXPORT_BASIC_STRING_COMMON
 // Remove vector base class
 #  define _LIBCPP_ABI_DO_NOT_EXPORT_VECTOR_BASE_COMMON
+
+_LIBCPP_DIAGNOSTIC_POP
 #elif _LIBCPP_ABI_VERSION == 1
 #  if !defined(_LIBCPP_OBJECT_FORMAT_COFF)
 // Enable compiling copies of now inline methods into the dylib to support
 // applications compiled against older libraries. This is unnecessary with
 // COFF dllexport semantics, since dllexport forces a non-inline definition
 // of inline functions to be emitted anyway. Our own non-inline copy would
 // conflict with the dllexport-emitted copy, so we disable it.
 #    define _LIBCPP_DEPRECATED_ABI_LEGACY_LIBRARY_DEFINITIONS_FOR_INLINE_FUNCTIONS

 # define _LIBCPP_ABI_ENABLE_ADDITIONAL_IOSTREAM_EXPLICIT_INSTANTIATIONS_1
 
 // Define a key function for `bad_function_call` in the library, to centralize
 // its vtable and typeinfo to libc++ rather than having all other libraries
 // using that class define their own copies.
 #  define _LIBCPP_ABI_BAD_FUNCTION_CALL_KEY_FUNCTION
 #endif
 
-#define _LIBCPP_TOSTRING2(x) #x
-#define _LIBCPP_TOSTRING(x) _LIBCPP_TOSTRING2(x)
-
 #if __cplusplus < 201103L
 #define _LIBCPP_CXX03_LANG
 #endif
 
 #ifndef __has_attribute
 #define __has_attribute(__x) 0
 #endif
 

 #endif
 
 #define __has_keyword(__x) !(__is_identifier(__x))
 
 #ifndef __has_include
 #define __has_include(...) 0
 #endif
 
-#if defined(__apple_build_version__)
-#  define _LIBCPP_COMPILER_CLANG_BASED
-#  define _LIBCPP_APPLE_CLANG_VER (__apple_build_version__ / 10000)
-#elif defined(__clang__)
-#  define _LIBCPP_COMPILER_CLANG_BASED
-#  define _LIBCPP_CLANG_VER (__clang_major__ * 100 + __clang_minor__)
-#elif defined(__GNUC__)
-#  define _LIBCPP_COMPILER_GCC
-#elif defined(_MSC_VER)
-#  define _LIBCPP_COMPILER_MSVC
-#elif defined(__IBMCPP__)
-#  define _LIBCPP_COMPILER_IBM
-#endif
-
 #if defined(_LIBCPP_COMPILER_GCC) && __cplusplus < 201103L
 #error "libc++ does not support using GCC with C++03. Please enable C++11"
 #endif
 
 // FIXME: ABI detection should be done via compiler builtin macros. This
 // is just a placeholder until Clang implements such macros. For now assume
 // that Windows compilers pretending to be MSVC++ target the Microsoft ABI,
 // and allow the user to explicitly specify the ABI to handle cases where this
#  define _LIBCPP_NO_UNIQUE_ADDRESS /* nothing */
    // Note that this can be replaced by #error as soon as clang-cl
    // implements msvc::no_unique_address, since there should be no C++20
    // compiler that doesn't support one of the two attributes at that point.
    // We generally don't want to use this macro outside of C++20-only code,
    // because using it conditionally in one language version only would make
    // the ABI inconsistent.
 #endif
 
-#ifdef _LIBCPP_COMPILER_CLANG_BASED
-#  define _LIBCPP_DIAGNOSTIC_PUSH _Pragma("clang diagnostic push")
-#  define _LIBCPP_DIAGNOSTIC_POP _Pragma("clang diagnostic pop")
-#  define _LIBCPP_CLANG_DIAGNOSTIC_IGNORED(str) _Pragma(_LIBCPP_TOSTRING(clang diagnostic ignored str))
-#  define _LIBCPP_GCC_DIAGNOSTIC_IGNORED(str)
-#elif defined(_LIBCPP_COMPILER_GCC)
-#  define _LIBCPP_DIAGNOSTIC_PUSH _Pragma("GCC diagnostic push")
-#  define _LIBCPP_DIAGNOSTIC_POP _Pragma("GCC diagnostic pop")
-#  define _LIBCPP_CLANG_DIAGNOSTIC_IGNORED(str)
-#  define _LIBCPP_GCC_DIAGNOSTIC_IGNORED(str) _Pragma(_LIBCPP_TOSTRING(GCC diagnostic ignored str))
-#else
-#  define _LIBCPP_DIAGNOSTIC_PUSH
-#  define _LIBCPP_DIAGNOSTIC_POP
-#  define _LIBCPP_CLANG_DIAGNOSTIC_IGNORED(str)
-#  define _LIBCPP_GCC_DIAGNOSTIC_IGNORED(str)
-#endif
-
 #endif // __cplusplus
 
 #endif // _LIBCPP_CONFIG

libcxx/include/__iterator/iterator_traits.h

concept __cpp17_random_access_iterator =
   __cpp17_bidirectional_iterator<_Ip> &&
   totally_ordered<_Ip> &&
   requires(_Ip __i, typename incrementable_traits<_Ip>::difference_type __n) {
     { __i += __n } -> same_as<_Ip&>;
     { __i -= __n } -> same_as<_Ip&>;
     { __i +  __n } -> same_as<_Ip>;
     { __n +  __i } -> same_as<_Ip>;
     { __i -  __n } -> same_as<_Ip>;
-    { __i -  __i } -> same_as<decltype(__n)>;
+    { __i -  __i } -> same_as<decltype(__n)>; // NOLINT(misc-redundant-expression)
     {  __i[__n]  } -> convertible_to<iter_reference_t<_Ip>>;
   };
 } // namespace __iterator_traits_detail
 
 template<class _Ip>
 concept __has_member_reference = requires { typename _Ip::reference; };
 
 template<class _Ip>

libcxx/include/__locale

 #include <cstdint>
 #include <locale.h>
 #include <memory>
 #include <mutex>
 #include <string>
 #include <utility>
 
 #if defined(_LIBCPP_MSVCRT_LIKE)
-# include <cstring>
 # include <__support/win32/locale_win32.h>
+# include <cstring>
 #elif defined(_AIX) || defined(__MVS__)
 # include <__support/ibm/xlocale.h>
 #elif defined(__ANDROID__)
 # include <__support/android/locale_bionic.h>
 #elif defined(__sun__)
 # include <xlocale.h>
 # include <__support/solaris/xlocale.h>
 #elif defined(_NEWLIB_VERSION)
 # include <__support/newlib/xlocale.h>

libcxx/include/__ranges/reverse_view.h

namespace ranges {
   template<class _Range>
   reverse_view(_Range&&) -> reverse_view<views::all_t<_Range>>;
 
   template<class _Tp>
   inline constexpr bool enable_borrowed_range<reverse_view<_Tp>> = enable_borrowed_range<_Tp>;
 
   namespace views {
   namespace __reverse {
     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>
-    constexpr bool __is_sized_reverse_subrange = false;
+    inline constexpr bool __is_sized_reverse_subrange = false;
 
     template<class _Iter>
-    constexpr bool __is_sized_reverse_subrange<subrange<reverse_iterator<_Iter>, reverse_iterator<_Iter>, subrange_kind::sized>> = true;
+    inline constexpr bool __is_sized_reverse_subrange<subrange<reverse_iterator<_Iter>, reverse_iterator<_Iter>, subrange_kind::sized>> = true;
 
     template<class _Tp>
-    constexpr bool __is_unsized_reverse_subrange = false;
+    inline constexpr bool __is_unsized_reverse_subrange = false;
 
     template<class _Iter, subrange_kind _Kind>
-    constexpr bool __is_unsized_reverse_subrange<subrange<reverse_iterator<_Iter>, reverse_iterator<_Iter>, _Kind>> = _Kind == subrange_kind::unsized;
+    inline constexpr bool __is_unsized_reverse_subrange<subrange<reverse_iterator<_Iter>, reverse_iterator<_Iter>, _Kind>> = _Kind == subrange_kind::unsized;
 
     template<class _Tp>
     struct __unwrapped_reverse_subrange {
       using type = void; // avoid SFINAE-ing out the overload below -- let the concept requirements do it for better diagnostics
     };
 
     template<class _Iter, subrange_kind _Kind>
     struct __unwrapped_reverse_subrange<subrange<reverse_iterator<_Iter>, reverse_iterator<_Iter>, _Kind>> {

libcxx/include/ext/__hash

 // -*- 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_EXT_HASH
 #define _LIBCPP_EXT_HASH
 
 #  pragma GCC system_header
 
-#include <__string>
+#include <__config>
 #include <cstring>
 #include <string>
 
+_LIBCPP_DIAGNOSTIC_PUSH
+_LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wprivate-header")
+#include <__string>
+_LIBCPP_DIAGNOSTIC_POP
+
 namespace __gnu_cxx {
 
 template <typename _Tp> struct _LIBCPP_TEMPLATE_VIS hash { };
 
 template <> struct _LIBCPP_TEMPLATE_VIS hash<const char*>
  : public std::unary_function<const char*, size_t>
 {
     _LIBCPP_INLINE_VISIBILITY

libcxx/include/ext/hash_map

template <class Key, class T, class Hash, class Pred, class Alloc>
     bool
     operator!=(const hash_multimap<Key, T, Hash, Pred, Alloc>& x,
                const hash_multimap<Key, T, Hash, Pred, Alloc>& y);
 
 }  // __gnu_cxx
 
 */
 
-#include <__algorithm/is_permutation.h>
 #include <__config>
 #include <__hash_table>
 #include <ext/__hash>
 #include <functional>
 #include <stdexcept>
 #include <type_traits>
 
+_LIBCPP_DIAGNOSTIC_PUSH
+_LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wprivate-header")
+#include <__algorithm/is_permutation.h>
+_LIBCPP_DIAGNOSTIC_POP
+
 #if defined(__DEPRECATED) && __DEPRECATED
 #if defined(_LIBCPP_WARNING)
     _LIBCPP_WARNING("Use of the header <ext/hash_map> is deprecated.  Migrate to <unordered_map>")
 #else
 #   warning Use of the header <ext/hash_map> is deprecated.  Migrate to <unordered_map>
 #endif
 #endif
 

libcxx/include/ext/hash_set

 template <class Value, class Hash, class Pred, class Alloc>
     bool
     operator!=(const hash_multiset<Value, Hash, Pred, Alloc>& x,
                const hash_multiset<Value, Hash, Pred, Alloc>& y);
 }  // __gnu_cxx
 
 */
 
-#include <__algorithm/is_permutation.h>
 #include <__config>
 #include <__hash_table>
 #include <ext/__hash>
 #include <functional>
 
+_LIBCPP_DIAGNOSTIC_PUSH
+_LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wprivate-header")
+#include <__algorithm/is_permutation.h>
+_LIBCPP_DIAGNOSTIC_POP
+
 #if defined(__DEPRECATED) && __DEPRECATED
 #if defined(_LIBCPP_WARNING)
     _LIBCPP_WARNING("Use of the header <ext/hash_set> is deprecated.  Migrate to <unordered_set>")
 #else
 #   warning Use of the header <ext/hash_set> is deprecated.  Migrate to <unordered_set>
 #endif
 #endif
 

libcxx/test/configs/apple-libc++-shared.cfg.in

 # Testing configuration for Apple's system libc++.
 #
 # This configuration differs from a normal LLVM shared library configuration in
 # that we must use DYLD_LIBRARY_PATH to run the tests against the just-built library,
 # since Apple's libc++ has an absolute install_name.
 #
 # We also don't use a per-target include directory layout, so we have only one
 # include directory for the libc++ headers.
 
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
 config.substitutions.append(('%{flags}',
     '-isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if '@CMAKE_OSX_SYSROOT@' else ''
 ))
 config.substitutions.append(('%{compile_flags}',
-    '-nostdinc++ -isystem %{install}/include/c++/v1 -I %{libcxx}/test/support'
+    '-nostdinc++ -I %{install}/include/c++/v1 -I %{libcxx}/test/support'
 ))
 config.substitutions.append(('%{link_flags}',
     '-nostdlib++ -L %{install}/lib -lc++'
 ))
 config.substitutions.append(('%{exec}',
     '%{executor} --execdir %T --env DYLD_LIBRARY_PATH=%{install}/lib -- '
 ))
 

libcxx/test/configs/ibm-libc++-shared.cfg.in

 # This testing configuration handles running the test suite against libc++ on
 # AIX using a shared library.
 #
 
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
 config.substitutions.append(('%{flags}', ''))
 config.substitutions.append(('%{compile_flags}',
-    '-nostdinc++ -D__LIBC_NO_CPP_MATH_OVERLOADS__ -isystem %{install}/include/c++/v1 -I %{libcxx}/test/support'
+    '-nostdinc++ -D__LIBC_NO_CPP_MATH_OVERLOADS__ -I %{install}/include/c++/v1 -I %{libcxx}/test/support'
 ))
 config.substitutions.append(('%{link_flags}',
     '-nostdlib++ -L %{install}/lib -lc++ -lc++abi -latomic -Wl,-bbigtoc'
 ))
 config.substitutions.append(('%{exec}',
     '%{executor} --execdir %T --env LIBPATH=%{install}/lib -- '
 ))
 

libcxx/test/configs/llvm-libc++-mingw.cfg.in

 # This testing configuration handles running the test suite against LLVM's libc++
 # using either a DLL or a static library, with MinGW/Clang on Windows.
 
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
 config.substitutions.append(('%{flags}', ''))
 config.substitutions.append(('%{compile_flags}',
-    '-nostdinc++ -isystem %{include} -isystem %{target-include} -I %{libcxx}/test/support'
+    '-nostdinc++ -I %{include} -I %{target-include} -I %{libcxx}/test/support'
 ))
 config.substitutions.append(('%{link_flags}',
     '-nostdlib++ -L %{lib} -lc++'
 ))
 config.substitutions.append(('%{exec}',
     '%{executor} --execdir %T --env PATH=%{lib} -- '
 ))
 

libcxx/test/configs/llvm-libc++-shared-clangcl.cfg.in

 # This testing configuration handles running the test suite against LLVM's libc++
 # using a DLL, with Clang-cl on Windows.
 
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
 config.substitutions.append(('%{flags}', '--driver-mode=g++'))
 config.substitutions.append(('%{compile_flags}',
-    '-nostdinc++ -isystem %{include} -isystem %{target-include} -I %{libcxx}/test/support -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_STDIO_ISO_WIDE_SPECIFIERS'
+    '-nostdinc++ -I %{include} -I %{target-include} -I %{libcxx}/test/support -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_STDIO_ISO_WIDE_SPECIFIERS'
 ))
 config.substitutions.append(('%{link_flags}',
     '-nostdlib -L %{lib} -lc++ -lmsvcrt -lmsvcprt -loldnames'
 ))
 config.substitutions.append(('%{exec}',
     '%{executor} --execdir %T --env PATH=%{lib} -- '
 ))
 

libcxx/test/configs/llvm-libc++-shared-gcc.cfg.in

 # This testing configuration handles running the test suite against LLVM's libc++
 # using a shared library, with GCC. This is done differently from Clang because
 # GCC does not support the -nostdlib++ command-line flag.
 
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
 config.substitutions.append(('%{flags}', ''))
 config.substitutions.append(('%{compile_flags}',
-    '-nostdinc++ -isystem %{include} -isystem %{target-include} -I %{libcxx}/test/support'
+    '-nostdinc++ -I %{include} -I %{target-include} -I %{libcxx}/test/support'
 ))
 config.substitutions.append(('%{link_flags}',
     '-L %{lib} -Wl,-rpath,%{lib} -nodefaultlibs -lc++ -lm -lgcc_s -lgcc -lpthread -lc -lgcc_s -lgcc -latomic'
 ))
 config.substitutions.append(('%{exec}',
     '%{executor} --execdir %T -- '
 ))
 

libcxx/test/configs/llvm-libc++-shared.cfg.in

 # This testing configuration handles running the test suite against LLVM's libc++
 # using a shared library.
 
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
 config.substitutions.append(('%{flags}',
     '-isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if '@CMAKE_OSX_SYSROOT@' else ''
 ))
 config.substitutions.append(('%{compile_flags}',
-    '-nostdinc++ -isystem %{include} -isystem %{target-include} -I %{libcxx}/test/support'
+    '-nostdinc++ -I %{include} -I %{target-include} -I %{libcxx}/test/support'
 ))
 config.substitutions.append(('%{link_flags}',
     '-nostdlib++ -L %{lib} -Wl,-rpath,%{lib} -lc++ -pthread'
 ))
 config.substitutions.append(('%{exec}',
     '%{executor} --execdir %T -- '
 ))
 

libcxx/test/configs/llvm-libc++-static-clangcl.cfg.in

 # This testing configuration handles running the test suite against LLVM's libc++
 # using a static library, with Clang-cl on Windows.
 
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
 config.substitutions.append(('%{flags}', '--driver-mode=g++'))
 config.substitutions.append(('%{compile_flags}',
-    '-nostdinc++ -isystem %{include} -isystem %{target-include} -I %{libcxx}/test/support -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_STDIO_ISO_WIDE_SPECIFIERS'
+    '-nostdinc++ -I %{include} -I %{target-include} -I %{libcxx}/test/support -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_STDIO_ISO_WIDE_SPECIFIERS'
 ))
 config.substitutions.append(('%{link_flags}',
     '-nostdlib -L %{lib} -llibc++ -lmsvcrt -lmsvcprt -loldnames'
 ))
 config.substitutions.append(('%{exec}',
     '%{executor} --execdir %T --env PATH=%{lib} -- '
 ))
 

libcxx/test/configs/llvm-libc++-static.cfg.in

 # This testing configuration handles running the test suite against LLVM's libc++
 # using a static library.
 
 lit_config.load_config(config, '@CMAKE_CURRENT_BINARY_DIR@/cmake-bridge.cfg')
 
 config.substitutions.append(('%{flags}',
     '-isysroot {}'.format('@CMAKE_OSX_SYSROOT@') if '@CMAKE_OSX_SYSROOT@' else ''
 ))
 config.substitutions.append(('%{compile_flags}',
-    '-nostdinc++ -isystem %{include} -isystem %{target-include} -I %{libcxx}/test/support'
+    '-nostdinc++ -I %{include} -I %{target-include} -I %{libcxx}/test/support'
 ))
 config.substitutions.append(('%{link_flags}',
     '-nostdlib++ -L %{lib} -lc++ -lc++abi -pthread'
 ))
 config.substitutions.append(('%{exec}',
     '%{executor} --execdir %T -- '
 ))
 

libcxx/test/std/containers/sequences/array/size_and_alignment.pass.cpp

 //===----------------------------------------------------------------------===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 // <array>
 
 // template <class T, size_t N >
 // struct array
 
 // Test the size and alignment matches that of an array of a given type.
 
+// Ignore error about requesting a large alignment not being ABI compatible with older AIX systems.
+#if defined(_AIX)
+# pragma clang diagnostic ignored "-Waix-compat"
+#endif
+
 #include <array>
 #include <iterator>
 #include <type_traits>
 #include <cstddef>
 
 #include "test_macros.h"
 
-
 template <class T, size_t Size>
 struct MyArray {
   T elems[Size];
 };
 
 template <class T, size_t Size>
 void test() {
   typedef T CArrayT[Size == 0 ? 1 : Size];

libcxx/utils/libcxx/test/config.py

def configure_compile_flags_header_includes(self):
         if not os.path.isdir(cxx_headers):
             self.lit_config.fatal("cxx_headers='{}' is not a directory.".format(cxx_headers))
         (path, version) = os.path.split(cxx_headers)
         (path, cxx) = os.path.split(path)
         triple = self.get_lit_conf('target_triple', None)
         if triple is not None:
             cxx_target_headers = os.path.join(path, triple, cxx, version)
             if os.path.isdir(cxx_target_headers):
-                self.cxx.compile_flags += ['-isystem', cxx_target_headers]
-        self.cxx.compile_flags += ['-isystem', cxx_headers]
+                self.cxx.compile_flags += ['-I', cxx_target_headers]
+        self.cxx.compile_flags += ['-I', cxx_headers]
         if self.libcxx_obj_root is not None:
             cxxabi_headers = os.path.join(self.libcxx_obj_root, 'include',
                                           'c++build')
             if os.path.isdir(cxxabi_headers):
                 self.cxx.compile_flags += ['-I' + cxxabi_headers]
 
     def configure_link_flags(self):
         # Configure library path