r195376 - ObjectiveC. Implement attribute 'objc_bridge_mutable'

jahanian fjahanian at apple.com
Thu Nov 21 16:10:00 PST 2013


On Nov 21, 2013, at 1:16 PM, Aaron Ballman <aaron at aaronballman.com> wrote:

> On Thu, Nov 21, 2013 at 3:50 PM, Fariborz Jahanian <fjahanian at apple.com> wrote:
>> Author: fjahanian
>> Date: Thu Nov 21 14:50:32 2013
>> New Revision: 195376
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=195376&view=rev
>> Log:
>> ObjectiveC. Implement attribute 'objc_bridge_mutable'
>> whose semantic is currently identical to objc_bridge,
>> but their differences may manifest down the road with
>> further enhancements. // rdar://15498044
>> 
>> Added:
>>    cfe/trunk/test/SemaObjC/objcbridgemutable-attribute.m
>> Modified:
>>    cfe/trunk/include/clang/Basic/Attr.td
>>    cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>>    cfe/trunk/lib/Sema/SemaExprObjC.cpp
>> 
>> Modified: cfe/trunk/include/clang/Basic/Attr.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=195376&r1=195375&r2=195376&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/Attr.td (original)
>> +++ cfe/trunk/include/clang/Basic/Attr.td Thu Nov 21 14:50:32 2013
>> @@ -551,6 +551,12 @@ def ObjCBridge : InheritableAttr {
>>   let Args = [IdentifierArgument<"BridgedType", 1>];
>> }
>> 
>> +def ObjCBridgeMutable : InheritableAttr {
>> +  let Spellings = [GNU<"objc_bridge_mutable">];
>> +  let Subjects = [Record];
>> +  let Args = [IdentifierArgument<"BridgedType", 1>];
> 
> This doesn't appear to actually be optional, so you should remove the ,1
> 
>> +}
>> +
>> def NSReturnsRetained : InheritableAttr {
>>   let Spellings = [GNU<"ns_returns_retained">];
>>   let Subjects = [ObjCMethod, Function];
>> 
>> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=195376&r1=195375&r2=195376&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Thu Nov 21 14:50:32 2013
>> @@ -4362,6 +4362,30 @@ static void handleObjCBridgeAttr(Sema &S
>>                            Attr.getAttributeSpellingListIndex()));
>> }
>> 
>> +static void handleObjCBridgeMutableAttr(Sema &S, Scope *Sc, Decl *D,
>> +                                        const AttributeList &Attr) {
>> +  if (!isa<RecordDecl>(D)) {
>> +    S.Diag(Attr.getLoc(), diag::err_attribute_wrong_decl_type)
>> +    << Attr.getName()
>> +    << (S.getLangOpts().CPlusPlus ? ExpectedStructOrUnionOrClass
>> +        : ExpectedStructOrUnion);
> 
> We don't usually check for CPlusPlus when reporting this sort of
> diagnostic. I think it's an improvement, but I would prefer if we were
> a bit more consistent in this regard. Maybe when I finally get the
> penultimate appertainsTo functionality down, I'll rectify...
> 
>> +    return;
>> +  }
>> +
>> +  IdentifierLoc *Parm = 0;
>> +  if (Attr.getNumArgs() == 1)
>> +    Parm = Attr.isArgIdent(0) ? Attr.getArgAsIdent(0) : 0;
> 
> This can be simplified once you remove the optional bit in Attr.td;
> the number of args will have already been checked.
>> +
>> +  if (!Parm) {
>> +    S.Diag(D->getLocStart(), diag::err_objc_attr_not_id) << Attr.getName() << 0;
>> +    return;
>> +  }
> 
> Shouldn't you be checking whether the parameter is a class or protocol here?

Diagnostic id has an alternative because of the work Ted has been doing for a different
attribute. Here I want to only mention class. Also, checking on whether the argument is 
a class happens when the typedef’ed object is being used in an expression. And
more tests have been added. In r195396.

- Thanks, Fariborz


> 
>> +
>> +  D->addAttr(::new (S.Context)
>> +             ObjCBridgeMutableAttr(Attr.getRange(), S.Context, Parm->Ident,
>> +                            Attr.getAttributeSpellingListIndex()));
>> +}
>> +
>> static void handleObjCOwnershipAttr(Sema &S, Decl *D,
>>                                     const AttributeList &Attr) {
>>   if (hasDeclarator(D)) return;
>> @@ -4651,6 +4675,9 @@ static void ProcessDeclAttribute(Sema &S
>> 
>>   case AttributeList::AT_ObjCBridge:
>>     handleObjCBridgeAttr(S, scope, D, Attr); break;
>> +
>> +  case AttributeList::AT_ObjCBridgeMutable:
>> +    handleObjCBridgeMutableAttr(S, scope, D, Attr); break;
>> 
>>   case AttributeList::AT_CFAuditedTransfer:
>>   case AttributeList::AT_CFUnknownTransfer:
>> 
>> Modified: cfe/trunk/lib/Sema/SemaExprObjC.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=195376&r1=195375&r2=195376&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaExprObjC.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Thu Nov 21 14:50:32 2013
>> @@ -3165,24 +3165,26 @@ diagnoseObjCARCConversion(Sema &S, Sourc
>>     << castRange << castExpr->getSourceRange();
>> }
>> 
>> -static inline ObjCBridgeAttr *getObjCBridgeAttr(const TypedefType *TD) {
>> +template <typename T>
>> +static inline T *getObjCBridgeAttr(const TypedefType *TD) {
>>   TypedefNameDecl *TDNDecl = TD->getDecl();
>>   QualType QT = TDNDecl->getUnderlyingType();
>>   if (QT->isPointerType()) {
>>     QT = QT->getPointeeType();
>>     if (const RecordType *RT = QT->getAs<RecordType>())
>>       if (RecordDecl *RD = RT->getDecl())
>> -        if (RD->hasAttr<ObjCBridgeAttr>())
>> -          return RD->getAttr<ObjCBridgeAttr>();
>> +        if (RD->hasAttr<T>())
>> +          return RD->getAttr<T>();
>>   }
>>   return 0;
>> }
>> 
>> +template <typename TB>
>> static bool CheckObjCBridgeNSCast(Sema &S, QualType castType, Expr *castExpr) {
>>   QualType T = castExpr->getType();
>>   while (const TypedefType *TD = dyn_cast<TypedefType>(T.getTypePtr())) {
>>     TypedefNameDecl *TDNDecl = TD->getDecl();
>> -    if (ObjCBridgeAttr *ObjCBAttr = getObjCBridgeAttr(TD)) {
>> +    if (TB *ObjCBAttr = getObjCBridgeAttr<TB>(TD)) {
>>       if (IdentifierInfo *Parm = ObjCBAttr->getBridgedType()) {
>>         NamedDecl *Target = 0;
>>         // Check for an existing type with this name.
>> @@ -3231,11 +3233,12 @@ static bool CheckObjCBridgeNSCast(Sema &
>>   return false;
>> }
>> 
>> +template <typename TB>
>> static bool CheckObjCBridgeCFCast(Sema &S, QualType castType, Expr *castExpr) {
>>   QualType T = castType;
>>   while (const TypedefType *TD = dyn_cast<TypedefType>(T.getTypePtr())) {
>>     TypedefNameDecl *TDNDecl = TD->getDecl();
>> -    if (ObjCBridgeAttr *ObjCBAttr = getObjCBridgeAttr(TD)) {
>> +    if (TB *ObjCBAttr = getObjCBridgeAttr<TB>(TD)) {
>>       if (IdentifierInfo *Parm = ObjCBAttr->getBridgedType()) {
>>         NamedDecl *Target = 0;
>>         // Check for an existing type with this name.
>> @@ -3289,10 +3292,14 @@ void Sema::CheckTollFreeBridgeCast(QualT
>>   // warn in presense of __bridge casting to or from a toll free bridge cast.
>>   ARCConversionTypeClass exprACTC = classifyTypeForARCConversion(castExpr->getType());
>>   ARCConversionTypeClass castACTC = classifyTypeForARCConversion(castType);
>> -  if (castACTC == ACTC_retainable && exprACTC == ACTC_coreFoundation)
>> -    (void)CheckObjCBridgeNSCast(*this, castType, castExpr);
>> -  else if (castACTC == ACTC_coreFoundation && exprACTC == ACTC_retainable)
>> -    (void)CheckObjCBridgeCFCast(*this, castType, castExpr);
>> +  if (castACTC == ACTC_retainable && exprACTC == ACTC_coreFoundation) {
>> +    (void)CheckObjCBridgeNSCast<ObjCBridgeAttr>(*this, castType, castExpr);
>> +    (void)CheckObjCBridgeNSCast<ObjCBridgeMutableAttr>(*this, castType, castExpr);
>> +  }
>> +  else if (castACTC == ACTC_coreFoundation && exprACTC == ACTC_retainable) {
>> +    (void)CheckObjCBridgeCFCast<ObjCBridgeAttr>(*this, castType, castExpr);
>> +    (void)CheckObjCBridgeCFCast<ObjCBridgeMutableAttr>(*this, castType, castExpr);
>> +  }
>> }
>> 
>> Sema::ARCConversionResult
>> @@ -3355,12 +3362,14 @@ Sema::CheckObjCARCConversion(SourceRange
>> 
>>   if (castACTC == ACTC_retainable && exprACTC == ACTC_coreFoundation &&
>>       (CCK == CCK_CStyleCast || CCK == CCK_FunctionalCast))
>> -    if (CheckObjCBridgeNSCast(*this, castType, castExpr))
>> +    if (CheckObjCBridgeNSCast<ObjCBridgeAttr>(*this, castType, castExpr) ||
>> +        CheckObjCBridgeNSCast<ObjCBridgeMutableAttr>(*this, castType, castExpr))
>>       return ACR_okay;
>> 
>>   if (castACTC == ACTC_coreFoundation && exprACTC == ACTC_retainable &&
>>       (CCK == CCK_CStyleCast || CCK == CCK_FunctionalCast))
>> -    if (CheckObjCBridgeCFCast(*this, castType, castExpr))
>> +    if (CheckObjCBridgeCFCast<ObjCBridgeAttr>(*this, castType, castExpr) ||
>> +        CheckObjCBridgeCFCast<ObjCBridgeMutableAttr>(*this, castType, castExpr))
>>       return ACR_okay;
>> 
>> 
>> 
>> Added: cfe/trunk/test/SemaObjC/objcbridgemutable-attribute.m
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/objcbridgemutable-attribute.m?rev=195376&view=auto
>> ==============================================================================
>> --- cfe/trunk/test/SemaObjC/objcbridgemutable-attribute.m (added)
>> +++ cfe/trunk/test/SemaObjC/objcbridgemutable-attribute.m Thu Nov 21 14:50:32 2013
>> @@ -0,0 +1,19 @@
>> +// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s
>> +// rdar://15498044
>> +
>> +typedef struct __attribute__((objc_bridge_mutable(NSMutableDictionary))) __CFDictionary * CFMutableDictionaryRef; // expected-note {{declared here}}
>> +
>> + at interface NSDictionary
>> + at end
>> +
>> + at interface NSMutableDictionary : NSDictionary
>> + at end
>> +
>> +void Test(NSMutableDictionary *md, NSDictionary *nd, CFMutableDictionaryRef mcf) {
>> +
>> +  (void) (CFMutableDictionaryRef)md;
>> +  (void) (CFMutableDictionaryRef)nd; // expected-warning {{'NSDictionary' cannot bridge to 'CFMutableDictionaryRef' (aka 'struct __CFDictionary *')}}
>> +  (void) (NSDictionary *)mcf;  // expected-warning {{'CFMutableDictionaryRef' (aka 'struct __CFDictionary *') bridges to NSMutableDictionary, not 'NSDictionary'}}
>> +  (void) (NSMutableDictionary *)mcf; // ok;
>> +}
>> +
> 
> You are missing all of the tests for the attribute semantics. You
> should have tests for the number of args passed, whether the arg is an
> ident or expr, and whether it's attached to the proper subject.
> 
> Thanks!
> 
> ~Aaron

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131121/64c7d21e/attachment.html>


More information about the cfe-commits mailing list