@ldionne this is ready for re-review when you're ready to re-review it :)

• Add SFINAE tests.
• Remove "no_unique_address".

It's unclear to me if I should add noexcept to the const char* overloads.

Thanks again for the cleanup!

Thanks for the cleanup, this is much improved!

• Format

• Add static size control block
• fix based on review

I think this makes sense, especially given that most other related algorithms also do this, and that __inplace_merge (__buffered_inplace_merge's only user) also specifies the template argument.

A few small comments. This is looking really good :)

@Quuxplusone I like this patch a lot better than mine. It seems like this implementation is essentially following the sample in the paper. I was meaning to update mine to use that approach but didn't find the time/remember. Anyway, this looks good to me, thanks!

This LGTM other than the small note.

Didn't read it too carefully, but this looks good to me.

• Add more multidimensional throwing tests.

Fix based on review.

Thanks for those suggestions, Louis! Going to add some more tests over the next few days.

I don't think you need to add this to the docs, but just so people know, you can also search reviews.llvm.org.

@ldionne thank you for adding this much-needed bit of documentation.

This looks great. Thanks for spending so much time iterating on this patch!

This LGTM. Thanks. Looking forward to using this in my bind_front patch ;)

This is now ready for review (cc @ldionne).

• Resolve TODOs and finish implementing.

We could probably do something like what this patch is doing and determine whether a class can be passed in registers based on whether its subobjects can be passed in registers. If all of the subobjects can be passed in registers, the current class can be passed in registers too unless something declared in the current class forces it to be passed indirectly (e.g., a virtual function is declared).

• Rebase & re-write patch to add a new control block: "__shared_ptr_array"

@ahatanak this patch should now be a nfc for types that don't have anything to do with the trivial-abi attribute.

• Fix base-derived case.
• Rebase.

This LGTM. Not only to fix the LWG issue, but this is a great cleanup to this part of the codebase. So, thanks Louis.

In D92361#2459655, @rjmccall wrote:

In D92361#2459513, @zoecarver wrote:

I think that as long as the class leaves a copy/move constructor defaulted, there's no need for a new trivial_abi attribute.

Sorry, I'm not sure I follow. Could you elaborate a bit or provide an example? What do you mean by "new" trivial_abi attribute?

Sorry, I mean that I think Akira's example should be passed directly. It shouldn't require its own trivial_abi attribute in order to get the treatment.

I think that as long as the class leaves a copy/move constructor defaulted, there's no need for a new trivial_abi attribute.

Let me start by saying that I haven't spent a lot of time looking at it but I really like this implementation.

Hey @abrachet. Thanks for the patch. Just FYI, I've got D67086 which also implements syncbuf. Maybe we can coordinate on how to merge these patches.

Sorry for the slow update. This patch now implements the suggested change (to "inherit" the trivial-abi attribute if there are no user-defined special members and its copy/move constructor is deleted solely because of a subobject with the attribute). Let me know what you think/if there are any other review comments.

• Add lots of tests for S0.
• Implement discussed change for handling S0.
• Update docs to reflect change.

Thanks for the cleanup. This LGTM!

This approach may be frowned upon because std::bit_cast is implemented with __builtin_bit_cast but, I think you could actually use __builtin_bit_cast to write the comparison function you want for the tests:

template<class T> struct Buff { char buff[sizeof(T)]; };

If the implementation-specific builtins don't match on these, then maybe they should have different names, is his argument I think.

In D87974#2438682, @BillyONeal wrote:

Are they actually the same, with the same handling of corner cases like unions and tail padding?
There's more to this than just the name, and if they aren't the same, it seems better to have two names.

They are both implementing the same C++ feature, with the same desired semantics of zeroing out any bits in the object representation that aren't in the value representation. If they differ, one or the other would have a bug.

I think perhaps we are talking past each other and have reached the limits of what this sub-thread can hope to achieve.

I'm good with landing this now. I would kind of like to make the tests a little less complicated, but I think they're OK as-is.

So, it looks like GCC already uses __builtin_clear_padding and MSVC already uses __builtin_zero_non_value_bits. This patch (obviously) is currently implementing __builtin_zero_non_value_bits but, I had planned to update it to use __builtin_clear_padding. Maybe that's not the best course of action, though.

@rjmccall Thanks for all that information. You're right; thinking about it in the context of four value operations is helpful.

Thanks for all the test overhaul! Probably better done as a follow-up patch, but this format will make it easier to add move-only tests.

If it is not possible to copy or move the type at all, it is not possible to copy or move it trivially.

• Add comment in SemaDeclCXX.
• Add tests for non-trivial bases/members.

• Update wording in AttrDocs

• Fix wording in AttrDocs
• Add more tests

• Use new status format
• Rebase on master

@ldionne ping.

Can we use the same trick here and enable the tests in C++03? I think it would be good for consistency.

• Rebase off master
• Support template.pass.cpp in C++03 mode

By the way, I had to disable the added tests in C++03 mode.

• Rebase off master
• Split into two tests

• Rebase to trigger CI