Hm. Here's that whole function (before the change):
ACCResult checkCallToFunction(FunctionDecl *fn) {
// Require a CF*Ref return type.
if (!isCFType(fn->getResultType()))
return ACC_invalid;
if (!isAnyRetainable(TargetClass))
return ACC_invalid;
// Honor an explicit 'not retained' attribute.
if (fn->hasAttr<CFReturnsNotRetainedAttr>())
return ACC_plusZero;
// Honor an explicit 'retained' attribute, except that for
// now we're not going to permit implicit handling of +1 results,
// because it's a bit frightening.
if (fn->hasAttr<CFReturnsRetainedAttr>())
return ACC_invalid; // ACC_plusOne if we start accepting this
// Recognize this specific builtin function, which is used by CFSTR.
unsigned builtinID = fn->getBuiltinID();
if (builtinID == Builtin::BI__builtin___CFStringMakeConstantString)
return ACC_bottom;
// Otherwise, don't do anything implicit with an unaudited function.
if (!fn->hasAttr<CFAuditedTransferAttr>())
return ACC_invalid;
// Otherwise, it's +0 unless it follows the create convention.
if (ento::coreFoundation::followsCreateRule(fn))
return ACC_invalid; // ACC_plusOne if we start accepting this
return ACC_plusZero;
}
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.
On the plus side, you should be able to check for ACC_plusZero as well, and
only suggest __bridge in that case.
On Jul 27, 2012, at 15:37 , Fariborz Jahanian <[email protected]> wrote:
> Author: fjahanian
> Date: Fri Jul 27 17:37:07 2012
> New Revision: 160900
>
> URL: http://llvm.org/viewvc/llvm-project?rev=160900&view=rev
> Log:
> objective-c arc: When function calls with known CFCreate naming convention
> are cast to retainable types, only suggest CFBridgingRelease/
> CFBridgingRetain and not the __bridge casts.
> // rdar://11923822
>
> Modified:
> cfe/trunk/lib/Sema/SemaExprObjC.cpp
> cfe/trunk/test/SemaObjC/arc-bridged-cast.m
> cfe/trunk/test/SemaObjC/arc-cf.m
> cfe/trunk/test/SemaObjC/arc-unbridged-cast.m
> cfe/trunk/test/SemaObjCXX/arc-unbridged-cast.mm
>
> Modified: cfe/trunk/lib/Sema/SemaExprObjC.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=160900&r1=160899&r2=160900&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExprObjC.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Fri Jul 27 17:37:07 2012
> @@ -2541,6 +2541,7 @@
> ASTContext &Context;
> ARCConversionTypeClass SourceClass;
> ARCConversionTypeClass TargetClass;
> + bool Diagnose;
>
> static bool isCFType(QualType type) {
> // Someday this can use ns_bridged. For now, it has to do this.
> @@ -2549,8 +2550,9 @@
>
> public:
> ARCCastChecker(ASTContext &Context, ARCConversionTypeClass source,
> - ARCConversionTypeClass target)
> - : Context(Context), SourceClass(source), TargetClass(target) {}
> + ARCConversionTypeClass target, bool diagnose)
> + : Context(Context), SourceClass(source), TargetClass(target),
> + Diagnose(diagnose) {}
>
> using super::Visit;
> ACCResult Visit(Expr *e) {
> @@ -2675,14 +2677,15 @@
> if (builtinID == Builtin::BI__builtin___CFStringMakeConstantString)
> return ACC_bottom;
>
> + // Otherwise, it's +0 unless it follows the create convention.
> + if (ento::coreFoundation::followsCreateRule(fn))
> + return Diagnose ? ACC_plusOne
> + : ACC_invalid; // ACC_plusOne if we start accepting
> this
> +
> // Otherwise, don't do anything implicit with an unaudited function.
> if (!fn->hasAttr<CFAuditedTransferAttr>())
> return ACC_invalid;
>
> - // Otherwise, it's +0 unless it follows the create convention.
> - if (ento::coreFoundation::followsCreateRule(fn))
> - return ACC_invalid; // ACC_plusOne if we start accepting this
> -
> return ACC_plusZero;
> }
>
> @@ -2856,6 +2859,10 @@
> << castRange
> << castExpr->getSourceRange();
> bool br = S.isKnownName("CFBridgingRelease");
> + bool fCreateRule =
> + ARCCastChecker(S.Context, exprACTC, castACTC, true).Visit(castExpr)
> + == ACC_plusOne;
> + if (!fCreateRule)
> {
> DiagnosticBuilder DiagB = S.Diag(noteLoc, diag::note_arc_bridge);
> addFixitForObjCARCConversion(S, DiagB, CCK, afterLParen,
> @@ -2884,7 +2891,10 @@
> << castType
> << castRange
> << castExpr->getSourceRange();
> -
> + bool fCreateRule =
> + ARCCastChecker(S.Context, exprACTC, castACTC, true).Visit(castExpr)
> + == ACC_plusOne;
> + if (!fCreateRule)
> {
> DiagnosticBuilder DiagB = S.Diag(noteLoc, diag::note_arc_bridge);
> addFixitForObjCARCConversion(S, DiagB, CCK, afterLParen,
> @@ -2965,7 +2975,7 @@
> CCK != CCK_ImplicitConversion)
> return ACR_okay;
>
> - switch (ARCCastChecker(Context, exprACTC, castACTC).Visit(castExpr)) {
> + switch (ARCCastChecker(Context, exprACTC, castACTC,
> false).Visit(castExpr)) {
> // For invalid casts, fall through.
> case ACC_invalid:
> break;
>
> Modified: cfe/trunk/test/SemaObjC/arc-bridged-cast.m
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-bridged-cast.m?rev=160900&r1=160899&r2=160900&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaObjC/arc-bridged-cast.m (original)
> +++ cfe/trunk/test/SemaObjC/arc-bridged-cast.m Fri Jul 27 17:37:07 2012
> @@ -38,30 +38,27 @@
>
> CFTypeRef fixits() {
> 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}} \
> - // expected-note{{use __bridge to convert directly (no change in
> ownership)}} \
> // expected-note{{use CFBridgingRelease call to transfer ownership of a +1
> 'CFTypeRef' (aka 'const void *') into ARC}}
> - // CHECK: fix-it:"{{.*}}":{40:14-40:14}:"__bridge "
> // CHECK: fix-it:"{{.*}}":{40:17-40:17}:"CFBridgingRelease("
> // CHECK: fix-it:"{{.*}}":{40:36-40:36}:")"
>
> 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}} \
> // expected-note{{use __bridge to convert directly (no change in
> ownership)}} \
> // expected-note{{use CFBridgingRetain call to make an ARC object available
> as a +1 'CFTypeRef' (aka 'const void *')}}
> - // CHECK: fix-it:"{{.*}}":{47:20-47:20}:"__bridge "
> - // CHECK: fix-it:"{{.*}}":{47:30-47:30}:"CFBridgingRetain("
> - // CHECK: fix-it:"{{.*}}":{47:47-47:47}:")"
> + // CHECK: fix-it:"{{.*}}":{45:30-45:30}:"CFBridgingRetain("
> + // CHECK: fix-it:"{{.*}}":{45:47-45:47}:")"
>
> return (obj1); // expected-error{{implicit conversion of Objective-C
> pointer type 'id' to C pointer type 'CFTypeRef' (aka 'const void *') requires
> a bridged cast}} \
> // expected-note{{use __bridge to convert directly (no change in
> ownership)}} \
> // expected-note{{use CFBridgingRetain call to make an ARC object available
> as a +1 'CFTypeRef' (aka 'const void *')}}
> - // CHECK: fix-it:"{{.*}}":{54:10-54:10}:"(__bridge CFTypeRef)"
> - // CHECK: fix-it:"{{.*}}":{54:10-54:10}:"CFBridgingRetain"
> + // CHECK: fix-it:"{{.*}}":{51:10-51:10}:"(__bridge CFTypeRef)"
> + // CHECK: fix-it:"{{.*}}":{51:10-51:10}:"CFBridgingRetain"
> }
>
> CFTypeRef fixitsWithSpace(id obj) {
> return(obj); // expected-error{{implicit conversion of Objective-C pointer
> type 'id' to C pointer type 'CFTypeRef' (aka 'const void *') requires a
> bridged cast}} \
> // expected-note{{use __bridge to convert directly (no change in
> ownership)}} \
> // expected-note{{use CFBridgingRetain call to make an ARC object available
> as a +1 'CFTypeRef' (aka 'const void *')}}
> - // CHECK: fix-it:"{{.*}}":{62:9-62:9}:"(__bridge CFTypeRef)"
> - // CHECK: fix-it:"{{.*}}":{62:9-62:9}:" CFBridgingRetain"
> + // CHECK: fix-it:"{{.*}}":{59:9-59:9}:"(__bridge CFTypeRef)"
> + // CHECK: fix-it:"{{.*}}":{59:9-59:9}:" CFBridgingRetain"
> }
>
> Modified: cfe/trunk/test/SemaObjC/arc-cf.m
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-cf.m?rev=160900&r1=160899&r2=160900&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaObjC/arc-cf.m (original)
> +++ cfe/trunk/test/SemaObjC/arc-cf.m Fri Jul 27 17:37:07 2012
> @@ -14,7 +14,7 @@
> void test0() {
> id x;
> x = (id) CFMakeString0(); // expected-error {{requires a bridged cast}}
> expected-note {{__bridge to convert directly}} expected-note
> {{CFBridgingRelease call to transfer}}
> - x = (id) CFCreateString0(); // expected-error {{requires a bridged cast}}
> expected-note {{__bridge to convert directly}} expected-note
> {{CFBridgingRelease call to transfer}}
> + x = (id) CFCreateString0(); // expected-error {{requires a bridged cast}}
> expected-note {{CFBridgingRelease call to transfer}}
> }
>
> extern CFStringRef CFMakeString1(void)
> __attribute__((cf_returns_not_retained));
> @@ -41,5 +41,5 @@
> x = (id) CFMakeString2();
> x = (id) CFCreateString2();
> x = (id) CFMakeString3(); // expected-error {{requires a bridged cast}}
> expected-note {{__bridge to convert directly}} expected-note
> {{CFBridgingRelease call to transfer}}
> - x = (id) CFCreateString3(); // expected-error {{requires a bridged cast}}
> expected-note {{__bridge to convert directly}} expected-note
> {{CFBridgingRelease call to transfer}}
> + x = (id) CFCreateString3(); // expected-error {{requires a bridged cast}}
> expected-note {{CFBridgingRelease call to transfer}}
> }
>
> Modified: cfe/trunk/test/SemaObjC/arc-unbridged-cast.m
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-unbridged-cast.m?rev=160900&r1=160899&r2=160900&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaObjC/arc-unbridged-cast.m (original)
> +++ cfe/trunk/test/SemaObjC/arc-unbridged-cast.m Fri Jul 27 17:37:07 2012
> @@ -44,10 +44,10 @@
> x = (id) (cond ? (void*) 0 : unauditedString()); // expected-error
> {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note
> {{use CFBridgingRelease call to}}
> x = (id) (cond ? (CFStringRef) @"help" : unauditedString()); //
> expected-error {{requires a bridged cast}} expected-note {{use __bridge to}}
> expected-note {{use CFBridgingRelease call to}}
>
> - x = (id) auditedCreateString(); // expected-error {{requires a bridged
> cast}} expected-note {{use __bridge to}} expected-note {{use
> CFBridgingRelease call to}}
> - x = (id) (cond ? auditedCreateString() : (void*) 0); // expected-error
> {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note
> {{use CFBridgingRelease call to}}
> - x = (id) (cond ? (void*) 0 : auditedCreateString()); // expected-error
> {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note
> {{use CFBridgingRelease call to}}
> - x = (id) (cond ? (CFStringRef) @"help" : auditedCreateString()); //
> expected-error {{requires a bridged cast}} expected-note {{use __bridge to}}
> expected-note {{use CFBridgingRelease call to}}
> + x = (id) auditedCreateString(); // expected-error {{requires a bridged
> cast}} expected-note {{use CFBridgingRelease call to}}
> + x = (id) (cond ? auditedCreateString() : (void*) 0); // expected-error
> {{requires a bridged cast}} expected-note {{use CFBridgingRelease call to}}
> + x = (id) (cond ? (void*) 0 : auditedCreateString()); // expected-error
> {{requires a bridged cast}} expected-note {{use CFBridgingRelease call to}}
> + x = (id) (cond ? (CFStringRef) @"help" : auditedCreateString()); //
> expected-error {{requires a bridged cast}} expected-note {{use
> CFBridgingRelease call to}}
>
> x = (id) [object property];
> x = (id) (cond ? [object property] : (void*) 0);
>
> Modified: cfe/trunk/test/SemaObjCXX/arc-unbridged-cast.mm
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/arc-unbridged-cast.mm?rev=160900&r1=160899&r2=160900&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaObjCXX/arc-unbridged-cast.mm (original)
> +++ cfe/trunk/test/SemaObjCXX/arc-unbridged-cast.mm Fri Jul 27 17:37:07 2012
> @@ -44,10 +44,10 @@
> x = (id) (cond ? (void*) 0 : unauditedString()); // expected-error
> {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note
> {{use CFBridgingRelease call to}}
> x = (id) (cond ? (CFStringRef) @"help" : unauditedString()); //
> expected-error {{requires a bridged cast}} expected-note {{use __bridge to}}
> expected-note {{use CFBridgingRelease call to}}
>
> - x = (id) auditedCreateString(); // expected-error {{requires a bridged
> cast}} expected-note {{use __bridge to}} expected-note {{use
> CFBridgingRelease call to}}
> - x = (id) (cond ? auditedCreateString() : (void*) 0); // expected-error
> {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note
> {{use CFBridgingRelease call to}}
> - x = (id) (cond ? (void*) 0 : auditedCreateString()); // expected-error
> {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note
> {{use CFBridgingRelease call to}}
> - x = (id) (cond ? (CFStringRef) @"help" : auditedCreateString()); //
> expected-error {{requires a bridged cast}} expected-note {{use __bridge to}}
> expected-note {{use CFBridgingRelease call to}}
> + x = (id) auditedCreateString(); // expected-error {{requires a bridged
> cast}} expected-note {{use CFBridgingRelease call to}}
> + x = (id) (cond ? auditedCreateString() : (void*) 0); // expected-error
> {{requires a bridged cast}} expected-note {{use CFBridgingRelease call to}}
> + x = (id) (cond ? (void*) 0 : auditedCreateString()); // expected-error
> {{requires a bridged cast}} expected-note {{use CFBridgingRelease call to}}
> + x = (id) (cond ? (CFStringRef) @"help" : auditedCreateString()); //
> expected-error {{requires a bridged cast}} expected-note {{use
> CFBridgingRelease call to}}
>
> x = (id) [object property];
> x = (id) (cond ? [object property] : (void*) 0);
>
>
> _______________________________________________
> cfe-commits mailing list
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
