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

[Akira Hatanaka via cfe-commits]

Yes, you are correct: clang implicitly marks indirect parameters with __autoreleasing.

The whole purpose of Wblock-capture-autoreleasing (which was committed in r285031) is to inform the users that clang has implicitly marked an out-parameter as __autoreleasing. clang doesn’t warn if the user explicitly marks the parameter __autoreleasing.

> On Feb 13, 2017, at 8:09 AM, Nico Weber <thakis at chromium.org> wrote:
> 
> 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 <mailto: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 <mailto: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 <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 <mailto: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 <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 <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 <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 <mailto:cfe-commits at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <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/20170214/d59a8aac/attachment.html>