In D52268#1239557, @riccibruno wrote:

In D52268#1239538, @erichkeane wrote:

Does this still work with 32 bit hosts? Does PointerIntPair have 3 bits in that case? Is the alignof static_assert valid in that case?

I think it does since Decl is manually over-aligned to 8 bytes. But you are right that the static_assert is probably not needed
here since llvm::PointerIntPair checks that the low bits are available.

I'm not sure how often this ever gets modified, but it is a touch scary to me that we've already maxed out the size of this struct.

I am not sure either... It was just a quick change I spotted and this is borderline not worth doing. If you think that we ought to keep
some space left I will abandon this revision.

Does this still work with 32 bit hosts? Does PointerIntPair have 3 bits in that case? Is the alignof static_assert valid in that case?

A few drive-by comments... This is a patch with interactions I'm not sure I feel comfortable approving myself, so I'll count on @rjmccall to do so.

Thanks for the feedback @rsmith! I'm working through the lambda issue and a few other things, but will get to this as soon as I can.

Based on feedback from @rsmith I've been trying to get this to work with Lambdas like GCC does, though I'm quite confused about how to go about it here. It seems that the lambda definition call operator is deferred, however I'm having a hard time making sure that this happens in the "IFunc" case. I'm still working on it, but it might be a bit.

fix aaron's comments.

Sorry about the long discussion on the comment... These bitfields are just hell on the next guy through if he doesn't have the context/knowledge of what are appropriate sizes.

In D50713#1200564, @riccibruno wrote:

The thing is that I have no idea what is the minimum number
of bits required. It was originally an unsigned but I suspect that
32 bits are not needed. Looking at [implimits] I see

Template arguments in a template declaration [1 024].
Recursively nested template instantiations, including substitution during template argument deduc-
tion (17.8.2) [1 024].

But 1024 seems to be too small and easily exceeded by a doing a bit
of template meta-programming.

Ah, right. I missed that the others already do it. Fine I guess...

I think that this causes UB. The Type baseclass will use the TypeBitfields active member, but this uses the TemplateSpecializationTypeBits, right? I know we take advantage of this elsewhere, but I'm tentative about this one...

After talking to Aaron, we're going to try to use the lexical names for ALL cases. This requires a bunch of of accessory work (in order to get the Identifier from the SourceRange), so it'll be split up into a couple of commits and done separately. I'll likely do the part that gives Attr a diagnostic operator<< as an NFC change in the near future.

Looks easy enough, just note the test issue.

Bump! I don't see anything here that seems questionable (and this has extensive testing), but I presume @rsmith should be the final one to approve.

In D50261#1187705, @bricci wrote:

It seems to me that the increased size of DeclarationNameTable is irrelevant since it is
only used as a member in ASTContext and never copied nor moved from. Also,
at least for C++ it seems to me that this structure is never "rarely used" since it is
used for every overloaded op/ctor/dtor (but I admit that I am not familiar with Sema)

On the face, this seems to be a nice patch. Removing extra allocations and replacing them with the stack is a good thing IMO. This is clearly an example of PIMPL, which while it has its advantages, performance is typically a hit. My 2 concerns with this are:

IMO, I think this (and the next 2 numerically), show to me that we have too much dependence on the order of attributes, which all of these show is not a reliable thing to count on. I quite dislike this as well, and think this is going to lead us to chasing these same issues forever.

In D48100#1186654, @Meinersbur wrote:

I have two approaches to tackle the wrong marker order: D50215 and D50216. IMHO both are too invasive to be justified for the small issue.

@bricci : This likely needs to be moved into the bitfield section I think. We had a couple of other static-asserts that moved, so this is likely one of them.

In D48100#1185189, @hfinkel wrote:

Assuming that @aaron.ballman is still okay with this, I think that we should move forward. @erichkeane, I suspect that fixing the ordering problems, which were often arbitrary before and are still so, is a larger change that we'd want to split out regardless (not to mention the fact that we'd need to decide how to fix them). Erich, is that alright with you?