> On Mar 4, 2015, at 6:50 AM, AlexDenisov <[email protected]> wrote:
>
> > No. I meant add expected-note which will point to the declaration as part
> > of the diagnostic.
> Got it. New patch attached.
>
> > You mean you cannot add it after checkRetainCycles(Result); as in:
> I can, but then it would be skipped for non-arc code, which still has this
> problem (not retain cycle, but ‘corruption’ at runtime).
> —
IIRC, you are saying that this intervening code:
if (!isImplicit && Method) {
if (const ObjCPropertyDecl *Prop = Method->findPropertyDecl()) {
bool IsWeak =
Prop->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_weak;
if (!IsWeak && Sel.isUnarySelector())
IsWeak = ReturnType.getObjCLifetime() & Qualifiers::OCL_Weak;
if (IsWeak &&
!Diags.isIgnored(diag::warn_arc_repeated_use_of_weak, LBracLoc))
getCurFunction()->recordUseOfWeak(Result, Prop);
}
}
}
is necessary for non-arc code. I don’t see it but it is not a major point.
Ok to check in.
- Thanks, Fariborz
> AlexDenisov
> Software Engineer, http://alexdenisov.github.io
> <http://alexdenisov.github.io/>
> On 3 Mar 2015 at 22:27:09, jahanian ([email protected]
> <mailto:[email protected]>) wrote:
>
>>
>> On Mar 3, 2015, at 12:33 PM, AlexDenisov <[email protected]
>> <mailto:[email protected]>> wrote:
>>
>>> > Provide a note where receiver/argument has been declared.
>>> Do you mean ‘add comments’?
>>
>> No. I meant add expected-note which will point to the declaration as part of
>> the diagnostic.
>>
>>>
>>> > Place CheckObjCCircularContainer(Result) right after
>>> > checkRetainCycles(Result).
>>> It still causes weird behaviour even without ARC. Of course there is no
>>> retain cycle anymore, but app still hangs with recursion/crash.
>>>
>>
>> You mean you cannot add it after checkRetainCycles(Result);
>> as in:
>>
>> checkRetainCycles(Result);
>> CheckObjCCircularContainer(Result) ;
>>
>> if (!isImplicit && Method) {
>> if (const ObjCPropertyDecl *Prop = Method->findPropertyDecl()) {
>> bool IsWeak =
>> Prop->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_weak;
>> if (!IsWeak && Sel.isUnarySelector())
>> IsWeak = ReturnType.getObjCLifetime() & Qualifiers::OCL_Weak;
>> if (IsWeak &&
>> !Diags.isIgnored(diag::warn_arc_repeated_use_of_weak, LBracLoc))
>> getCurFunction()->recordUseOfWeak(Result, Prop);
>> }
>> }
>> }
>>
>> return MaybeBindToTemporary(Result);
>> }
>>
>>
>>
>>> > This patch does not address the general case of same expression used as
>>> > receiver and addObject argument.
>>> > Is this something that you care enough to address?
>>> Do you mean something like ’[self.array addObject:self.array]’?
>>> 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.
>>
>> Ok,
>> - Fariborz
>>
>>> --
>>> AlexDenisov
>>> Software Engineer, http://alexdenisov.github.io
>>> <http://alexdenisov.github.io/>
>>> On 3 Mar 2015 at 20:46:15, jahanian ([email protected]
>>> <mailto:[email protected]>) wrote:
>>>
>>>>
>>>> On Mar 3, 2015, at 11:02 AM, jahanian <[email protected]
>>>> <mailto:[email protected]>> wrote:
>>>>
>>>>> Patch looks good with couple of minors.
>>>>> Provide a note where receiver/argument has been declared.
>>>>> Place CheckObjCCircularContainer(Result) right after
>>>>> checkRetainCycles(Result).
>>>>
>>>> This patch does not address the general case of same expression used as
>>>> receiver and addObject argument.
>>>> Is this something that you care enough to address? Need not be in this
>>>> patch though.
>>>>
>>>>>
>>>>> - Fariborz
>>>>>
>>>>>
>>>>
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> [email protected] <mailto:[email protected]>
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>> <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
> <objc_circular_container.patch>
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
