This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by ahatanak on Apr 4 2017, 10:16 AM.

Details

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

Diff Detail

Event Timeline

ahatanak created this revision.Apr 4 2017, 10:16 AM
rjmccall added inline comments.Apr 4 2017, 1:18 PM
lib/Sema/SemaExprObjC.cpp
3358

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

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

ahatanak updated this revision to Diff 94460.Apr 6 2017, 4:54 PM
ahatanak marked an inline comment as done.
ahatanak added inline comments.
lib/Sema/SemaExprObjC.cpp
3358

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

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

rjmccall added inline comments.Apr 6 2017, 8:11 PM
lib/Sema/SemaExprObjC.cpp
3358

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 updated this revision to Diff 94686.Apr 10 2017, 10:05 AM
ahatanak marked an inline comment as done.
ahatanak added inline comments.
lib/Sema/SemaExprObjC.cpp
3358

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

rjmccall accepted this revision.Apr 10 2017, 11:46 AM

Thanks, LGTM.

This revision is now accepted and ready to land.Apr 10 2017, 11:46 AM
This revision was automatically updated to reflect the committed changes.