[cfe-commits] r160900 - in /cfe/trunk: lib/Sema/SemaExprObjC.cpp test/SemaObjC/arc-bridged-cast.m test/SemaObjC/arc-cf.m test/SemaObjC/arc-unbridged-cast.m test/SemaObjCXX/arc-unbridged-cast.mm
Jordan Rose
jordan_rose at apple.com
Fri Jul 27 15:49:20 PDT 2012
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 <fjahanian at apple.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120727/ad77ae08/attachment.html>
More information about the cfe-commits
mailing list