[cfe-commits] r161187 - in /cfe/trunk: lib/Sema/SemaPseudoObject.cpp test/SemaObjC/arc-dict-bridged-cast.m

[Jordan Rose]

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 <fjahanian at apple.com> 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;
> +
> + at protocol NSCopying @end
> +
> + at interface NSDictionary
> +- (id)objectForKeyedSubscript:(id<NSCopying>)key;
> + at end
> +
> +extern
> +CFMutableStringRef CFStringCreateMutable(CFAllocatorRef alloc, CFIndex maxLength);
> +
> +typedef const void * CFTypeRef;
> +
> +id CFBridgingRelease(CFTypeRef __attribute__((cf_consumed)) X);
> +
> + at 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits