[PATCH] [ObjC] New warning: circular containers

[jahanian]

> On Mar 4, 2015, at 6:50 AM, AlexDenisov <1101.debian at gmail.com> 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 (fjahanian at apple.com <mailto:fjahanian at apple.com>) wrote:
> 
>> 
>> On Mar 3, 2015, at 12:33 PM, AlexDenisov <1101.debian at gmail.com <mailto:1101.debian at gmail.com>> 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 (fjahanian at apple.com <mailto:fjahanian at apple.com>) wrote:
>>> 
>>>> 
>>>> On Mar 3, 2015, at 11:02 AM, jahanian <fjahanian at apple.com <mailto:fjahanian at apple.com>> 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 
>>>> cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu> 
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
> <objc_circular_container.patch>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150304/a85d07e2/attachment.html>