[PATCH] D136639: [CodeGen][ObjC] Fix a memory leak that occurs when a non-trivial C struct property is set using dot notation

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.


================
Comment at: clang/test/CodeGenObjC/nontrivial-c-struct-property.m:89
+
+// CHECK: call void @__destructor_8_s0(ptr %[[AGG_TMP_ENSURED]])
+
----------------
rjmccall wrote:
> ahatanak wrote:
> > rjmccall wrote:
> > > It looks like we're copying `a` into a temporary and then copying out of that temporary into the argument slot.  I don't see any reason the extra copy is required.
> > > 
> > > Also, aren't non-trivial structs consumed as arguments?  If so, this would be an over-release.
> > It isn't an over-release because, if the receiver isn't nil, temporary `AGG_TMP` is destructed in the callee and, if the receiver isn't nil, the temporary is destructed in the caller (by the call to `@__destructor_8_s0(ptr %[[AGG_TMP]])`).
> > 
> > As you pointed out, the copy isn't needed if the result of the assignment isn't used.
> Oh, is this second destructor call in conditionally-executed code?  Can we test for that, at least?
The first copy constructor call is emitted when `emitPseudoObjectExpr` evaluates the `OpaqueValueExpr` that is the result expression. The second copy constructor call is emitted to copy the argument `*a` to the argument temporary.

Since the result expression isn't used in this case, there isn't any reason to emit the first copy constructor call. If Sema marks the `OpaqueValueExpr` for the result expression as unique when the result is unused, CodeGen avoids emitting the first copy constructor.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136639/new/

https://reviews.llvm.org/D136639