LGTM

Sorry for the delay in responding. I've addressed Jordan's review comments.

Sink diagnoseUnavailableAlignedAllocation into DiagnoseUseOfDecl and add calls to it in a few other places so that diagnostics are produced for calls to aligned operator and builtin operator new/delete in addition to aligned deallocation functions called from deleting destructors.

In D45015#1123097, @EricWF wrote:

In D45015#1121874, @ahatanak wrote:

I see, thank you.

clang front-end currently fails to issue a warning or error when an aligned allocation/deallocation functions are required but not available in a few cases (e.g., delete called from a deleting destructor, calls to operator or builtin operator new/delete). I suppose those bugs should be fixed in separate patches.

I don't think we need to emit warnings from __builtin_operator_new/__builtin_operator_delete. Libc++ is the only consumer, and I think we can trust it to know what it's doing.

I see, thank you.

Could you elaborate on what kind of changes you are planning to make in libc++ after committing this patch?

I think clang should error out or warn when aligned operator or builtin operator new/delete functions are used when they are not available (r306722 should have handled them).

In D45015#1105388, @rsmith wrote:

In D45015#1105372, @vsapsai wrote:

What when compiler has __builtin_operator_new, __builtin_operator_delete? If I build libc++ tests with recent Clang which has these builtins and run tests with libc++.dylib from old Darwin, there are no linkage errors. Should we define __cpp_aligned_allocation in this case?

I don't see why that should make any difference -- those builtins call the same functions that the new and delete operator call. Perhaps libc++ isn't calling the forms of those builtins that take an alignment argument yet?

It doesn't look like it's possible to get the context needed to do accessibility check (CXXMethodDecl 'method' for 'C::overloadedMethod' in the test case) from 'Expr *OvlExpr' in CheckAddressOfMemberAccess. It's possible to get the class in which the member is defined (class ''C' for C::overloadedMethod'), but we want the context in which the member is used (that is 'method').

ping

Add a test case for the Chromium failure. Also, simplify a bit by capturing the calling context in Sema::DeduceTemplateArguments.

Could you rebase this patch against ToT?

In D45015#1105314, @rsmith wrote:

Hmm, perhaps our strategy for handling aligned allocation on Darwin should be revisited. We shouldn't be defining __cpp_aligned_allocation if we believe it doesn't work -- that will break code that uses aligned allocation where available and falls back to something else where it's unavailable.

@ahatanak: how about this:

• Change the driver to not pass -faligned-alloc-unavailable if an explicit -faligned-allocation or -fno-aligned-allocation flag is given; update Clang's note to suggest explicitly passing -faligned-allocation rather than -Wno-aligned-alloc-unavailable if the user provides their own aligned allocation function.
• Change -faligned-alloc-unavailable so that it does not define __cpp_aligned_allocation.
• Change Sema's handling of the "aligned allocation unavailable" case so that, after warning on selecting an aligned allocation function, it then removes those functions from the candidate set and tries again.
  
  That is: on old Darwin, we should not define __cpp_aligned_allocation (even in C++17), produce the "no aligned allocation support" warning in C++17 mode, and then not try to call the aligned allocation function. But if -faligned-allocation or -fno-aligned-allocation is specified explicitly, then the user knows what they're doing and they get no warning.

In D45898#1104025, @rsmith wrote:

As it happens, the C++ committee fixed the language wording hole here very recently. The new rule can be found here: http://wg21.link/p0968r0#2227
In summary: we should to consider the destructor for all elements of the aggregate to be potentially-invoked.

Richard and Doug, do you have any thoughts on John's suggestion?

LGTM

Sorry for the delay, LGTM.

Note that this sort of attribute is stripped from template arguments in template substitution, so there's a possibility that code templated over vectors will produce inadequately-aligned objects.

LGTM

LGTM

Rename variable to something OldCleanupScopeDepth.

x.

Address review comments.

Deactivate the cleanups for callee-destructed parameters that are being forwarded to a delegated constructor.

Fix indentation.

Move method visitArray to the derived classes and pass the array type to the method instead of the QualType.

Address review comments.

Fix an error in emitGlobalDtorWithCXAAtExit and make sure @dso_handle is hidden.

Use pair<const void*, unsigned> as the key.

Committed in r329617.

Address review comments.

Replace flags CanPassInRegisters and CannotPassInRegisters with a 2-bit enum in RecordDecl.

In D45382#1060163, @rjmccall wrote:

Hmm. I'm not actually sure *why* it's not okay to forward callee-cleanup arguments. Do we just not have the necessary logic to disable the cleanup in the caller?

Yes. I intended it as a property that propagates to classes that contain or derive from the type.

In D45384#1060164, @rjmccall wrote:

Well, but I think CanPassInRegisters==false in the base class does always mean CanPassInRegisters==false in the subclass.