[libcxx-commits] [libcxx] [libc++] Allow the use of extensions in the implementation (PR #79532)

Nikolas Klauser via libcxx-commits libcxx-commits at lists.llvm.org
Thu Mar 14 07:21:07 PDT 2024


philnik777 wrote:

> We're seeing major compile-time regressions (50%+) from this change in normal-looking code that uses libc++, using clang from head.
> 
> My best guess is that we're now adding lots of diagnostics-overridden regions to most TUs, and querying them (when clang checks: should we emit diagnostic X) is inefficient. I will dig a bit into this.

I'm pretty sure I've checked compile times before and after and didn't see any significant regression. While it's been a bit more than noise, it didn't seem significant enough to be worrisome, especially since should be able to offset that by simplifying the code.

> If this is not related to our use of modules (I need to check) then I think we should revert this and hopefully find a cheaper way. WDYT? If it is modules-related, then it seems more complicated and will depend on the details.

I don't think we can revert. We're already depending on this in parts of the code base. IMO we should fix forward instead if this is actually a problem with this approach.

> If it is because of the diagnostic overrides, we should be able to go back to simply using the pragma system header.
> 
> The diagnostic disabling is only really for our test suite (and people who enable -Wsystem-header, but they asking to see diagnostics). We should just be able to disable the warning entirely in the test suite. IDK Why we have it on when we allow the use of extensions.
> 
> @philnik777 Can you say why you chose to go this route?

We're still using `#pragma GCC system_header`. I've done it this way to avoid introducing extensions in the test suite, since that shouldn't be using extensions as much as possible as a conformance test suite and it's used by other implementations.


https://github.com/llvm/llvm-project/pull/79532


More information about the libcxx-commits mailing list