<div dir="ltr">On Fri, Jun 21, 2013 at 5:01 PM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div class="h5"><div>
<div>On Jun 20, 2013, at 12:49 PM, Reid Kleckner <<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>> wrote:</div><blockquote type="cite"><div dir="ltr">On Thu, Jun 20, 2013 at 1:37 PM, John McCall <span dir="ltr"><<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>></span> wrote:<br>
<div class="gmail_extra"><div class="gmail_quote">

<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><span style="color:rgb(34,34,34)">Good point, sorry.</span><br>

</div><br></div><div>Hmm.  It looks like ActOnStartOfFunctionDef, ActOnStartOfObjCMethodDef, ActOnBlockArguments, and addLambdaParameters all have basically the same loop in them, but doing slightly different checks for no particular reason.  And there's already a CheckParmsForFunctionDef method, but it's apparently not called for ObjC methods.</div>


<div><br></div><div>I think it makes sense to have a single method called in all of these cases which performs the basic checks, sets ownership, and pushes parameters into scope.  That method would be a natural place for your call to FinalizeVarWithDestructor.</div>


</div></blockquote><div><br></div><div>I ended up putting it in CheckParmsForFunctionDef.  Probably ObjC methods should be calling that?</div></div></div></div>
</blockquote></div><br></div></div><div>I was hoping that you'd do the refactoring to make that happen. :)</div></div></blockquote><div><br></div><div>I did a little with r184740.  There's still some duplication of code like:</div>
<div><div>    Param->setOwningFunction(FD);</div><div>    if (Param->getIdentifier() && FnBodyScope) {<br></div><div>      CheckShadow(FnBodyScope, Param);</div><div>      PushOnScopeChains(Param, FnBodyScope);</div>
<div>    }</div></div><div><br></div><div style>I guess ObjC methods should be calling CheckShadow?</div></div></div></div>