Should be fixed in 18e093faf726d15f210ab4917142beec51848258. Sorry I
overlooked your comment on the previous landing of this change.

Thanks, looking.

In D77056#2465936, @rsandifo-arm wrote:

Either way, I realise this isn't great style. It just seems like a practical compromise between the rock of classes having a constant size and the hard place of vectors having a variable length.

The implementation looks fine to me.

In D91659#2458872, @rnk wrote:

In D91659#2458639, @rsmith wrote:

What is the issue with the current structure, ActOnTag produces the wrong AST node, an ElaboratedType, but we want to produce a TypedefType?

@rnk Your thoughts on this would be appreciated.

Looks like the in-tree consumers are fine with being called on invalid declarations (CodeGen bails out if any errors have occurred). I don't think there's any easy way to find if out-of-tree consumers will be OK with this, but it does seem reasonable to ensure that we pass all the tag definitions to the consumer, even if they're invalid.

In D91488#2451755, @teemperor wrote:

I believe rsmith is in GMT-8, so I assume this won't get a fix soon and I went ahead and reverted in 22ccdb787024e954318e35fcf904fd4fa36f5679

In D92409#2450550, @thopre wrote:

In D92409#2426196, @thopre wrote:

Is there a way to disable it from the header? I've noticed this warning when compiling an application using libclang with g++. So I'm looking for a fix outside clang's build system which is not used in that case.

Ping?

Rebase and fix test failure.

I'm overall pretty happy about how clean and non-invasive the changes required here are. But please make sure you don't change the encodings of u8"..." / u"..." / U"..." literals; those need to stay as UTF-8 / UTF-16 / UTF-32. Also, we should have a story for how the wide execution character set is controlled -- is it derived from the narrow execution character set, or can the two be changed independently, or ...?

• Handle @rjmccall's review feedback.
• Properly handle the case of a pack expansion into a non-pack and add tests for that.

Rebase.

I don't like having new in the name of the flag; I think that's going to age badly (and I would argue it already has done; the "new pass manager" isn't really all that new any more).

There are some mechanical changes we should make to our documentation too, particularly www/cxx_status.html.

Looks good with a minor cleanup. Thanks!

Let's just disable this broken GCC warning. IIRC, the corresponding clang warning covers the actual bugs caused by line continuation in comments, and doesn't warn if the next line begins with a //.

In D91311#2418881, @ldionne wrote:

In D91311#2417293, @rsmith wrote:

In D91311#2416940, @ldionne wrote:

LGTM from the libc++ point of view. The CI is passing -- those failures are flaky modules tests that we need to fix.

Perhaps we need to specify -fmodules-validate-system-headers in the test so Clang doesn't assume that system headers are unchanged?

Oh, would that be it? We're not including libc++ headers as system headers when running the tests, though (and we're disabling the system header pragma). Do you think that might still be the issue?

Looks good. There are a few other places where we unconditionally insert dereferenceable attributes (for reference parameters / return types, and for C99's T [static] array parameters). We should presumably apply the same workaround everywhere we add dereferenceable.

Thanks, that's very clean.

This was discussed on the original patch (https://reviews.llvm.org/D17993#inline-830995) and @jdoerfert argued that dereferenceable is correct even in NullPointerIsValid mode. Does this indicate a bug in the middle-end? If so, we should add a FIXME here to remove the workaround once the underlying bug is fixed.