That is a really ugly fixit: (__bridge __strong id <NSCopying>). At the very least we should drop the __strong, but I could see an argument for just offering (__bridge id). That's all the type info we get with CFBridgingRelease, and it's not like we need more info to type-check here (since they're already casting).
Stepping back, this is another case where we know the ownership semantics and should only offer one of the fixits. I assume this commit will already do the right thing if CFStringCreateMutable were annotated with cf_audited_transfer / arc_cf_code_audited, but maybe we should explicitly add that to the test? Jordan On Aug 2, 2012, at 11:03 , Fariborz Jahanian <[email protected]> wrote: > Author: fjahanian > Date: Thu Aug 2 13:03:58 2012 > New Revision: 161187 > > URL: http://llvm.org/viewvc/llvm-project?rev=161187&view=rev > Log: > objective-c arc: Patch to suggest bridge casting of CF > objects used as dictionary subscript objects. > // rdar://11913153 > > Added: > cfe/trunk/test/SemaObjC/arc-dict-bridged-cast.m > Modified: > cfe/trunk/lib/Sema/SemaPseudoObject.cpp > > Modified: cfe/trunk/lib/Sema/SemaPseudoObject.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaPseudoObject.cpp?rev=161187&r1=161186&r2=161187&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaPseudoObject.cpp (original) > +++ cfe/trunk/lib/Sema/SemaPseudoObject.cpp Thu Aug 2 13:03:58 2012 > @@ -955,6 +955,27 @@ > return OS_Error; > } > > +/// CheckKeyForObjCARCConversion - This routine suggests bridge casting of CF > +/// objects used as dictionary subscript key objects. > +static void CheckKeyForObjCARCConversion(Sema &S, QualType ContainerT, > + Expr *Key) { > + if (ContainerT.isNull()) > + return; > + // dictionary subscripting. > + // - (id)objectForKeyedSubscript:(id)key; > + IdentifierInfo *KeyIdents[] = { > + &S.Context.Idents.get("objectForKeyedSubscript") > + }; > + Selector GetterSelector = S.Context.Selectors.getSelector(1, KeyIdents); > + ObjCMethodDecl *Getter = S.LookupMethodInObjectType(GetterSelector, > ContainerT, > + true /*instance*/); > + if (!Getter) > + return; > + QualType T = Getter->param_begin()[0]->getType(); > + S.CheckObjCARCConversion(Key->getSourceRange(), > + T, Key, Sema::CCK_ImplicitConversion); > +} > + > bool ObjCSubscriptOpBuilder::findAtIndexGetter() { > if (AtIndexGetter) > return true; > @@ -972,8 +993,12 @@ > } > Sema::ObjCSubscriptKind Res = > S.CheckSubscriptingKind(RefExpr->getKeyExpr()); > - if (Res == Sema::OS_Error) > + if (Res == Sema::OS_Error) { > + if (S.getLangOpts().ObjCAutoRefCount) > + CheckKeyForObjCARCConversion(S, ResultType, > + RefExpr->getKeyExpr()); > return false; > + } > bool arrayRef = (Res == Sema::OS_Array); > > if (ResultType.isNull()) { > @@ -1080,8 +1105,12 @@ > > Sema::ObjCSubscriptKind Res = > S.CheckSubscriptingKind(RefExpr->getKeyExpr()); > - if (Res == Sema::OS_Error) > + if (Res == Sema::OS_Error) { > + if (S.getLangOpts().ObjCAutoRefCount) > + CheckKeyForObjCARCConversion(S, ResultType, > + RefExpr->getKeyExpr()); > return false; > + } > bool arrayRef = (Res == Sema::OS_Array); > > if (ResultType.isNull()) { > > Added: cfe/trunk/test/SemaObjC/arc-dict-bridged-cast.m > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-dict-bridged-cast.m?rev=161187&view=auto > ============================================================================== > --- cfe/trunk/test/SemaObjC/arc-dict-bridged-cast.m (added) > +++ cfe/trunk/test/SemaObjC/arc-dict-bridged-cast.m Thu Aug 2 13:03:58 2012 > @@ -0,0 +1,47 @@ > +// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fsyntax-only -fobjc-arc > -verify %s > +// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fsyntax-only -fobjc-arc > -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s > +// rdar://11913153 > + > +typedef const struct __CFString * CFStringRef; > +typedef struct __CFString * CFMutableStringRef; > +typedef signed long CFIndex; > +typedef const struct __CFAllocator * CFAllocatorRef; > + > +extern const CFStringRef kCFBundleNameKey; > + > +@protocol NSCopying @end > + > +@interface NSDictionary > +- (id)objectForKeyedSubscript:(id<NSCopying>)key; > +@end > + > +extern > +CFMutableStringRef CFStringCreateMutable(CFAllocatorRef alloc, CFIndex > maxLength); > + > +typedef const void * CFTypeRef; > + > +id CFBridgingRelease(CFTypeRef __attribute__((cf_consumed)) X); > + > +@interface NSMutableString @end > + > +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<NSCopying>' 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}} > + 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<NSCopying>' 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 'CFMutableStringRef' > (aka 'struct __CFString *') into ARC}} > + > +} > + > +// CHECK: fix-it:"{{.*}}":{29:18-29:18}:"(__bridge __strong id<NSCopying>)(" > +// CHECK: fix-it:"{{.*}}":{29:34-29:34}:")" > +// CHECK: fix-it:"{{.*}}":{29:18-29:18}:"CFBridgingRelease(" > +// CHECK: fix-it:"{{.*}}":{29:34-29:34}:")" > +// CHECK: fix-it:"{{.*}}":{33:25-33:25}:"(__bridge __strong id<NSCopying>)(" > +// CHECK: fix-it:"{{.*}}":{33:63-33:63}:")" > +// CHECK: fix-it:"{{.*}}":{33:25-33:25}:"CFBridgingRelease(" > +// CHECK: fix-it:"{{.*}}":{33:63-33:63}:")" > > > _______________________________________________ > 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
