<div dir="ltr">I agree with you, that was actually what John suggested too.<div><br></div><div>I'll update the patch shortly.<br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Feb 5, 2016 at 3:38 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
> On 2016-Feb-04, at 18:54, Akira Hatanaka via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br>
><br>
> ahatanak created this revision.<br>
> ahatanak added a subscriber: cfe-commits.<br>
><br>
> The assert near CGCall.cpp:2465 is triggered because QualType::isObjCRetainableType() is called on different types. In CodeGenFunction::StartFunction, it returns true because it is called on the typedef marked with __attribute__((NSObject)), whereas in CodeGenFunction::EmitFunctionEpilog it returns false because it's called on the canonical type.<br>
><br>
> To fix the assert, this patch changes the code in CodeGenFunction::EmitFunctionEpilog to get the function's return type from CodeGenFunction::CurCodeDecl or BlockInfo instead of from CGFunctionInfo.<br>
><br>
> <a href="http://reviews.llvm.org/D16914" rel="noreferrer" target="_blank">http://reviews.llvm.org/D16914</a><br>
<br>
</span>I have a nitpick below.  (I think you should find someone that knows<br>
this code better than I do to sign off on this though.)<br>
<div><div class="h5"><br>
> Files:<br>
>  lib/CodeGen/CGCall.cpp<br>
>  test/CodeGenObjCXX/<a href="http://auto-release-result-assert.mm" rel="noreferrer" target="_blank">auto-release-result-assert.mm</a><br>
><br>
> Index: test/CodeGenObjCXX/<a href="http://auto-release-result-assert.mm" rel="noreferrer" target="_blank">auto-release-result-assert.mm</a><br>
> ===================================================================<br>
> --- /dev/null<br>
> +++ test/CodeGenObjCXX/<a href="http://auto-release-result-assert.mm" rel="noreferrer" target="_blank">auto-release-result-assert.mm</a><br>
> @@ -0,0 +1,35 @@<br>
> +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fblocks -fobjc-arc -o - %s | FileCheck %s<br>
> +<br>
> +// CHECK-LABEL: define %struct.S1* @_Z4foo1i(<br>
> +// CHECK: %[[CALL:[a-z0-9]+]] = call %struct.S1* @_Z4foo0i<br>
> +// CHECK: ret %struct.S1* %[[CALL]]<br>
> +<br>
> +// CHECK-LABEL: define %struct.S1* @_ZN2S22m1Ev(<br>
> +// CHECK: %[[CALL:[a-z0-9]+]] = call %struct.S1* @_Z4foo0i<br>
> +// CHECK: ret %struct.S1* %[[CALL]]<br>
> +<br>
> +// CHECK-LABEL: define internal %struct.S1* @Block1_block_invoke(<br>
> +// CHECK: %[[CALL:[a-z0-9]+]] = call %struct.S1* @_Z4foo0i<br>
> +// CHECK: ret %struct.S1* %[[CALL]]<br>
> +<br>
> +struct S1;<br>
> +<br>
> +typedef __attribute__((NSObject)) struct __attribute__((objc_bridge(id))) S1 * S1Ref;<br>
> +<br>
> +S1Ref foo0(int);<br>
> +<br>
> +struct S2 {<br>
> +  S1Ref m1();<br>
> +};<br>
> +<br>
> +S1Ref foo1(int a) {<br>
> +  return foo0(a);<br>
> +}<br>
> +<br>
> +S1Ref S2::m1() {<br>
> +  return foo0(0);<br>
> +}<br>
> +<br>
> +S1Ref (^Block1)(void) = ^{<br>
> +  return foo0(0);<br>
> +};<br>
> Index: lib/CodeGen/CGCall.cpp<br>
> ===================================================================<br>
> --- lib/CodeGen/CGCall.cpp<br>
> +++ lib/CodeGen/CGCall.cpp<br>
> @@ -14,6 +14,7 @@<br>
><br>
> #include "CGCall.h"<br>
> #include "ABIInfo.h"<br>
> +#include "CGBlocks.h"<br>
> #include "CGCXXABI.h"<br>
> #include "CGCleanup.h"<br>
> #include "CodeGenFunction.h"<br>
> @@ -2462,9 +2463,21 @@<br>
>     // In ARC, end functions that return a retainable type with a call<br>
>     // to objc_autoreleaseReturnValue.<br>
>     if (AutoreleaseResult) {<br>
> +      QualType RT;<br>
> +<br>
> +      if (auto *FD = dyn_cast<FunctionDecl>(CurCodeDecl))<br>
> +        RT = FD->getReturnType();<br>
> +      else if (auto *MD = dyn_cast<ObjCMethodDecl>(CurCodeDecl))<br>
> +        RT = MD->getReturnType();<br>
> +      else if (isa<BlockDecl>(CurCodeDecl))<br>
> +        RT = BlockInfo->BlockExpression->getFunctionType()->getReturnType();<br>
> +      else<br>
> +        llvm_unreachable("Unexpected function/method type");<br>
<br>
</div></div>There's no reason for this code to run when NDEBUG, is there?<br>
<span class=""><br>
> +<br>
> +      (void)RT;<br>
>       assert(getLangOpts().ObjCAutoRefCount &&<br>
>              !FI.isReturnsRetained() &&<br>
> -             RetTy->isObjCRetainableType());<br>
> +             RT->isObjCRetainableType());<br>
<br>
</span>I think you should change this to:<br>
```<br>
#ifndef NDEBUG<br>
QualType RT;<br>
if (...)<br>
   RT = ...<br>
else if (...)<br>
   RT = ...<br>
else if (...)<br>
   RT = ...<br>
else<br>
   llvm_unreachable(...);<br>
assert(...);<br>
#endif<br>
```<br>
<br>
>       RV = emitAutoreleaseOfResult(*this, RV);<br>
>     }<br>
><br>
><br>
><br>
> <D16914.46990.patch>_______________________________________________<br>
> cfe-commits mailing list<br>
> <a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
<br>
</blockquote></div><br></div></div></div>