The definition of max_size() is the maximum number of elements that a string_view can hold.
(Actual definition: Returns: The largest possible number of char-like objects that can be referred to by a basic_string_view.)

In D114395#3159050, @Quuxplusone wrote:

FWIW, I'm still of the (very firmly held) opinion that this patch is unnecessary. max_size() is always just an upper bound on the size of the sequence. Lowering that upper bound

• might introduce actual factual errors, e.g. if we get the bound wrong — whereas size_t(-1) can never be wrong
• will never help any user of libc++, because a loose upper bound (as returned by max_size) is never useful and always useless
• is no closer to being "accurate," because it is just as impossible in practice to have a string_view of length 9223372036854775807 or 4611686018427387903, as to have a string_view of length 18446744073709551615. These are all equally lying numbers, and there's no point pretending that any one of them is "more useful" or "more accurate" than any other.

In D70631#3112196, @Mordante wrote:

In D70631#3111085, @floam wrote:

The C++17 limitation sure seems hard to justify.

Since this is added to the Standard for C++17 and our current policy is not to backport features it will be limited to C++17 and later.
After moving the code to the dylib the reason stated for only enabling it in C++17 is just no longer the proper reason.

When I wrote span, I tried to do it this way, and I found that it led to much more complicated code.
Maybe span has changed enough that this is no longer true - dunno.

What's the goal here? In terms of behavior?

LGTM

There's also all the ones in __functional/operations.h, like greater_equal<void>

I suggest you take all the techniques at http://graphics.stanford.edu/~seander/bithacks.html and make sure they don't cause a warning.

I am not an approver for LLVM, but this LGTM.

In D106899#2908395, @gAlfonso-bit wrote:

Done!

I would like to see you undo all the formatting changes.
They are not germane for this patch, and (IMHO) reduce the readability of the code.

How about some tests?

In D103160#2784959, @ldionne wrote:

We need to decide whether the ABI version is something that can be selected by users when they use libc++, or whether it's something that's baked in when the library is built and shipped (usually by a vendor). In my opinion, the former doesn't make sense, because the ABI version has an influence on what is provided in the .so - so the behavior of the headers has to match exactly how the .so was built. I'd like to get a common understanding and agreement that this is how things should work.

wchar is 2 bytes not 4,

(2.3) — a #include of <ciso646>, <cstdalign>, or <cstdbool> can simply be removed.

This makes me think that it maybe better to generate #error instead of trying to workaround MSVC problem.

@mclow.lists, @ldionne, what do you think?

That cppreference page you quote says:

The same words are defined in the C programming language in the include file <iso646.h> as macros. Because in C++ these are built into the language, the C++ version of <iso646.h>, as well as <ciso646>, does not define anything.

In D96221#2547462, @mclow.lists wrote:

There's no 128 bit integral support, which I'd like to have in std::format, I can add a __u128toa to fix this.

If you really want to support 128 bit integers (it's optional), I'd suggest thinking about extending to_chars

There's no 128 bit integral support, which I'd like to have in std::format, I can add a __u128toa to fix this.

Why do we need *another* integer to string conversion function in libc++?

In D95251#2516668, @CaseyCarter wrote:

In D95251#2516256, @zoecarver wrote:

In the issue, I'll ask for clarification and propose either that all these find-like members are marked noexcept or none of them are. The thing that's really making me uneasy is that two of them are and the rest aren't.

It's probably a wide vs. narrow contract thing. The find overloads that take const char*, for example, have a precondition that the pointer is non-null. Under the so-called "Lakos rule" they would/could/should be "Throws: Nothing" but not noexcept.

• If the standard says they should be noexecpt, we should keep them noexcept.
• Yes, file an LWG issue.

Ok, I take it back. __str_find calls Traits::find, which is NOT always noexcept. (It *is* for all the specializations of std::char_traits, but that's not enough). So we can't slap NOEXCEPT here; in fact, we should revisit __str_find, and remove some of the NOEXCEPTs that are already there.

In D56692#2497567, @curdeius wrote:

Shouldn't this patch implement also the renames of leap and link?

The C11 standard states (in section 7.21.7.5 The getc function):

This looks like WAY too much code for "add string contains".

If you don't have a steady clock, then none of the timed synchronization calls can work.

Starting in C++11, the standard library requires a monotonic clock.

How about something more like:

make_string<char>(
                  " !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMN"
                          "OPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~")
      ==          " !\"#$%&'()*+,-./0123456789:;<=>?@ABCDEFGHIJKLMN"
                          "OPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~");

You need to bump the TEST_STD_VER too - see test/support/test_macros.h

In D93233#2452718, @MaskRay wrote:

IMHO bitset sort is not a standard term, so it needs some clarification. From a glance this uses a bitset partition and needs temporary storage (heap memory allocation?). Does the original algorithm have heap memory allocation?

In D93071#2447283, @ldionne wrote:

Indeed, however std::allocator is specified to use operator new and operator delete.

In D93071#2447271, @mclow.lists wrote:

if you allocate memory with an allocator, you need to deallocate it with the same allocator. Not with operator delete

if you allocate memory with an allocator, you need to deallocate it with the same allocator. Not with operator delete

if you need to remove something for C++03, then the test would be:
#ifndef _LIBCPP_CXX03_LANG

I'm not seeing any coordination between different syncbufs on the same stream here.
My impression was that implementing this required a global registry of streams that had syscstreams associated with them.

Consider the following program:

I think this is the wrong direction - getting rid of _LIBCPP_NO_HAS_CHAR8_T
We're making the feature macro __cpp_lib_char8_t always match __cpp_char8_t
There are two macros for a reason.

And we should be careful to define __cpp_lib_char8_t only when we provide the feature (which we do, based on _LIBCPP_NO_HAS_CHAR8_T)

(reading back)

Could you elaborate / invent artificial scenario when such logic is necessary?

I don't have to invent any such logic. It's right there in the existing code.

Commented there.
For the record, the link is https://reviews.llvm.org/D92212 (without the trailing slash.
Alternately, you can just write D92212 and Phab will figure it out.

Has clang-format been improved enough that it will not "wreck" std::less<void>? Last time I tried it on that code, it pessimized it pretty thoroughly.
Code pasted here for reference:

#if _LIBCPP_STD_VER > 11
template <>
struct _LIBCPP_TEMPLATE_VIS less<void>
{
    template <class _T1, class _T2>
    _LIBCPP_CONSTEXPR_AFTER_CXX11 _LIBCPP_INLINE_VISIBILITY
    auto operator()(_T1&& __t, _T2&& __u) const
    _NOEXCEPT_(noexcept(_VSTD::forward<_T1>(__t) < _VSTD::forward<_T2>(__u)))
    -> decltype        (_VSTD::forward<_T1>(__t) < _VSTD::forward<_T2>(__u))
        { return        _VSTD::forward<_T1>(__t) < _VSTD::forward<_T2>(__u); }
    typedef void is_transparent;
};
#endif

It would be nice if the test had a reference to the bug report.

In D63744#2385750, @jyknight wrote:

Hm, that might indeed be feasible. We'll need to potentially insert padding both before the first type and after the second type, but we can static_assert the correctness against the old layout, so that's not _too_ scary.

If we go this route, it will mean that libc++ is only usable in an ABI-stable manner on Clang 9+ or GCC 9+. On earlier compilers, and on MSVC, the attribute is ignored, and the layout will thus be incorrect/incompatible. Is it viable to require those compiler versions to both build and use libc++? The LLVM build itself only requires GCC 5+/Clang 3.5 at the moment, so that would mean we'd need to start requiring the use of the 2-stage "runtimes" build in order to build libc++, at the least.

I'm fine with this.
This is not how I'd write it, but that's ok :-)

I'm fine with rebasing and committing this.

LGTM

In D88727#2330329, @mclow.lists wrote:

• Make all the containers do resolve this the same way.

So the goals of this patch should be:

• Resolve the original problem,
• Decide if the problem shown in D1723 is real, and if so, make sure that still works, and
• Make all the containers do resolve this the same way.

In D88727#2330308, @kamleshbhalui wrote:

Looks like because of this patch vector deviated from other container implementation.
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20130916/089030.html

Is this a problem unique to vector? Do the other containers have the same constructor? Do those constructors have the same issue?
Checking ... list does not seem to have this problem. deque does not either. forward_list does not either.

A general comment - is someone going to run a libc++ test bot for NuttX? Otherwise, how will we know when changes that we make to libc++ break NuttX support?

I would rather see:

llvm_unreachable("unexpected type");
return ""; // or string()

LGTM.

A couple of quick comments, more to come.

I see no tests.

In D82627#2158668, @eugenis wrote:

You are right. CFI also checks reinterpret_casts even if they are not UB, because they are just as likely to be a source of type confusion bugs. So, the choice here is between this change, and _LIBCPP_NO_CFI attribute. The attribute change would ideally move the casts into a helper function to avoid suppressing too much checking.

In D82810#2123005, @compnerd wrote:

Might be useful to just run IWYU over the tree at some point to remove extraneous headers.

General comment: would it not be better to make the compiler smarter to solve this (and reap the benefits for vector and other code)?

I wouldn't change the __advance definitions. That's just spreading the pain around. Convert from Distance to iterator_traits<...>::difference_type in advance

In D81425#2080807, @ldionne wrote:

I think it's weird to have a wide open template parameter like this in std::distance, but it appears to be what the Standard wants.

So I don't understand what the observable difference is here. Could you add a test?

I've got nothing to say here.

LGTM. Feel free to ignore the clang-format whining.

This is not sufficient; it just "moves the pain" from the callers code into the dylib. All the functions in charconv.cpp need to be marked as noexcept.

BTW, string(nullptr, 0) is a valid call. The range [nullptr, nullptr) is valid.

You're not really solving the problem you're talking about.

This looks fine to me.