r293199 - Turn on -Wblock-capture-autoreleasing by default.

[Nico Weber via cfe-commits]

If I'm understanding you correctly, you're saying that clang implicitly
marks the indirect NSError** parameter with __autoreleasing, so there's no
bug here. Is that correct? If so, should Wblock-capture-autoreleasing just
not fire on parameters that are already implicitly marked as
__autoreleasing?

On Mon, Feb 13, 2017 at 1:54 AM, Akira Hatanaka <ahatanaka at apple.com> wrote:

> Hi Nico,
>
> The documentation might confuse people now that the warning is on by
> default, but it’s correct as clang still qualifies indirect parameters with
> __autoreleasing. We had to add this warning and turn it on by default
> because a lot of people capture indirect parameters in their code not
> realizing the object assigned might get released by an autorelease pool.
>
> Perhaps we can change the warning message or the documentation to make it
> clear that the parameter has been implicitly marked __autoreleasing
> regardless.
>
> On Feb 11, 2017, at 11:54 AM, Nico Weber <thakis at chromium.org> wrote:
>
> Hi Akira,
>
> this fires in internal chrome/ios code like so:
>
> somefile.m: error: block captures an autoreleasing out-parameter, which
> may result in use-after-free bugs [-Werror,-Wblock-capture-autoreleasing]
>           if (error) {
>               ^
> somefile.m: note: declare the parameter __autoreleasing explicitly to
> suppress this warning
> - (NSDictionary *)fooWithError:(NSError **)error {
>                                                   ^
>                                                   __autoreleasing
> somefile.m: note: declare the parameter __strong or capture a __block
> __strong variable to keep values alive across autorelease pools
>
>
> A colleague points out that http://clang.llvm.org/docs/
> AutomaticReferenceCounting.html#indirect-parameters suggests NSError **
> should "be implicitly qualified with __autoreleasing." Is that a bug in the
> implementation of the warning, is AutomaticReferenceCounting.html
> incorrect, or are we just misunderstanding that document?
>
> Thanks,
> Nico
>
> On Thu, Jan 26, 2017 at 1:51 PM, Akira Hatanaka via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: ahatanak
>> Date: Thu Jan 26 12:51:10 2017
>> New Revision: 293199
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=293199&view=rev
>> Log:
>> Turn on -Wblock-capture-autoreleasing by default.
>>
>> Turning on the warning by default helps the users as it's a common
>> mistake to capture out-parameters in a block without ensuring the object
>> assigned doesn't get released.
>>
>> rdar://problem/30200058
>>
>> Modified:
>>     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>     cfe/trunk/test/SemaObjC/arc.m
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
>> Basic/DiagnosticSemaKinds.td?rev=293199&r1=293198&r2=293199&view=diff
>> ============================================================
>> ==================
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Jan 26
>> 12:51:10 2017
>> @@ -5185,7 +5185,7 @@ def err_arc_inconsistent_property_owners
>>  def warn_block_capture_autoreleasing : Warning<
>>    "block captures an autoreleasing out-parameter, which may result in "
>>    "use-after-free bugs">,
>> -  InGroup<BlockCaptureAutoReleasing>, DefaultIgnore;
>> +  InGroup<BlockCaptureAutoReleasing>;
>>  def note_declare_parameter_autoreleasing : Note<
>>    "declare the parameter __autoreleasing explicitly to suppress this
>> warning">;
>>  def note_declare_parameter_strong : Note<
>>
>> Modified: cfe/trunk/test/SemaObjC/arc.m
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/
>> arc.m?rev=293199&r1=293198&r2=293199&view=diff
>> ============================================================
>> ==================
>> --- cfe/trunk/test/SemaObjC/arc.m (original)
>> +++ cfe/trunk/test/SemaObjC/arc.m Thu Jan 26 12:51:10 2017
>> @@ -1,4 +1,4 @@
>> -// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak
>> -fsyntax-only -fobjc-arc -fblocks -verify -Wno-objc-root-class
>> -Wblock-capture-autoreleasing %s
>> +// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak
>> -fsyntax-only -fobjc-arc -fblocks -verify -Wno-objc-root-class %s
>>
>>  typedef unsigned long NSUInteger;
>>  typedef const void * CFTypeRef;
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170213/be5682b3/attachment-0001.html>