Let's talk about these use cases.

1. A normal operating system target with a "hosted" libc.
2. A baremetal target with an "embedded" libc; basically a stripped down libc which only includes stuff that doesn't require an operating system. What exactly this includes varies; may include some form of "malloc", some "POSIX" APIs, semihosting, etc.
3. A baremetal target with no libc, using custom implementations for everything. This is what we conventionally referred to as "freestanding", and what -ffreestanding implements. The user provides memcpy/memmove/memset, and uses the compiler-provided "builtins" lib if neceessary, but nothing we would traditionally call "libc".

Okay, so this is potentially ignorance on my part. I was under the impression that folks using freestanding mode did not want any library to be linked in. Are there freestanding libc implementations out there that the user would link in instead that we defer to (same as we defer hosted C standard library interfaces to the CRT)? Basically, do we need to do nothing in Clang to support N2524?

Okay, so this is potentially ignorance on my part. I was under the impression that folks using freestanding mode did not want any library to be linked in. Are there freestanding libc implementations out there that the user would link in instead that we defer to (same as we defer hosted C standard library interfaces to the CRT)? Basically, do we need to do nothing in Clang to support N2524?

A freestanding implementation doesn't necessarily mean that everything is header-only. It's fine to require linking against a (freestanding) C runtime library. All header-only is fine too though, if you want to make that work.

Messed with this some over the weekend, and realized there's ambiguity in the request. I also found that there are some challenges with each of the reasonable things that I think you could be requesting. Note that all 3 approaches result in failing tests, and fixing those tests will take time.

I haven't messed with the pipeline or buildbot scripts before. Is there some way for me to test those changes before landing them?

Removing the preprocessor defines, in order to divorce the tests from the library under test.

gentle ping

In D68275#1690334, @broadwaylamb wrote:

I think it would be a huge help, because running libc++ tests on a remote target is exactly what I'm trying to accomplish.

I like this too, however I don't know how this interacts with tests like test/libcxx/language.support/support.dynamic/new_faligned_allocation.sh.cpp that specify the build command-line explicitly:

// RUN: %build -faligned-allocation

We'd make sure that %build links in the additional entry point?

Agreed that seems problematic, but I think we can fix it after this patch lands. :)

For what it's worth, when I ran these tests with MSVC in the Windows kernel, I just called main, and falling off the end of main happened to work. I can't say I'm thrilled with that approach though.

add a comment, and it will be fine in my eyes. You'll need signoff from the code owner though.

You should probably add whoever the current code owner of that static analyzer to this review. I have very little involvement with Clang these days, so a "LGTM" from me doesn't carry much weight.

For those that would prefer random device to not exist if it isn't cryptographically secure, please write a wg21 paper. I will gladly review such a paper, and if you need a presenter, then I will present it if I am attending. I won't be at Rapperswil, but I will be at San Diego.

In D32411#1056381, @EricWF wrote:

I spoke to STL on the MSVC team a while ago, and he stated that if we produced a paper describing why we need #include_next and the rational behind it, and they would pass that on to the front-end team. They didn't guarantee that they would implement it, but that seems like a good first step.

Rebased

Talked with mclow and some Microsoft devs in Albuquerque. I'm not expecting include_next to show up in MSVC. Sometime in the next month I hope to rebase this change.

In D37677#866743, @Quuxplusone wrote:

This current patch just swaps out std::mutex for a std::mutex-alike class that claims to be faster for uncontested accesses. Definitely safer than my interpretation. :) If this patch actually helps, then I would offer that the class could be provided as a reusable class std::__spin_lock in the <mutex> header instead of being hidden inside __assoc_shared_state.

Is there a benchmark where this demonstrates some performance improvement? I fear that the switch to condition_variable_any will swamp any performance gains from the switch to a spin lock.

LGTM. You should probably get one other person to approve though. I'm hoping that @EricWF or @mclow.lists will take a look

The test headers should not be in the production include folder. They should probably be in the benchmark folder.

ping

ping

If you want the performance improvements in the BM_sort_std_worst_quick cases preserved, you really need to port the benchmark from Aditya's repo into the libcxx benchmark code base. Otherwise, the next person that comes along to improve std::sort performance may very well wreck the performance gains you achieved.

I like this change in general. Dinkumware has been using introsort for 10+ years, so I'm a bit surprised that libc++ wasn't already.

Those are interesting (and useful) results... but they don't look like they came from the same algorithms.bench.cpp that I'm looking at...
https://github.com/llvm-mirror/libcxx/blob/master/benchmarks/algorithms.bench.cpp

alternatively, you could report the comparison of the old code vs. the new code with an existing benchmark, like benchmarks/algorithms.bench.cpp

This patch needs benchmarks that demonstrate the performance changes.

ping

ping

Switched to static consts

Rebased.
Separating out logging into it's own class.
Also tweaked the output slightly so that the language dialect under test shows up as the first line of output.

_LIBCPP_MS_CRT seems fine too.

LGTM, but you should probably get approval from somebody a bit more senior with the project.

ping

ping

This is very stale at this point, and isn't blocking anything. Closing.

LGTM.
@waltl : Do you need me to submit the changes, or will you handle that?

Are you suggesting that libc++ should stop declaring/defining these functions, at least publically?

I noticed that the Windows STL headers have to do this dance with new (even though they do (foo)(...) for min and max). If we're going to need
to guard against a bunch of macros I would like to use a single approach. Other than updating the #if defined(min) || defined(max) lines it's trivial to guard
against additional macros.

I agree that we need to be precise in our platform detection macros. On Windows, we currently need to worry about which compiler is being used, whether we are targeting the mingw environment or the native (?) environment, and which c-runtime is being used.

I like the warning that you generate for min and max macros existing. Is the push_macro / pop_macro the right way to go though? You could throw extra parenthesis around the declaration and usages of min/max to avoid macro expansion.

LGTM

Getting the test suite green sooner rather than later seems like a good reason to temporarily pick a 1-3 year solution rather than a 5+ year solution. Also, as you mention, this isn't all throw away work, so it's still progress.

libstdc++ and the Visual Studio C++ runtime have very different compatibility expectations.

I'm still working on this.

In D32411#735128, @halyavin wrote:

BTW, the list of include files which are located in [PROGRAM_FILES]\Microsoft Visual Studio 14.0\VC\include directory is

• stdbool.h
• limits.h
• stdint.h
• setjmp.h

The rest is in [PROGRAM_FILES]\Windows Kits\10\Include\10.0.whatever.0\ucrt directory. Which directory @_LIBCPP_INCLUDE_NEXT@ is supposed to point to?

Neat! I would have liked to have had this for the Excess Padding Checker. Currently, the padding checker has a very long diagnostic that recommends a new order for data members. I think a note (or fixit) would be more appropriate, but that support didn't exist previously.

In D24374#538521, @smeenai wrote:

@bcraig thanks for pointing me to that diff; there's a lot of nice cleanup going on there. Were you planning on updating and following up on it?

I also realized I forgot to adjust locale_win32.cpp for this diff. Will re-upload with that fixed.

This seems related:
https://reviews.llvm.org/D20596

LGTM

The "@" will do a ping through phabricator, but a direct email is probably going to be your best bet at this point.

In D21803#530678, @tavianator wrote:

Ping?

In D23719#520969, @rsmith wrote:

In D23719#520954, @bcraig wrote:

In D23719#520952, @rsmith wrote:

Are we really guaranteed that the C and C++ compiler behave the same way here? I don't see why that would necessarily be the case.

For libc++, std::atomic is implemented in terms of _Atomic. So as long as the C++ compiler doesn't butcher _Atomic, it seems that the behavior would be the same.

I don't see any good reason to assume that's the case. GCC, for instance, does not define _Atomic *at all* in C++ mode; the implementation used by libc++ in that case is completely different. Also, as far as I know, libc++ did not previously require the host to have a C11 compiler. And there's no reason to assume that the C compiler picked up by cmake is in any way related to the C++ compiler.

Bottom line: if you want to know how the C++ compiler behaves, you need to test the C++ compiler not the C compiler.

In D23719#520953, @smeenai wrote:

@bcraig: __STDC_NO_ATOMICS__ wouldn't be defined for pre-C11 compilers either, right?

From what I understand of the original code sample, one of the purposes was to check for 64-bit atomic support on 32-bit systems (hence the use of uintmax_t and the check for __atomic_fetch_add_8). I don't really know of a portable way of accomplishing that without at least including <stdint.h>.

In D23719#520952, @rsmith wrote:

Are we really guaranteed that the C and C++ compiler behave the same way here? I don't see why that would necessarily be the case.

I like the rationale here, but can we avoid pulling in headers at all?
You could test _ _STDC_NO_ATOMICS_ _. You could also have some code like this...

committed rL277357: Improve shared_ptr dtor performance.

I am going to submit the code changes and the tests independently. I'm having trouble getting cmake to use the right compiler for the libcxx-benchmarks target.

Added weak_ptr benchmark, as that's where the cost shifted.

LGTM (with a comment nit), but you'll need to get approval from @EricWF or @mclow.lists.

LGTM. As usual, you'll want to get one of the code owner's sign off first though.