<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><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;"><span class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""></div><div class=""><br class=""><div class=""><div class="">On Mar 3, 2015, at 12:33 PM, AlexDenisov <<a href="mailto:1101.debian@gmail.com" class="">1101.debian@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><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="">> Provide a note where receiver/argument has been declared.</div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class="">Do you mean ‘add comments’?</div></div></blockquote><div class=""><br class=""></div>No. I meant add expected-note which will point to the declaration as part of the diagnostic.</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_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><br class=""></div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class="">> Place CheckObjCCircularContainer(Result) right after checkRetainCycles(Result).</div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class="">It still causes weird behaviour even without ARC. Of course there is no retain cycle anymore, but app still hangs with recursion/crash.</div><div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; margin: 0px;" class=""><br class=""></div></div></blockquote><div class=""><br class=""></div>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></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></span></blockquote><span id="cid:EC84B90C-10DF-4709-83BF-8E64F34A450A@attlocal.net"><objc_circular_container.patch></span></div></blockquote></div><br class=""></body></html>