<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Nov 21, 2013, at 1:16 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">On Thu, Nov 21, 2013 at 3:50 PM, Fariborz Jahanian <<a href="mailto:fjahanian@apple.com">fjahanian@apple.com</a>> wrote:<br><blockquote type="cite">Author: fjahanian<br>Date: Thu Nov 21 14:50:32 2013<br>New Revision: 195376<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=195376&view=rev">http://llvm.org/viewvc/llvm-project?rev=195376&view=rev</a><br>Log:<br>ObjectiveC. Implement attribute 'objc_bridge_mutable'<br>whose semantic is currently identical to objc_bridge,<br>but their differences may manifest down the road with<br>further enhancements. // <a href="rdar://15498044">rdar://15498044</a><br><br>Added:<br> cfe/trunk/test/SemaObjC/objcbridgemutable-attribute.m<br>Modified:<br> cfe/trunk/include/clang/Basic/Attr.td<br> cfe/trunk/lib/Sema/SemaDeclAttr.cpp<br> cfe/trunk/lib/Sema/SemaExprObjC.cpp<br><br>Modified: cfe/trunk/include/clang/Basic/Attr.td<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=195376&r1=195375&r2=195376&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=195376&r1=195375&r2=195376&view=diff</a><br>==============================================================================<br>--- cfe/trunk/include/clang/Basic/Attr.td (original)<br>+++ cfe/trunk/include/clang/Basic/Attr.td Thu Nov 21 14:50:32 2013<br>@@ -551,6 +551,12 @@ def ObjCBridge : InheritableAttr {<br> let Args = [IdentifierArgument<"BridgedType", 1>];<br> }<br><br>+def ObjCBridgeMutable : InheritableAttr {<br>+ let Spellings = [GNU<"objc_bridge_mutable">];<br>+ let Subjects = [Record];<br>+ let Args = [IdentifierArgument<"BridgedType", 1>];<br></blockquote><br>This doesn't appear to actually be optional, so you should remove the ,1</blockquote><blockquote type="cite"><br><blockquote type="cite">+}<br>+<br> def NSReturnsRetained : InheritableAttr {<br> let Spellings = [GNU<"ns_returns_retained">];<br> let Subjects = [ObjCMethod, Function];<br><br>Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=195376&r1=195375&r2=195376&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=195376&r1=195375&r2=195376&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)<br>+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Thu Nov 21 14:50:32 2013<br>@@ -4362,6 +4362,30 @@ static void handleObjCBridgeAttr(Sema &S<br> Attr.getAttributeSpellingListIndex()));<br> }<br><br>+static void handleObjCBridgeMutableAttr(Sema &S, Scope *Sc, Decl *D,<br>+ const AttributeList &Attr) {<br>+ if (!isa<RecordDecl>(D)) {<br>+ S.Diag(Attr.getLoc(), diag::err_attribute_wrong_decl_type)<br>+ << Attr.getName()<br>+ << (S.getLangOpts().CPlusPlus ? ExpectedStructOrUnionOrClass<br>+ : ExpectedStructOrUnion);<br></blockquote><br>We don't usually check for CPlusPlus when reporting this sort of<br>diagnostic. I think it's an improvement, but I would prefer if we were<br>a bit more consistent in this regard. Maybe when I finally get the<br>penultimate appertainsTo functionality down, I'll rectify...<br><br><blockquote type="cite">+ return;<br>+ }<br>+<br>+ IdentifierLoc *Parm = 0;<br>+ if (Attr.getNumArgs() == 1)<br>+ Parm = Attr.isArgIdent(0) ? Attr.getArgAsIdent(0) : 0;<br></blockquote><br>This can be simplified once you remove the optional bit in Attr.td;<br>the number of args will have already been checked.<br><blockquote type="cite">+<br>+ if (!Parm) {<br>+ S.Diag(D->getLocStart(), diag::err_objc_attr_not_id) << Attr.getName() << 0;<br>+ return;<br>+ }<br></blockquote><br>Shouldn't you be checking whether the parameter is a class or protocol here?<br></blockquote><div><br></div>Diagnostic id has an alternative because of the work Ted has been doing for a different</div><div>attribute. Here I want to only mention class. Also, checking on whether the argument is </div><div>a class happens when the typedef’ed object is being used in an expression. And</div><div>more tests have been added. In r<span style="font-family: Menlo; font-size: 11px;">195396.</span></div><div><span style="font-family: Menlo; font-size: 11px;"><br></span></div><div><span style="font-family: Menlo; font-size: 11px;">- Thanks, Fariborz</span></div><div><span style="font-family: Menlo; font-size: 11px;"><br></span></div><div><br><blockquote type="cite"><br><blockquote type="cite">+<br>+ D->addAttr(::new (S.Context)<br>+ ObjCBridgeMutableAttr(Attr.getRange(), S.Context, Parm->Ident,<br>+ Attr.getAttributeSpellingListIndex()));<br>+}<br>+<br> static void handleObjCOwnershipAttr(Sema &S, Decl *D,<br> const AttributeList &Attr) {<br> if (hasDeclarator(D)) return;<br>@@ -4651,6 +4675,9 @@ static void ProcessDeclAttribute(Sema &S<br><br> case AttributeList::AT_ObjCBridge:<br> handleObjCBridgeAttr(S, scope, D, Attr); break;<br>+<br>+ case AttributeList::AT_ObjCBridgeMutable:<br>+ handleObjCBridgeMutableAttr(S, scope, D, Attr); break;<br><br> case AttributeList::AT_CFAuditedTransfer:<br> case AttributeList::AT_CFUnknownTransfer:<br><br>Modified: cfe/trunk/lib/Sema/SemaExprObjC.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=195376&r1=195375&r2=195376&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=195376&r1=195375&r2=195376&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/Sema/SemaExprObjC.cpp (original)<br>+++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Thu Nov 21 14:50:32 2013<br>@@ -3165,24 +3165,26 @@ diagnoseObjCARCConversion(Sema &S, Sourc<br> << castRange << castExpr->getSourceRange();<br> }<br><br>-static inline ObjCBridgeAttr *getObjCBridgeAttr(const TypedefType *TD) {<br>+template <typename T><br>+static inline T *getObjCBridgeAttr(const TypedefType *TD) {<br> TypedefNameDecl *TDNDecl = TD->getDecl();<br> QualType QT = TDNDecl->getUnderlyingType();<br> if (QT->isPointerType()) {<br> QT = QT->getPointeeType();<br> if (const RecordType *RT = QT->getAs<RecordType>())<br> if (RecordDecl *RD = RT->getDecl())<br>- if (RD->hasAttr<ObjCBridgeAttr>())<br>- return RD->getAttr<ObjCBridgeAttr>();<br>+ if (RD->hasAttr<T>())<br>+ return RD->getAttr<T>();<br> }<br> return 0;<br> }<br><br>+template <typename TB><br> static bool CheckObjCBridgeNSCast(Sema &S, QualType castType, Expr *castExpr) {<br> QualType T = castExpr->getType();<br> while (const TypedefType *TD = dyn_cast<TypedefType>(T.getTypePtr())) {<br> TypedefNameDecl *TDNDecl = TD->getDecl();<br>- if (ObjCBridgeAttr *ObjCBAttr = getObjCBridgeAttr(TD)) {<br>+ if (TB *ObjCBAttr = getObjCBridgeAttr<TB>(TD)) {<br> if (IdentifierInfo *Parm = ObjCBAttr->getBridgedType()) {<br> NamedDecl *Target = 0;<br> // Check for an existing type with this name.<br>@@ -3231,11 +3233,12 @@ static bool CheckObjCBridgeNSCast(Sema &<br> return false;<br> }<br><br>+template <typename TB><br> static bool CheckObjCBridgeCFCast(Sema &S, QualType castType, Expr *castExpr) {<br> QualType T = castType;<br> while (const TypedefType *TD = dyn_cast<TypedefType>(T.getTypePtr())) {<br> TypedefNameDecl *TDNDecl = TD->getDecl();<br>- if (ObjCBridgeAttr *ObjCBAttr = getObjCBridgeAttr(TD)) {<br>+ if (TB *ObjCBAttr = getObjCBridgeAttr<TB>(TD)) {<br> if (IdentifierInfo *Parm = ObjCBAttr->getBridgedType()) {<br> NamedDecl *Target = 0;<br> // Check for an existing type with this name.<br>@@ -3289,10 +3292,14 @@ void Sema::CheckTollFreeBridgeCast(QualT<br> // warn in presense of __bridge casting to or from a toll free bridge cast.<br> ARCConversionTypeClass exprACTC = classifyTypeForARCConversion(castExpr->getType());<br> ARCConversionTypeClass castACTC = classifyTypeForARCConversion(castType);<br>- if (castACTC == ACTC_retainable && exprACTC == ACTC_coreFoundation)<br>- (void)CheckObjCBridgeNSCast(*this, castType, castExpr);<br>- else if (castACTC == ACTC_coreFoundation && exprACTC == ACTC_retainable)<br>- (void)CheckObjCBridgeCFCast(*this, castType, castExpr);<br>+ if (castACTC == ACTC_retainable && exprACTC == ACTC_coreFoundation) {<br>+ (void)CheckObjCBridgeNSCast<ObjCBridgeAttr>(*this, castType, castExpr);<br>+ (void)CheckObjCBridgeNSCast<ObjCBridgeMutableAttr>(*this, castType, castExpr);<br>+ }<br>+ else if (castACTC == ACTC_coreFoundation && exprACTC == ACTC_retainable) {<br>+ (void)CheckObjCBridgeCFCast<ObjCBridgeAttr>(*this, castType, castExpr);<br>+ (void)CheckObjCBridgeCFCast<ObjCBridgeMutableAttr>(*this, castType, castExpr);<br>+ }<br> }<br><br> Sema::ARCConversionResult<br>@@ -3355,12 +3362,14 @@ Sema::CheckObjCARCConversion(SourceRange<br><br> if (castACTC == ACTC_retainable && exprACTC == ACTC_coreFoundation &&<br> (CCK == CCK_CStyleCast || CCK == CCK_FunctionalCast))<br>- if (CheckObjCBridgeNSCast(*this, castType, castExpr))<br>+ if (CheckObjCBridgeNSCast<ObjCBridgeAttr>(*this, castType, castExpr) ||<br>+ CheckObjCBridgeNSCast<ObjCBridgeMutableAttr>(*this, castType, castExpr))<br> return ACR_okay;<br><br> if (castACTC == ACTC_coreFoundation && exprACTC == ACTC_retainable &&<br> (CCK == CCK_CStyleCast || CCK == CCK_FunctionalCast))<br>- if (CheckObjCBridgeCFCast(*this, castType, castExpr))<br>+ if (CheckObjCBridgeCFCast<ObjCBridgeAttr>(*this, castType, castExpr) ||<br>+ CheckObjCBridgeCFCast<ObjCBridgeMutableAttr>(*this, castType, castExpr))<br> return ACR_okay;<br><br><br><br>Added: cfe/trunk/test/SemaObjC/objcbridgemutable-attribute.m<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/objcbridgemutable-attribute.m?rev=195376&view=auto">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/objcbridgemutable-attribute.m?rev=195376&view=auto</a><br>==============================================================================<br>--- cfe/trunk/test/SemaObjC/objcbridgemutable-attribute.m (added)<br>+++ cfe/trunk/test/SemaObjC/objcbridgemutable-attribute.m Thu Nov 21 14:50:32 2013<br>@@ -0,0 +1,19 @@<br>+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s<br>+// <a href="rdar://15498044">rdar://15498044</a><br>+<br>+typedef struct __attribute__((objc_bridge_mutable(NSMutableDictionary))) __CFDictionary * CFMutableDictionaryRef; // expected-note {{declared here}}<br>+<br>+@interface NSDictionary<br>+@end<br>+<br>+@interface NSMutableDictionary : NSDictionary<br>+@end<br>+<br>+void Test(NSMutableDictionary *md, NSDictionary *nd, CFMutableDictionaryRef mcf) {<br>+<br>+ (void) (CFMutableDictionaryRef)md;<br>+ (void) (CFMutableDictionaryRef)nd; // expected-warning {{'NSDictionary' cannot bridge to 'CFMutableDictionaryRef' (aka 'struct __CFDictionary *')}}<br>+ (void) (NSDictionary *)mcf; // expected-warning {{'CFMutableDictionaryRef' (aka 'struct __CFDictionary *') bridges to NSMutableDictionary, not 'NSDictionary'}}<br>+ (void) (NSMutableDictionary *)mcf; // ok;<br>+}<br>+<br></blockquote><br>You are missing all of the tests for the attribute semantics. You<br>should have tests for the number of args passed, whether the arg is an<br>ident or expr, and whether it's attached to the proper subject.<br><br>Thanks!<br><br>~Aaron<br></blockquote></div><br></body></html>