<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>Hm. Here's that whole function (before the change):</div><div><br></div><div><div style="margin: 0px; font-size: 11px; font-family: Menlo; "><div style="margin: 0px; "><span style="color: #4f8187">ACCResult</span> checkCallToFunction(<span style="color: #4f8187">FunctionDecl</span> *fn) {</div><div style="margin: 0px; color: rgb(0, 132, 0); "><span style="color: #000000"> </span>// Require a CF*Ref return type.</div><div style="margin: 0px; color: rgb(49, 89, 93); "><span style="color: #000000"> </span><span style="color: #bb2ca2">if</span><span style="color: #000000"> (!</span>isCFType<span style="color: #000000">(fn-></span>getResultType<span style="color: #000000">()))</span></div><div style="margin: 0px; color: rgb(49, 89, 93); "><span style="color: #000000"> </span><span style="color: #bb2ca2">return</span><span style="color: #000000"> </span>ACC_invalid<span style="color: #000000">;</span></div><div style="margin: 0px; color: rgb(49, 89, 93); "><span style="color: #000000"><br></span></div><div style="margin: 0px; color: rgb(49, 89, 93); "><span style="color: #000000"> </span><span style="color: #bb2ca2">if</span><span style="color: #000000"> (!</span>isAnyRetainable<span style="color: #000000">(</span><span style="color: #4f8187">TargetClass</span><span style="color: #000000">))</span></div><div style="margin: 0px; color: rgb(49, 89, 93); "><span style="color: #000000"> </span><span style="color: #bb2ca2">return</span><span style="color: #000000"> </span>ACC_invalid<span style="color: #000000">;</span></div><div style="margin: 0px; color: rgb(0, 132, 0); "><span style="color: #000000"><br></span></div><div style="margin: 0px; color: rgb(0, 132, 0); "><span style="color: #000000"> </span>// Honor an explicit 'not retained' attribute.</div><div style="margin: 0px; color: rgb(112, 61, 170); "><span style="color: #000000"> </span><span style="color: #bb2ca2">if</span><span style="color: #000000"> (fn-></span><span style="color: #31595d">hasAttr</span><span style="color: #000000"><</span>CFReturnsNotRetainedAttr<span style="color: #000000">>())</span></div><div style="margin: 0px; color: rgb(49, 89, 93); "><span style="color: #000000"> </span><span style="color: #bb2ca2">return</span><span style="color: #000000"> </span>ACC_plusZero<span style="color: #000000">;</span></div><div style="margin: 0px; color: rgb(0, 132, 0); "><span style="color: #000000"><br></span></div><div style="margin: 0px; color: rgb(0, 132, 0); "><span style="color: #000000"> </span>// Honor an explicit 'retained' attribute, except that for</div><div style="margin: 0px; color: rgb(0, 132, 0); "><span style="color: #000000"> </span>// now we're not going to permit implicit handling of +1 results,</div><div style="margin: 0px; color: rgb(0, 132, 0); "><span style="color: #000000"> </span>// because it's a bit frightening.</div><div style="margin: 0px; color: rgb(112, 61, 170); "><span style="color: #000000"> </span><span style="color: #bb2ca2">if</span><span style="color: #000000"> (fn-></span><span style="color: #31595d">hasAttr</span><span style="color: #000000"><</span>CFReturnsRetainedAttr<span style="color: #000000">>())</span></div><div style="margin: 0px; color: rgb(0, 132, 0); "><span style="color: #000000"> </span><span style="color: #bb2ca2">return</span><span style="color: #000000"> </span><span style="color: #31595d">ACC_invalid</span><span style="color: #000000">; </span>// ACC_plusOne if we start accepting this</div><div style="margin: 0px; color: rgb(0, 132, 0); "><span style="color: #000000"><br></span></div><div style="margin: 0px; color: rgb(0, 132, 0); "><span style="color: #000000"> </span>// Recognize this specific builtin function, which is used by CFSTR.</div><div style="margin: 0px; "> <span style="color: #bb2ca2">unsigned</span> builtinID = fn-><span style="color: #31595d">getBuiltinID</span>();</div><div style="margin: 0px; color: rgb(49, 89, 93); "><span style="color: #000000"> </span><span style="color: #bb2ca2">if</span><span style="color: #000000"> (builtinID == </span><span style="color: #4f8187">Builtin</span><span style="color: #000000">::</span>BI__builtin___CFStringMakeConstantString<span style="color: #000000">)</span></div><div style="margin: 0px; color: rgb(49, 89, 93); "><span style="color: #000000"> </span><span style="color: #bb2ca2">return</span><span style="color: #000000"> </span>ACC_bottom<span style="color: #000000">;</span></div><div style="margin: 0px; color: rgb(0, 132, 0); "><span style="color: #000000"><br></span></div><div style="margin: 0px; color: rgb(0, 132, 0); "><span style="color: #000000"> </span>// Otherwise, don't do anything implicit with an unaudited function.</div><div style="margin: 0px; color: rgb(112, 61, 170); "><span style="color: #000000"> </span><span style="color: #bb2ca2">if</span><span style="color: #000000"> (!fn-></span><span style="color: #31595d">hasAttr</span><span style="color: #000000"><</span>CFAuditedTransferAttr<span style="color: #000000">>())</span></div><div style="margin: 0px; color: rgb(49, 89, 93); "><span style="color: #000000"> </span><span style="color: #bb2ca2">return</span><span style="color: #000000"> </span>ACC_invalid<span style="color: #000000">;</span></div><div style="margin: 0px; color: rgb(0, 132, 0); "><span style="color: #000000"><br></span></div><div style="margin: 0px; color: rgb(0, 132, 0); "><span style="color: #000000"> </span>// Otherwise, it's +0 unless it follows the create convention.</div><div style="margin: 0px; color: rgb(79, 129, 135); "><span style="color: #000000"> </span><span style="color: #bb2ca2">if</span><span style="color: #000000"> (</span>ento<span style="color: #000000">::</span>coreFoundation<span style="color: #000000">::</span><span style="color: #31595d">followsCreateRule</span><span style="color: #000000">(fn))</span></div><div style="margin: 0px; color: rgb(0, 132, 0); "><span style="color: #000000"> </span><span style="color: #bb2ca2">return</span><span style="color: #000000"> </span><span style="color: #31595d">ACC_invalid</span><span style="color: #000000">; </span>// ACC_plusOne if we start accepting this</div><div style="margin: 0px; color: rgb(49, 89, 93); "><span style="color: #000000"><br></span></div><div style="margin: 0px; color: rgb(49, 89, 93); "><span style="color: #000000"> </span><span style="color: #bb2ca2">return</span><span style="color: #000000"> </span>ACC_plusZero<span style="color: #000000">;</span></div><div style="margin: 0px; ">}</div></div></div><div><br></div><div>I wonder if leaving the "followsCreateRule" after the CFAuditedTransferAttr check would be better; relying on the naming convention for un-audited code may be a bad idea even for fixits. On the other hand, we can go ahead and turn on the CFReturnsRetainedAttr case (for diagnostics only, of course), since if that attribute was added the semantics of the function have implicitly been audited.</div><div><br></div><div>On the plus side, you should be able to check for ACC_plusZero as well, and only suggest __bridge in that case.</div><div><br></div><br><div><div>On Jul 27, 2012, at 15:37 , Fariborz Jahanian <<a href="mailto:fjahanian@apple.com">fjahanian@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">Author: fjahanian<br>Date: Fri Jul 27 17:37:07 2012<br>New Revision: 160900<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=160900&view=rev">http://llvm.org/viewvc/llvm-project?rev=160900&view=rev</a><br>Log:<br>objective-c arc: When function calls with known CFCreate naming convention<br>are cast to retainable types, only suggest CFBridgingRelease/<br>CFBridgingRetain and not the __bridge casts.<br>// <a href="rdar://11923822">rdar://11923822</a><br><br>Modified:<br> cfe/trunk/lib/Sema/SemaExprObjC.cpp<br> cfe/trunk/test/SemaObjC/arc-bridged-cast.m<br> cfe/trunk/test/SemaObjC/arc-cf.m<br> cfe/trunk/test/SemaObjC/arc-unbridged-cast.m<br> cfe/trunk/test/SemaObjCXX/arc-unbridged-cast.mm<br><br>Modified: cfe/trunk/lib/Sema/SemaExprObjC.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=160900&r1=160899&r2=160900&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=160900&r1=160899&r2=160900&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/Sema/SemaExprObjC.cpp (original)<br>+++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Fri Jul 27 17:37:07 2012<br>@@ -2541,6 +2541,7 @@<br> ASTContext &Context;<br> ARCConversionTypeClass SourceClass;<br> ARCConversionTypeClass TargetClass;<br>+ bool Diagnose;<br><br> static bool isCFType(QualType type) {<br> // Someday this can use ns_bridged. For now, it has to do this.<br>@@ -2549,8 +2550,9 @@<br><br> public:<br> ARCCastChecker(ASTContext &Context, ARCConversionTypeClass source,<br>- ARCConversionTypeClass target)<br>- : Context(Context), SourceClass(source), TargetClass(target) {}<br>+ ARCConversionTypeClass target, bool diagnose)<br>+ : Context(Context), SourceClass(source), TargetClass(target),<br>+ Diagnose(diagnose) {}<br><br> using super::Visit;<br> ACCResult Visit(Expr *e) {<br>@@ -2675,14 +2677,15 @@<br> if (builtinID == Builtin::BI__builtin___CFStringMakeConstantString)<br> return ACC_bottom;<br><br>+ // Otherwise, it's +0 unless it follows the create convention.<br>+ if (ento::coreFoundation::followsCreateRule(fn))<br>+ return Diagnose ? ACC_plusOne <br>+ : ACC_invalid; // ACC_plusOne if we start accepting this<br>+ <br> // Otherwise, don't do anything implicit with an unaudited function.<br> if (!fn->hasAttr<CFAuditedTransferAttr>())<br> return ACC_invalid;<br><br>- // Otherwise, it's +0 unless it follows the create convention.<br>- if (ento::coreFoundation::followsCreateRule(fn))<br>- return ACC_invalid; // ACC_plusOne if we start accepting this<br>-<br> return ACC_plusZero;<br> }<br><br>@@ -2856,6 +2859,10 @@<br> << castRange<br> << castExpr->getSourceRange();<br> bool br = S.isKnownName("CFBridgingRelease");<br>+ bool fCreateRule = <br>+ ARCCastChecker(S.Context, exprACTC, castACTC, true).Visit(castExpr) <br>+ == ACC_plusOne;<br>+ if (!fCreateRule)<br> {<br> DiagnosticBuilder DiagB = S.Diag(noteLoc, diag::note_arc_bridge);<br> addFixitForObjCARCConversion(S, DiagB, CCK, afterLParen,<br>@@ -2884,7 +2891,10 @@<br> << castType<br> << castRange<br> << castExpr->getSourceRange();<br>-<br>+ bool fCreateRule = <br>+ ARCCastChecker(S.Context, exprACTC, castACTC, true).Visit(castExpr) <br>+ == ACC_plusOne;<br>+ if (!fCreateRule)<br> {<br> DiagnosticBuilder DiagB = S.Diag(noteLoc, diag::note_arc_bridge);<br> addFixitForObjCARCConversion(S, DiagB, CCK, afterLParen,<br>@@ -2965,7 +2975,7 @@<br> CCK != CCK_ImplicitConversion)<br> return ACR_okay;<br><br>- switch (ARCCastChecker(Context, exprACTC, castACTC).Visit(castExpr)) {<br>+ switch (ARCCastChecker(Context, exprACTC, castACTC, false).Visit(castExpr)) {<br> // For invalid casts, fall through.<br> case ACC_invalid:<br> break;<br><br>Modified: cfe/trunk/test/SemaObjC/arc-bridged-cast.m<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-bridged-cast.m?rev=160900&r1=160899&r2=160900&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-bridged-cast.m?rev=160900&r1=160899&r2=160900&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/SemaObjC/arc-bridged-cast.m (original)<br>+++ cfe/trunk/test/SemaObjC/arc-bridged-cast.m Fri Jul 27 17:37:07 2012<br>@@ -38,30 +38,27 @@<br><br> CFTypeRef fixits() {<br> id obj1 = (id)CFCreateSomething(); // expected-error{{cast of C pointer type 'CFTypeRef' (aka 'const void *') to Objective-C pointer type 'id' requires a bridged cast}} \<br>- // expected-note{{use __bridge to convert directly (no change in ownership)}} \<br> // expected-note{{use CFBridgingRelease call to transfer ownership of a +1 'CFTypeRef' (aka 'const void *') into ARC}}<br>- // CHECK: fix-it:"{{.*}}":{40:14-40:14}:"__bridge "<br> // CHECK: fix-it:"{{.*}}":{40:17-40:17}:"CFBridgingRelease("<br> // CHECK: fix-it:"{{.*}}":{40:36-40:36}:")"<br><br> CFTypeRef cf1 = (CFTypeRef)CreateSomething(); // expected-error{{cast of Objective-C pointer type 'id' to C pointer type 'CFTypeRef' (aka 'const void *') requires a bridged cast}} \<br> // expected-note{{use __bridge to convert directly (no change in ownership)}} \<br> // expected-note{{use CFBridgingRetain call to make an ARC object available as a +1 'CFTypeRef' (aka 'const void *')}}<br>- // CHECK: fix-it:"{{.*}}":{47:20-47:20}:"__bridge "<br>- // CHECK: fix-it:"{{.*}}":{47:30-47:30}:"CFBridgingRetain("<br>- // CHECK: fix-it:"{{.*}}":{47:47-47:47}:")"<br>+ // CHECK: fix-it:"{{.*}}":{45:30-45:30}:"CFBridgingRetain("<br>+ // CHECK: fix-it:"{{.*}}":{45:47-45:47}:")"<br><br> return (obj1); // expected-error{{implicit conversion of Objective-C pointer type 'id' to C pointer type 'CFTypeRef' (aka 'const void *') requires a bridged cast}} \<br> // expected-note{{use __bridge to convert directly (no change in ownership)}} \<br> // expected-note{{use CFBridgingRetain call to make an ARC object available as a +1 'CFTypeRef' (aka 'const void *')}}<br>- // CHECK: fix-it:"{{.*}}":{54:10-54:10}:"(__bridge CFTypeRef)"<br>- // CHECK: fix-it:"{{.*}}":{54:10-54:10}:"CFBridgingRetain"<br>+ // CHECK: fix-it:"{{.*}}":{51:10-51:10}:"(__bridge CFTypeRef)"<br>+ // CHECK: fix-it:"{{.*}}":{51:10-51:10}:"CFBridgingRetain"<br> }<br><br> CFTypeRef fixitsWithSpace(id obj) {<br> return(obj); // expected-error{{implicit conversion of Objective-C pointer type 'id' to C pointer type 'CFTypeRef' (aka 'const void *') requires a bridged cast}} \<br> // expected-note{{use __bridge to convert directly (no change in ownership)}} \<br> // expected-note{{use CFBridgingRetain call to make an ARC object available as a +1 'CFTypeRef' (aka 'const void *')}}<br>- // CHECK: fix-it:"{{.*}}":{62:9-62:9}:"(__bridge CFTypeRef)"<br>- // CHECK: fix-it:"{{.*}}":{62:9-62:9}:" CFBridgingRetain"<br>+ // CHECK: fix-it:"{{.*}}":{59:9-59:9}:"(__bridge CFTypeRef)"<br>+ // CHECK: fix-it:"{{.*}}":{59:9-59:9}:" CFBridgingRetain"<br> }<br><br>Modified: cfe/trunk/test/SemaObjC/arc-cf.m<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-cf.m?rev=160900&r1=160899&r2=160900&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-cf.m?rev=160900&r1=160899&r2=160900&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/SemaObjC/arc-cf.m (original)<br>+++ cfe/trunk/test/SemaObjC/arc-cf.m Fri Jul 27 17:37:07 2012<br>@@ -14,7 +14,7 @@<br> void test0() {<br> id x;<br> x = (id) CFMakeString0(); // expected-error {{requires a bridged cast}} expected-note {{__bridge to convert directly}} expected-note {{CFBridgingRelease call to transfer}}<br>- x = (id) CFCreateString0(); // expected-error {{requires a bridged cast}} expected-note {{__bridge to convert directly}} expected-note {{CFBridgingRelease call to transfer}}<br>+ x = (id) CFCreateString0(); // expected-error {{requires a bridged cast}} expected-note {{CFBridgingRelease call to transfer}}<br> }<br><br> extern CFStringRef CFMakeString1(void) __attribute__((cf_returns_not_retained));<br>@@ -41,5 +41,5 @@<br> x = (id) CFMakeString2();<br> x = (id) CFCreateString2();<br> x = (id) CFMakeString3(); // expected-error {{requires a bridged cast}} expected-note {{__bridge to convert directly}} expected-note {{CFBridgingRelease call to transfer}}<br>- x = (id) CFCreateString3(); // expected-error {{requires a bridged cast}} expected-note {{__bridge to convert directly}} expected-note {{CFBridgingRelease call to transfer}}<br>+ x = (id) CFCreateString3(); // expected-error {{requires a bridged cast}} expected-note {{CFBridgingRelease call to transfer}}<br> }<br><br>Modified: cfe/trunk/test/SemaObjC/arc-unbridged-cast.m<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-unbridged-cast.m?rev=160900&r1=160899&r2=160900&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-unbridged-cast.m?rev=160900&r1=160899&r2=160900&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/SemaObjC/arc-unbridged-cast.m (original)<br>+++ cfe/trunk/test/SemaObjC/arc-unbridged-cast.m Fri Jul 27 17:37:07 2012<br>@@ -44,10 +44,10 @@<br> x = (id) (cond ? (void*) 0 : unauditedString()); // expected-error {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note {{use CFBridgingRelease call to}}<br> x = (id) (cond ? (CFStringRef) @"help" : unauditedString()); // expected-error {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note {{use CFBridgingRelease call to}}<br><br>- x = (id) auditedCreateString(); // expected-error {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note {{use CFBridgingRelease call to}}<br>- x = (id) (cond ? auditedCreateString() : (void*) 0); // expected-error {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note {{use CFBridgingRelease call to}}<br>- x = (id) (cond ? (void*) 0 : auditedCreateString()); // expected-error {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note {{use CFBridgingRelease call to}}<br>- x = (id) (cond ? (CFStringRef) @"help" : auditedCreateString()); // expected-error {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note {{use CFBridgingRelease call to}}<br>+ x = (id) auditedCreateString(); // expected-error {{requires a bridged cast}} expected-note {{use CFBridgingRelease call to}}<br>+ x = (id) (cond ? auditedCreateString() : (void*) 0); // expected-error {{requires a bridged cast}} expected-note {{use CFBridgingRelease call to}}<br>+ x = (id) (cond ? (void*) 0 : auditedCreateString()); // expected-error {{requires a bridged cast}} expected-note {{use CFBridgingRelease call to}}<br>+ x = (id) (cond ? (CFStringRef) @"help" : auditedCreateString()); // expected-error {{requires a bridged cast}} expected-note {{use CFBridgingRelease call to}}<br><br> x = (id) [object property];<br> x = (id) (cond ? [object property] : (void*) 0);<br><br>Modified: cfe/trunk/test/SemaObjCXX/arc-unbridged-cast.mm<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/arc-unbridged-cast.mm?rev=160900&r1=160899&r2=160900&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/arc-unbridged-cast.mm?rev=160900&r1=160899&r2=160900&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/SemaObjCXX/arc-unbridged-cast.mm (original)<br>+++ cfe/trunk/test/SemaObjCXX/arc-unbridged-cast.mm Fri Jul 27 17:37:07 2012<br>@@ -44,10 +44,10 @@<br> x = (id) (cond ? (void*) 0 : unauditedString()); // expected-error {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note {{use CFBridgingRelease call to}}<br> x = (id) (cond ? (CFStringRef) @"help" : unauditedString()); // expected-error {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note {{use CFBridgingRelease call to}}<br><br>- x = (id) auditedCreateString(); // expected-error {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note {{use CFBridgingRelease call to}}<br>- x = (id) (cond ? auditedCreateString() : (void*) 0); // expected-error {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note {{use CFBridgingRelease call to}}<br>- x = (id) (cond ? (void*) 0 : auditedCreateString()); // expected-error {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note {{use CFBridgingRelease call to}}<br>- x = (id) (cond ? (CFStringRef) @"help" : auditedCreateString()); // expected-error {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note {{use CFBridgingRelease call to}}<br>+ x = (id) auditedCreateString(); // expected-error {{requires a bridged cast}} expected-note {{use CFBridgingRelease call to}}<br>+ x = (id) (cond ? auditedCreateString() : (void*) 0); // expected-error {{requires a bridged cast}} expected-note {{use CFBridgingRelease call to}}<br>+ x = (id) (cond ? (void*) 0 : auditedCreateString()); // expected-error {{requires a bridged cast}} expected-note {{use CFBridgingRelease call to}}<br>+ x = (id) (cond ? (CFStringRef) @"help" : auditedCreateString()); // expected-error {{requires a bridged cast}} expected-note {{use CFBridgingRelease call to}}<br><br> x = (id) [object property];<br> x = (id) (cond ? [object property] : (void*) 0);<br><br><br>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits<br></blockquote></div><br></body></html>