Allow casting C pointers declared using extern "C" to ObjC pointer types

[ahatanak]

Summary

ARCCastChecker::VisitDeclRefExpr allows casting a constant pointer declared in a header file to an ObjC pointer type. However, it rejects a cast from a variable declared with a language linkage specification (e.g., extern "C") to an ObjC pointer type.

According to the standard, declarations directly contained in a language linkage specification (single line "extern C") are treated as if they contain the extern specifier for the purpose of determining whether they are definitions, so such variables should be treated as declarations unless they have initializers.

This patch replaces the call to VarDecl::getStorageClass (which returns SC_None for extern "C" variables) with a call to VarDecl::isThisDeclarationADefinition to detect variable declarations more precisely.

rdar://problem/29249853

[rjmccall]

lib/Sema/SemaExprObjC.cpp

3358 ↗

(On Diff #94087)

Hmm. Come to think of it, I wonder if we actually care whether the variable has a definition, given that it's const.

Well, we can consider that later. I agree that this change is good.

test/SemaObjCXX/arc-bridged-cast.mm

59 ↗

(On Diff #94087)

These examples are a little unfortunate because these values are known to be null pointers.

[ahatanak]

lib/Sema/SemaExprObjC.cpp

3358 ↗

(On Diff #94087)

If we don't care whether the variable is a definition, it's possible to check whether the variable declaration is directly contained in a language linkage instead (using a function like isSingleLineLanguageLinkage in lib/AST/Decl.cpp).

test/SemaObjCXX/arc-bridged-cast.mm

59 ↗

(On Diff #94087)

Changed the initializer to be the address of a __CFAnnotatedObject variable.

[rjmccall]

lib/Sema/SemaExprObjC.cpp

3358 ↗

(On Diff #94087)

Nah, let's keep it simple for now.

Well, maybe the rule ought to be "does a definition exist?" rather than "is the specific declaration that lookup found a definition?"

[ahatanak]

lib/Sema/SemaExprObjC.cpp

3358 ↗

(On Diff #94087)

I think you are right. Changed it to a call to hasDefinition.

[rjmccall]

Thanks, LGTM.

cfe/trunk/lib/Sema/SemaExprObjC.cpp

public:
 
     /// Some declaration references are okay.
     ACCResult VisitDeclRefExpr(DeclRefExpr *e) {
       VarDecl *var = dyn_cast<VarDecl>(e->getDecl());
       // References to global constants are okay.
       if (isAnyRetainable(TargetClass) &&
           isAnyRetainable(SourceClass) &&
           var &&
-          var->getStorageClass() == SC_Extern &&
+          !var->hasDefinition(Context) &&
           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;

cfe/trunk/test/SemaObjCXX/arc-bridged-cast.mm

void testObjCBridgeId() {
   obj = (__bridge_transfer NSString*)CFGetAnnotated();
 
   CFAnnotatedObjectRef ref;
   ref = (__bridge CFAnnotatedObjectRef) CreateSomething();
   ref = (__bridge CFAnnotatedObjectRef) CreateNSString();
   ref = (__bridge_retained CFAnnotatedObjectRef) CreateSomething();
   ref = (__bridge_retained CFAnnotatedObjectRef) CreateNSString();
 }
+
+struct __CFAnnotatedObject {
+} cf0;
+
+extern const CFAnnotatedObjectRef r0;
+extern const CFAnnotatedObjectRef r1 = &cf0;
+extern "C" const CFAnnotatedObjectRef r2;
+extern "C" const CFAnnotatedObjectRef r3 = &cf0;
+
+void testExternC() {
+  id obj;
+  obj = (id)r0;
+  obj = (id)r1; // expected-error{{cast of C pointer type 'CFAnnotatedObjectRef' (aka 'const __CFAnnotatedObject *') to Objective-C pointer type 'id' requires a bridged cast}} expected-note{{use __bridge to convert directly}} expected-note{{use __bridge_transfer to transfer ownership of a +1 'CFAnnotatedObjectRef'}}
+  obj = (id)r2;
+  obj = (id)r3; // expected-error{{cast of C pointer type 'CFAnnotatedObjectRef' (aka 'const __CFAnnotatedObject *') to Objective-C pointer type 'id' requires a bridged cast}} expected-note{{use __bridge to convert directly}} expected-note{{use __bridge_transfer to transfer ownership of a +1 'CFAnnotatedObjectRef'}}
+}