r195376 - ObjectiveC. Implement attribute 'objc_bridge_mutable'

Aaron Ballman aaron at aaronballman.com
Thu Nov 21 13:16:55 PST 2013


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?

> +
> +  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



More information about the cfe-commits mailing list