FYI FreeBSD is one of the few OSes that enables -Wsystem-headers by default, *and* uses libc++ as its default C++ library. So we've already submitted a bunch of warning fixes for libc++.

In D6379#1934620, @nickdesaulniers wrote:

Hello, we're hitting this issue when assembling the Linux kernel with Clang (https://github.com/ClangBuiltLinux/linux/issues/716). I'd be happy to dust this off/rebase and add tests. @dim would you prefer to do it or shall I?

As far as I'm concerned, most suggestions make sense, and it is good to have this information clearly stated.

In D76192#1923936, @hans wrote:

This seems reasonable, but can we wait until after the release? I would prefer to not change these things right now.

Note: one thing that I am unsure about is the _NOEXCEPT after some of the copy constructors. I only applied those to classes that have other _NOEXCEPT constructors, but copying may involve other things that could throw.

In D73990#1868108, @plotfi wrote:

In D73990#1868085, @dim wrote:

In D73990#1868071, @plotfi wrote:

This breaks in our downstream runtimes builds for armv7-none-linux-androideabi16. FYI

Can you point me to some build logs? (I didn't receive any failure mails from bots yet.)

Posted a fix at D74358

Yeah sorry, I used a whitelist instead, here: rG52f2df1ecdd79cc550b694ab280f3b0396d7cf9a

In D73990#1868071, @plotfi wrote:

This breaks in our downstream runtimes builds for armv7-none-linux-androideabi16. FYI

Ping, can we get this committed then? :)

Remove stray newline.

Add better NetBSD implementation of internal_dlinfo().

• Rebase onto reformatted files.
• Add internal_dlinfo() for FreeBSD and NetBSD, with stubs for other platforms.
• Add GET_LINK_MAP_BY_DLOPEN_HANDLE() macro for NetBSD.

Btw, note that according to the Linux dlinfo() manpage, it also has RTLD_DI_LINKMAP as a documented way to get at the link map. Maybe we should generalize this in a follow-up review?

Formatting.

Cast return value to link_map to avoid call site errors.

Aha, which version of clang-cl are you using? With the released version of clang-cl 9.0.1, I get a warning instead of an error:

Eh, no it does not crash clang, at least not here? Instead it gives you a compile error, as it should:

LGTM.

We should abandon this, for e.g. 9.0.1 the whole Base_url variable has become unused. :)

In D69990#1750348, @tra wrote:

LGTM, though I'm curious if it's particularly useful. Last time I checked NVIDIA didn't ship libcudart for FreeBSD and without it it's rather cumbersome to use CUDA in practice.

Adding a few people who might know a bit more about CUDA specific things. Please take a look if this review makes any sense, thanks. :)

• Add cuda options test, copied from cuda-options.cu.

I'll work on a test.

Now opt supports -disable-builtin, move the test to llvm/test/Transforms/InstCombine.

In D56398#1744265, @ldionne wrote:

Sorry if this is a basic question, but why are we trying to provide EINTEGRITY in libc++'s errno.h header at all? I don't see this being part of the C or C++ Standard.

I submitted D70193 for adding a -disable-builtin option to opt. Once that is committed, this review can continue.

In D56398#1742899, @xbolva00 wrote:

cem == dim?

In D70143#1742885, @lebedev.ri wrote:

This should have a llvm ir test in llvm/test/transforms/instcombine i think, not a clang test.

LGTM

Another attempt to grab your attention :)

Hm, I would really say that __isnan and the other __ prefixed functions are Linuxisms, or more accurately, glibc-isms. They also don't exist on e.g. macOS:

Ping :)

N.B., EOWNERDEAD and ENOTRECOVERABLE are already defined on lines 158 and line 170, respectively.

Get rid of the ELAST trickery, which is hard to maintain, and does not
appear to serve any purpose. There is no mention of ELAST in the C or
C++ standards, as far as I know.

Now that I'm reading this header again, why do we even bother to define ELAST at all? On Linux, there is no such thing, while on BSDs and macOS, it is already provided by the regular errno.h.

Rewrite the __FreeBSD_version condition to be more straightforward.

Obsoleted by D58425 (and rCXX354515).

Convert m_(monitor|operation)_thread to llvm::Optional<>.

In rL365761#695543, @dim wrote:

@devnexen this appears to cause https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=241137, where simply launching a process in lldb triggers a failure:

Expected<T> must be checked before access or destruction.
Expected<T> value was in success state. (Note: Expected<T> values in success mode must still be checked prior to being destroyed).

@devnexen this appears to cause https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=241137, where simply launching a process in lldb triggers a failure:

For some reason this didn't get closed by Phabricator. Committed in rCRT374070.

@spatel, you mentioned this should be in 9.0.1, with "A noticeable perf regression for x86 vector code made it into the 9.0 release". Does it have a lot of influence on compile-time performance, or run-time performance? (I'd like to pull this one into FreeBSD's clang 9.0.)

In D68269#1691913, @ldionne wrote:

Pinging vendors for awareness of the change: @phosek @danalbert @dim

This is going in soon and will require a change in how you build libc++abi if you need new/delete definitions to be provided in libc++abi.so, namely to use -DLIBCXXABI_ENABLE_NEW_DELETE_DEFINITIONS=ON when you configure CMake for libc++abi.

The upshot of this change is that by default, we won't get both definitions in libc++ and libc++abi, thus unbreaking the default configuration.

LGTM from a FreeBSD point of view. :)

This works for me, and also for the original test case from https://bugs.freebsd.org/240764.

Third time's the charm.