<html><head><style>body{font-family:Helvetica,Arial;font-size:13px}</style></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div id="bloop_customfont" style="font-family:Helvetica,Arial;font-size:13px; color: rgba(0,0,0,1.0); margin: 0px; line-height: auto;">My point was that this check needs to be applied for both ARC and non-ARC code.</div><div id="bloop_customfont" style="font-family:Helvetica,Arial;font-size:13px; color: rgba(0,0,0,1.0); margin: 0px; line-height: auto;">Non-ARC code: weird behaviour, ARC code: weird behaviour + retain cycle.</div><div id="bloop_customfont" style="font-family:Helvetica,Arial;font-size:13px; color: rgba(0,0,0,1.0); margin: 0px; line-height: auto;">Anyway, going to commit, if it’s ok.</div> <div id="bloop_sign_1425489666858618112" class="bloop_sign"><div style="font-family:helvetica,arial;font-size:13px">-- <br>AlexDenisov</div><div style="font-family:helvetica,arial;font-size:13px">Software Engineer, <a href="http://alexdenisov.github.io"><span style="color: rgb(0, 0, 0);">http://alexdenisov.github.io</span></a></div></div> <br><p style="color:#000;">On 4 Mar 2015 at 18:13:10, jahanian (<a href="mailto:fjahanian@apple.com">fjahanian@apple.com</a>) wrote:</p> <blockquote type="cite" class="clean_bq"><span><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div></div><div>
<title></title>
<br class="">
<div>
<blockquote type="cite" class="">
<div class="">On Mar 4, 2015, at 6:50 AM, AlexDenisov <<a href="mailto:1101.debian@gmail.com" class="">1101.debian@gmail.com</a>> wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; margin: 0px;" class="">> No. I meant add expected-note which will point to the
declaration as part of the diagnostic.</div>
<div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; margin: 0px;" class="">Got it. New patch attached.</div>
<div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; margin: 0px;" class=""><br class=""></div>
<div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; margin: 0px;" class="">> You mean you cannot add it after
checkRetainCycles(Result); as in:</div>
<div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; margin: 0px;" class="">I can, but then it would be skipped for non-arc code,
which still has this problem (not retain cycle, but ‘corruption’ at
runtime).</div>
<div id="bloop_sign_1425480471465657088" class="bloop_sign" style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">
<div style="font-family: helvetica, arial; font-size: 13px;" class="">— <br class=""></div>
</div>
</div>
</blockquote>
<div><br class=""></div>
IIRC, you are saying that this intervening code:</div>
<div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">if <span style="font-variant-ligatures: no-common-ligatures; background-color: #00e6e5" class="">(</span>!isImplicit && Method<span style="font-variant-ligatures: no-common-ligatures; background-color: #00e6e5" class="">)</span> {</div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""> if (const ObjCPropertyDecl *Prop =
Method->findPropertyDecl()) {</div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""> bool IsWeak =</div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">
Prop->getPropertyAttributes() &
ObjCPropertyDecl::OBJC_PR_weak;</div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""> if (!IsWeak &&
Sel.isUnarySelector())</div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""> IsWeak =
ReturnType.getObjCLifetime() & Qualifiers::OCL_Weak;</div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""> if (IsWeak &&</div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">
!Diags.isIgnored(diag::warn_arc_repeated_use_of_weak,
LBracLoc))</div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">
getCurFunction()->recordUseOfWeak(Result, Prop);</div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""> }</div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""> }</div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""> }</div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><br class=""></div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">is necessary for non-arc code. I don’t see it but it is
not a major point.</div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">Ok to check in.</div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">- Thanks, Fariborz</div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><br class=""></div>
<blockquote type="cite" class="">
<div class="">
<div id="bloop_sign_1425480471465657088" class="bloop_sign" style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">
<div style="font-family: helvetica, arial; font-size: 13px;" class="">AlexDenisov</div>
<div style="font-family: helvetica, arial; font-size: 13px;" class="">Software Engineer, <a href="http://alexdenisov.github.io/" class=""><span style="" class="">http://alexdenisov.github.io</span></a></div>
</div>
<br style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
<p style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">On 3 Mar 2015 at 22:27:09, jahanian (<a href="mailto:fjahanian@apple.com" class="">fjahanian@apple.com</a>)
wrote:</p>
<blockquote type="cite" class="clean_bq" style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">
<div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">
<div class=""></div>
<div class=""><span class=""><br class=""></span>
<div class="">
<div class=""><span class="">On Mar 3, 2015, at 12:33 PM,
AlexDenisov <<a href="mailto:1101.debian@gmail.com" class="">1101.debian@gmail.com</a>> wrote:</span></div>
<span class=""><br class="Apple-interchange-newline"></span>
<blockquote type="cite" class="">
<div style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">
<div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">> Provide a note where
receiver/argument has been declared.</span></div>
<div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">Do you mean ‘add comments’?</span></div>
</div>
</blockquote>
<div class=""><span class=""><br class=""></span></div>
<span class="">No. I meant add expected-note which will point to
the declaration as part of the diagnostic.</span></div>
<div class=""><span class=""><br class=""></span>
<blockquote type="cite" class="">
<div style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">
<div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><br class=""></span></div>
<div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">> Place
CheckObjCCircularContainer(Result) right after
checkRetainCycles(Result).</span></div>
<div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class="">It still causes weird behaviour even
without ARC. Of course there is no retain cycle anymore, but app
still hangs with recursion/crash.</span></div>
<div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><span class=""><br class=""></span></div>
</div>
</blockquote>
<div class=""><span class=""><br class=""></span></div>
<span class="">You mean you cannot add it after <span style="color: rgb(49, 89, 93); font-family: Menlo; font-size: 11px;" class="">checkRetainCycles</span><span style="font-family: Menlo; font-size: 11px;" class="">(Result); </span></span></div>
<div class=""><font face="Menlo" class=""><span style="font-size: 11px;" class="">as in:</span></font></div>
<div class=""><font face="Menlo" class=""><span style="font-size: 11px;" class=""><br class=""></span></font></div>
<div class="">
<div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(49, 89, 93);" class="">checkRetainCycles<span style="" class="">(Result);</span></div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(49, 89, 93);" class=""><span style="font-family: Helvetica, Arial; font-size: 13px;" class="">CheckObjCCircularContainer(Result) ;</span></div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(49, 89, 93);" class=""><span style="font-family: Helvetica, Arial; font-size: 13px;" class=""><br class=""></span></div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(49, 89, 93);" class="">
<div style="margin: 0px;" class=""><span style="color: rgb(187, 44, 162);" class="">if</span><span class="Apple-converted-space"> </span>(!isImplicit &&
Method) {</div>
<div style="margin: 0px;" class=""> <span class="Apple-converted-space"> </span><span style="color: rgb(187, 44, 162);" class="">if</span><span class="Apple-converted-space"> </span>(<span style="color: rgb(187, 44, 162);" class="">const</span><span class="Apple-converted-space"> </span><span style="color: rgb(79, 129, 135);" class="">ObjCPropertyDecl</span><span class="Apple-converted-space"> </span>*Prop =
Method->findPropertyDecl()) {</div>
<div style="margin: 0px;" class="">
<span class="Apple-converted-space"> </span><span style="color: rgb(187, 44, 162);" class="">bool</span><span class="Apple-converted-space"> </span>IsWeak =</div>
<div style="margin: 0px;" class=""><span style="" class="">
Prop-></span>getPropertyAttributes<span style="" class="">()
&</span><span class="Apple-converted-space"> </span><span style="color: rgb(79, 129, 135);" class="">ObjCPropertyDecl</span><span style="" class="">::</span>OBJC_PR_weak<span style="" class="">;</span></div>
<div style="margin: 0px;" class="">
<span class="Apple-converted-space"> </span><span style="color: rgb(187, 44, 162);" class="">if</span><span class="Apple-converted-space"> </span>(!IsWeak &&
Sel.isUnarySelector())</div>
<div style="margin: 0px;" class="">
IsWeak = ReturnType.getObjCLifetime() &<span class="Apple-converted-space"> </span><span style="color: rgb(79, 129, 135);" class="">Qualifiers</span>::OCL_Weak;</div>
<div style="margin: 0px;" class="">
<span class="Apple-converted-space"> </span><span style="color: rgb(187, 44, 162);" class="">if</span><span class="Apple-converted-space"> </span>(IsWeak &&</div>
<div style="margin: 0px;" class="">
!Diags.isIgnored(diag::warn_arc_repeated_use_of_weak,
LBracLoc))</div>
<div style="margin: 0px;" class="">
getCurFunction()->recordUseOfWeak(Result, Prop);</div>
<div style="margin: 0px;" class=""> }</div>
<div style="margin: 0px;" class=""> }</div>
<div style="margin: 0px;" class=""> }</div>
<div style="margin: 0px;" class=""><br class=""></div>
<div style="margin: 0px; min-height: 13px;" class="">
<span style="color: rgb(187, 44, 162);" class="">return</span><span class="Apple-converted-space"> </span>MaybeBindToTemporary<span style="" class="">(Result);</span></div>
<div style="margin: 0px; min-height: 13px;" class="">}</div>
<div style="margin: 0px; min-height: 13px;" class="">
<br class="webkit-block-placeholder"></div>
<div class=""><br class=""></div>
</div>
<div style="margin: 0px; font-size: 11px; font-family: Menlo; color: rgb(49, 89, 93);" class=""><span style="" class=""><br class=""></span></div>
<blockquote type="cite" class="">
<div style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">
<div id="bloop_customfont" style="margin: 0px;" class="">
<span style="font-family: Helvetica, Arial; font-size: 13px;" class="">><span class="Apple-converted-space"> </span></span>This patch does not
address the general case of same expression used as receiver and
addObject argument.</div>
<div id="bloop_customfont" style="margin: 0px;" class="">> Is
this something that you care enough to address?</div>
<div id="bloop_customfont" style="margin: 0px;" class="">Do you
mean something like ’[self.array addObject:self.array]’?</div>
<div id="bloop_customfont" style="margin: 0px;" class="">If so,
then it doesn’t really makes sense, because we can’t ensure that
returned objects are the same, there’ll be false positives.</div>
</div>
</blockquote>
<div class=""><br class=""></div>
Ok,</div>
<div class="">- Fariborz</div>
<div class=""><br class="">
<blockquote type="cite" class="">
<div style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">
<div id="bloop_sign_1425414448608411136" class="bloop_sign">
<div style="font-family: helvetica, arial; font-size: 13px;" class="">-- <br class="">
AlexDenisov</div>
<div style="font-family: helvetica, arial; font-size: 13px;" class="">Software Engineer, <a href="http://alexdenisov.github.io/" class=""><span class="">http://alexdenisov.github.io</span></a></div>
</div>
<br class="">
<p class="">On 3 Mar 2015 at 20:46:15, jahanian (<a href="mailto:fjahanian@apple.com" class="">fjahanian@apple.com</a>)
wrote:</p>
<blockquote type="cite" class="clean_bq"><span class=""><br class=""></span>
<div class="">
<div class=""><span class="">On Mar 3, 2015, at 11:02 AM, jahanian
<<a href="mailto:fjahanian@apple.com" class="">fjahanian@apple.com</a>> wrote:</span></div>
<span class=""><br class="Apple-interchange-newline"></span>
<blockquote type="cite" class="">
<div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><span class="">Patch looks good with couple of
minors.</span>
<div class=""><span class="">Provide a note where receiver/argument
has been declared.</span></div>
<div class=""><span class="">Place CheckObjCCircularContainer(Result) right
after <span style="color: rgb(49, 89, 93); font-family: Menlo; font-size: 11px;" class="">checkRetainCycles</span><span style="font-family: Menlo; font-size: 11px;" class="">(Result).</span></span></div>
</div>
</blockquote>
<div class=""><br class=""></div>
This patch does not address the general case of same expression
used as receiver and addObject argument.
<div class="">Is this something that you care enough to address?
Need not be in this patch though.</div>
<div class=""><br class=""></div>
<blockquote type="cite" class="">
<div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">
<div class=""><span style="font-family: Menlo; font-size: 11px;" class=""><br class=""></span></div>
<div class=""><span style="font-family: Menlo; font-size: 11px;" class="">- Fariborz</span></div>
<div class=""><span style="font-family: Menlo; font-size: 11px;" class=""><br class=""></span></div>
<div class=""><br class=""></div>
</div>
</blockquote>
</div>
<br class="">
_______________________________________________ <br class="">
cfe-commits mailing list <br class="">
<a href="mailto:cfe-commits@cs.uiuc.edu" class="">cfe-commits@cs.uiuc.edu</a> <br class="">
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" class="">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a></blockquote>
</div>
</blockquote>
</div>
<br class=""></div>
</div>
</blockquote>
<span id="cid:EC84B90C-10DF-4709-83BF-8E64F34A450A@attlocal.net"><objc_circular_container.patch></span></div>
</blockquote>
</div>
<br class="">
</div></div></span></blockquote></body></html>