In Type.h, move method declarations down and mention that the predicates imply the associated basic non-triviality predicates.

Improve comments.

Diagnose C union globals that are non-trivial to default-initialize. Add 3 bits to RecordDeclBitfields for default-initialize, destruct, and copy to fast-path checking.

I didn't know spaces are not allowed between "CHECK" and ":". CodeGen tests LGTM.

Address review comments.

Add a bit to RecordDeclBitfields that indicates whether the record has a non-trivial C union member.

Add an implicit-only attribute ObjCDefaultedAnyToId to avoid passing the flag down.

Call hasNonTrivialPrimitiveCStruct to check whether the type is a non-trivial C struct/union before calling checkNonTrivialCUnion. Fix comments.

Add function Expr::hasNonOverloadPlaceholderType.

Currently a block captures a variable (POD or non-POD) by reference if the enclosing lambda captures it by reference and captures by copy if the enclosing lambda captures by copy.

Address review comments. Diagnose lvalue-to-rvalue conversion of volatile non-trivial C types.

This patch should diagnose lvalue-to-rvalue conversion of volatile non-trivial C unions too, but it currently doesn't.

Address review comments.

Richard, could you shed light on why it's done this way?

I think I now have a better idea of what's causing the crash in IRGen.

• Add another test case which has a block nested inside a lambda and causes clang to crash.
• Fix the capture type passed to addCapture in RebuildLambdaScopeInfo.

There are many attributes that start with objc_ including objc_arc_weak_reference_unavailable.

John and I had a discussion offline and decided that we should not pursue the approach taken in this patch. I'll try to work on a patch that follows the C++11 approach when I have time later.

In D62988#1538577, @rsmith wrote:

How do you write correct (non-leaking, non-double-freeing, non-releasing-invalid-pointers) code with this attribute? For example, suppose I have a __strong union member: does storing to it release the old value (which might be a different union member)? If so, how do you work around that? https://clang.llvm.org/docs/AutomaticReferenceCounting.html#ownership-qualified-fields-of-structs-and-unions should be updated to say what happens. If manual reference counting code is required to make any use of this feature correct (which seems superficially likely), is this a better programming model than __unsafe_unretained?

I agree. Something like arc_inert is probably a better name in this case for the reason you mentioned.

In D62988#1537082, @rjmccall wrote:

Does this lead to C/C++ ABI mismatches? Should we just honor this in C++ as well by ignoring it when deciding to delete special members? Is taking such a general name a good idea if it's language-specific? Richard, thoughts?

Address review comments.

Just set isClassMessage instead of passing a separate flag to tryGenerateSpecializedMessageSend.

In D62433#1520061, @gottesmm wrote:

I don't remember. That being said, this should be a conservative optimization. What if someone adds in the future a different global where this isn't a no-op. Is it possible if you know what the global is to add a whitelist that you only care about those 2 things.

Check the presence of attribute "arc_retain_agnostic" on the global variable before deleting the ARC calls.

In D61974#1521334, @theraven wrote:

LGTM. I wonder if we have any other ugly GCC bug compatibility parts in clang's Objective-C implementation...

I didn't find anything in the past commit log that mentions this optimization . Dan/Michael, do you know the reason ARC optimizer hasn't tried to remove these no-op calls?

Remove flag EncodePointerToObjCTypedef, which is no longer needed.

ping

This patch is based on the assumption that the argument and the return value of objc_autorelease are equal, which isn't always true in MRR.

This isn't the right way to make a call to objc_autorelease a tail call, so I'm closing this for now. If we decide to convert calls to objc_autorelease and objc_retain to calls to objc_autoreleaseReturnValue and objc_retainAutoreleasedReturnValue for other reasons in the future, I can reopen this.

Yes, I think we can add something to the release note.

Okay, I see. The result of calling rootAutorelease when !ISA()->hasCustomRR() is true is the same as a message send, so it doesn't break the basic rules.

IIUC, this is what happens when there is an autorelease message send in the source code:

Delete the entire code path that tries to work around a bug in gcc.

We discussed this before, but we already break the base ObjC rules since the runtime elides the autorelease message send in MRR when it's possible to perform the retainRV/autoreleaseRV optimization, which only happens when the caller (or a caller higher up in the call-chain if it's tail-called) is compiled with ARC. IIUC, the only semantic change we''ll be making if we emit an objc_autoreleaseReturnValue call instead of an objc_autorelease call is that we no longer guarantee that the message send will happen when the class overrides the autorelease method, which we do guarantee with calls to objc_autorelease (see objc_object::autorelease and objc_object::rootAutorelease in the link below).

In D61808#1505608, @rjmccall wrote:

In D61808#1505544, @ahatanak wrote:

In D61808#1505023, @rjmccall wrote:

The assumption that these methods return the receiver is not true without additional semantic assumptions about these methods, so the current behavior of unconditionally turning [x autorelease] into objc_autorelease(x), x seems quite broken.

Because it's possible to override an autorelease method and make the method return something other than the passed argument, for example?

If it's not possible to assume that the method returns the passed argument in MRR, the direction of this patch and https://reviews.llvm.org/D61970 is completely wrong.

Well, objc_retain and objc_autorelease (and objc_release, trivially) do return the normal return value of the message send in MRR, so assuming that's preserved in the IR, you just need to use that as the expression result. But if the backend's going to unconditionally optimize the intrinsics based on the assumption that those values are the same, that's problematic, just as it would be problematic for it to do ARC-approved reordering on intrinsics emitted for MRR.

In D61808#1505023, @rjmccall wrote:

The assumption that these methods return the receiver is not true without additional semantic assumptions about these methods, so the current behavior of unconditionally turning [x autorelease] into objc_autorelease(x), x seems quite broken.

Run the main ARC optimization passes when ObjCNoBuiltinRetainRelease is not set.