From 443aa4b4b0cb7098fdf6ae238ebbbed3f64b4719 Mon Sep 17 00:00:00 2001 From: Ben Langmuir Date: Wed, 25 Feb 2015 20:09:06 +0000 Subject: [PATCH] Allow (Object *)kMyGlobalCFObj casts without bridging Previously we allowed these casts only for constants declared in system headers, which we assume are retain/release-neutral. Now also allow them for constants in user headers, treating them as +0. Practically, this means that we will now allow: id x = (id)kMyGlobalConst; But unlike with system headers we cannot mix them with +1 values: id y = (id)(b ? kMyGlobalConst : [Obj newValAtPlusOne]); // error id z = (id)(b ? kSystemGlobalConst: [Obj newValAtPlusOne]); // OK Thanks to John for suggesting this improvement. llvm-svn: 230534 --- clang/docs/AutomaticReferenceCounting.rst | 10 +++++++++- clang/lib/Sema/SemaExprObjC.cpp | 15 +++++++++------ clang/test/ARCMT/nonobjc-to-objc-cast.m | 11 ++++++++--- clang/test/ARCMT/nonobjc-to-objc-cast.m.result | 11 ++++++++--- clang/test/SemaObjC/arc-dict-bridged-cast.m | 13 +++---------- clang/test/SemaObjC/arc-unbridged-cast.m | 7 +++++++ 6 files changed, 44 insertions(+), 23 deletions(-) diff --git a/clang/docs/AutomaticReferenceCounting.rst b/clang/docs/AutomaticReferenceCounting.rst index 1457b60..2faed23 100644 --- a/clang/docs/AutomaticReferenceCounting.rst +++ b/clang/docs/AutomaticReferenceCounting.rst @@ -594,7 +594,9 @@ retainable pointer type ` and it is: * a message send, and the declared method either has the ``cf_returns_not_retained`` attribute or it has neither the ``cf_returns_retained`` attribute nor a :ref:`selector family - ` that implies a retained result. + ` that implies a retained result, or +* :when-revised:`[beginning LLVM 3.6]` :revision:`a load from a` ``const`` + :revision:`non-system global variable.` An expression is :arc-term:`known retained` if it is an rvalue of :ref:`C retainable pointer type ` and it is: @@ -631,6 +633,12 @@ retain-agnostic, the conversion is treated as a ``__bridge`` cast. to an ObjC-typed local, and then calling ``CFRelease`` when done --- are a bit too likely to be accidentally accepted, leading to mysterious behavior. + For loads from ``const`` global variables of :ref:`C retainable pointer type + `, it is reasonable to assume that global system + constants were initialitzed with true constants (e.g. string literals), but + user constants might have been initialized with something dynamically + allocated, using a global initializer. + .. _arc.objects.restrictions.conversion-exception-contextual: Conversion from retainable object pointer type in certain contexts diff --git a/clang/lib/Sema/SemaExprObjC.cpp b/clang/lib/Sema/SemaExprObjC.cpp index cb1f21a..ebe6cbb 100644 --- a/clang/lib/Sema/SemaExprObjC.cpp +++ b/clang/lib/Sema/SemaExprObjC.cpp @@ -3031,17 +3031,20 @@ namespace { /// Some declaration references are okay. ACCResult VisitDeclRefExpr(DeclRefExpr *e) { - // References to global constants from system headers are okay. - // These are things like 'kCFStringTransformToLatin'. They are - // can also be assumed to be immune to retains. VarDecl *var = dyn_cast(e->getDecl()); + // References to global constants are okay. if (isAnyRetainable(TargetClass) && isAnyRetainable(SourceClass) && var && var->getStorageClass() == SC_Extern && - var->getType().isConstQualified() && - Context.getSourceManager().isInSystemHeader(var->getLocation())) { - return ACC_bottom; + var->getType().isConstQualified()) { + + // In system headers, they can also be assumed to be immune to retains. + // These are things like 'kCFStringTransformToLatin'. + if (Context.getSourceManager().isInSystemHeader(var->getLocation())) + return ACC_bottom; + + return ACC_plusZero; } // Nothing else. diff --git a/clang/test/ARCMT/nonobjc-to-objc-cast.m b/clang/test/ARCMT/nonobjc-to-objc-cast.m index b7d2a73..7913661 100644 --- a/clang/test/ARCMT/nonobjc-to-objc-cast.m +++ b/clang/test/ARCMT/nonobjc-to-objc-cast.m @@ -7,6 +7,7 @@ typedef const struct __CFString * CFStringRef; extern const CFStringRef kUTTypePlainText; extern const CFStringRef kUTTypeRTF; +extern CFStringRef kNonConst; typedef const struct __CFAllocator * CFAllocatorRef; typedef const struct __CFUUID * CFUUIDRef; @@ -28,11 +29,15 @@ struct StrS { @end void f(BOOL b, id p) { - NSString *str = (NSString *)kUTTypePlainText; - str = b ? kUTTypeRTF : kUTTypePlainText; - str = (NSString *)(b ? kUTTypeRTF : kUTTypePlainText); + NSString *str = (NSString *)kUTTypePlainText; // no change + str = b ? kUTTypeRTF : kUTTypePlainText; // no change + str = (NSString *)(b ? kUTTypeRTF : kUTTypePlainText); // no change str = (NSString *)p; // no change. + str = (NSString *)kNonConst; + str = b ? kUTTypeRTF : kNonConst; + str = (NSString *)(b ? kUTTypeRTF : kNonConst); + CFUUIDRef _uuid; NSString *_uuidString = (NSString *)CFUUIDCreateString(kCFAllocatorDefault, _uuid); _uuidString = [(NSString *)CFUUIDCreateString(kCFAllocatorDefault, _uuid) autorelease]; diff --git a/clang/test/ARCMT/nonobjc-to-objc-cast.m.result b/clang/test/ARCMT/nonobjc-to-objc-cast.m.result index ce827ba..8f3092f 100644 --- a/clang/test/ARCMT/nonobjc-to-objc-cast.m.result +++ b/clang/test/ARCMT/nonobjc-to-objc-cast.m.result @@ -7,6 +7,7 @@ typedef const struct __CFString * CFStringRef; extern const CFStringRef kUTTypePlainText; extern const CFStringRef kUTTypeRTF; +extern CFStringRef kNonConst; typedef const struct __CFAllocator * CFAllocatorRef; typedef const struct __CFUUID * CFUUIDRef; @@ -28,11 +29,15 @@ struct StrS { @end void f(BOOL b, id p) { - NSString *str = (__bridge NSString *)kUTTypePlainText; - str = (__bridge NSString *)(b ? kUTTypeRTF : kUTTypePlainText); - str = (__bridge NSString *)(b ? kUTTypeRTF : kUTTypePlainText); + NSString *str = (NSString *)kUTTypePlainText; // no change + str = b ? kUTTypeRTF : kUTTypePlainText; // no change + str = (NSString *)(b ? kUTTypeRTF : kUTTypePlainText); // no change str = (NSString *)p; // no change. + str = (__bridge NSString *)kNonConst; + str = (__bridge NSString *)(b ? kUTTypeRTF : kNonConst); + str = (__bridge NSString *)(b ? kUTTypeRTF : kNonConst); + CFUUIDRef _uuid; NSString *_uuidString = (NSString *)CFBridgingRelease(CFUUIDCreateString(kCFAllocatorDefault, _uuid)); _uuidString = (NSString *)CFBridgingRelease(CFUUIDCreateString(kCFAllocatorDefault, _uuid)); diff --git a/clang/test/SemaObjC/arc-dict-bridged-cast.m b/clang/test/SemaObjC/arc-dict-bridged-cast.m index e00c47f..e683343 100644 --- a/clang/test/SemaObjC/arc-dict-bridged-cast.m +++ b/clang/test/SemaObjC/arc-dict-bridged-cast.m @@ -28,19 +28,12 @@ id CFBridgingRelease(CFTypeRef __attribute__((cf_consumed)) X); NSMutableString *test() { NSDictionary *infoDictionary; - infoDictionary[kCFBundleNameKey] = 0; // expected-error {{indexing expression is invalid because subscript type 'CFStringRef' (aka 'const struct __CFString *') is not an integral or Objective-C pointer type}} \ - // expected-error {{implicit conversion of C pointer type 'CFStringRef' (aka 'const struct __CFString *') to Objective-C pointer type '__strong 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 'CFStringRef' (aka 'const struct __CFString *') into ARC}} + infoDictionary[kCFBundleNameKey] = 0; // expected-error {{indexing expression is invalid because subscript type 'CFStringRef' (aka 'const struct __CFString *') is not an integral or Objective-C pointer type}} return infoDictionary[CFStringCreateMutable(((void*)0), 100)]; // expected-error {{indexing expression is invalid because subscript type 'CFMutableStringRef' (aka 'struct __CFString *') is not an integral or Objective-C pointer type}} \ // expected-error {{implicit conversion of C pointer type 'CFMutableStringRef' (aka 'struct __CFString *') to Objective-C pointer type '__strong id' requires a bridged cast}} \ // expected-note {{use CFBridgingRelease call to transfer ownership of a +1 'CFMutableStringRef' (aka 'struct __CFString *') into ARC}} } -// CHECK: fix-it:"{{.*}}":{31:18-31:18}:"(__bridge __strong id)(" -// CHECK: fix-it:"{{.*}}":{31:34-31:34}:")" -// CHECK: fix-it:"{{.*}}":{31:18-31:18}:"CFBridgingRelease(" -// CHECK: fix-it:"{{.*}}":{31:34-31:34}:")" -// CHECK: fix-it:"{{.*}}":{35:25-35:25}:"CFBridgingRelease(" -// CHECK: fix-it:"{{.*}}":{35:63-35:63}:")" +// CHECK: fix-it:"{{.*}}":{32:25-32:25}:"CFBridgingRelease(" +// CHECK: fix-it:"{{.*}}":{32:63-32:63}:")" diff --git a/clang/test/SemaObjC/arc-unbridged-cast.m b/clang/test/SemaObjC/arc-unbridged-cast.m index 6a39e70..3c0e3f2 100644 --- a/clang/test/SemaObjC/arc-unbridged-cast.m +++ b/clang/test/SemaObjC/arc-unbridged-cast.m @@ -32,15 +32,20 @@ CFStringRef auditedString(void); CFStringRef auditedCreateString(void); #pragma clang arc_cf_code_audited end +extern const CFStringRef kUserConst; + void test1(int cond) { id x; x = (id) auditedString(); + x = (id) kUserConst; x = (id) (cond ? auditedString() : (void*) 0); x = (id) (cond ? (void*) 0 : auditedString()); x = (id) (cond ? (CFStringRef) @"help" : auditedString()); + x = (id) (cond ? (CFStringRef) @"help" : kUserConst); x = (id) unauditedString(); // expected-error {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note {{use CFBridgingRelease call to}} x = (id) (cond ? unauditedString() : (void*) 0); // expected-error {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note {{use CFBridgingRelease call to}} + x = (id) (cond ? unauditedString() : kUserConst); // expected-error {{requires a bridged cast}} expected-note {{use __bridge to}} expected-note {{use CFBridgingRelease call to}} 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}} @@ -68,11 +73,13 @@ void test1(int cond) { x = (id) (cond ? [object makeString] : (void*) 0); x = (id) (cond ? (void*) 0 : [object makeString]); x = (id) (cond ? (CFStringRef) @"help" : [object makeString]); + x = (id) (cond ? kUserConst : [object makeString]); x = (id) [object newString]; x = (id) (cond ? [object newString] : (void*) 0); x = (id) (cond ? (void*) 0 : [object newString]); x = (id) (cond ? (CFStringRef) @"help" : [object newString]); // a bit questionable + x = (id) (cond ? kUserConst : [object newString]); // expected-error{{requires a bridged cast}} expected-note{{use __bridge to}} expected-note{{use CFBridgingRelease call to}} } // rdar://problem/10246264 -- 2.7.4