r195376 - ObjectiveC. Implement attribute 'objc_bridge_mutable'

[Aaron Ballman]

On Thu, Nov 21, 2013 at 7:10 PM, jahanian <fjahanian at apple.com> wrote:
>
> 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! The changes look good.

~Aaron