On Nov 21, 2013, at 1:16 PM, Aaron Ballman <[email protected]> wrote:
> On Thu, Nov 21, 2013 at 3:50 PM, Fariborz Jahanian <[email protected]> > 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}} >> + >> +@interface NSDictionary >> +@end >> + >> +@interface NSMutableDictionary : NSDictionary >> +@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
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
