r196314 - [objc] Introduce attribute 'objc_designated_initializer'.

Argyrios Kyrtzidis akyrtzi at gmail.com
Thu Dec 5 12:35:47 PST 2013


Thanks for reviewing! Comments inline.

On Dec 3, 2013, at 1:38 PM, Aaron Ballman <aaron at aaronballman.com> wrote:

> On Tue, Dec 3, 2013 at 4:11 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
>> Author: akirtzidis
>> Date: Tue Dec  3 15:11:25 2013
>> New Revision: 196314
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=196314&view=rev
>> Log:
>> [objc] Introduce attribute 'objc_designated_initializer'.
>> 
>> It only applies to methods of init family in an interface declaration.
>> 
>> Added:
>>    cfe/trunk/test/SemaObjC/attr-designated-init.m
>> Modified:
>>    cfe/trunk/include/clang/Basic/Attr.td
>>    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>>    cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>> 
>> Modified: cfe/trunk/include/clang/Basic/Attr.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=196314&r1=196313&r2=196314&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/Attr.td (original)
>> +++ cfe/trunk/include/clang/Basic/Attr.td Tue Dec  3 15:11:25 2013
>> @@ -684,6 +684,11 @@ def ObjCSuppressProtocol : InheritableAt
>>   let Args = [IdentifierArgument<"Protocol">];
>> }
>> 
>> +def ObjCDesignatedInitializer : Attr {
>> +  let Spellings = [GNU<"objc_designated_initializer">];
>> +  let Subjects = SubjectList<[ObjCMethod], ErrorDiag>;
> 
> Considering how simple the semantic checks are, I think a
> SubsetSubject is more appropriate.  Something like:
> 
> def ObjCInitFamily : SubsetSubject<ObjCMethod, [{S->getMethodFamily()
> == OMF_init}]>;
> 
> def ObjCInterfaceDeclContext : SubsetSubject<ObjCMethod,
> [{isa<ObjCInterfaceDecl>(S->getDeclContext())}]>;
> 
>> +}
>> +
>> def Overloadable : Attr {
>>   let Spellings = [GNU<"overloadable">];
>>   let Subjects = SubjectList<[Function], ErrorDiag>;
>> 
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=196314&r1=196313&r2=196314&view=diff
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Dec  3 15:11:25 2013
>> @@ -2427,6 +2427,12 @@ def warn_objc_requires_super_protocol :
>> def note_protocol_decl : Note<
>>   "protocol is declared here">;
>> 
>> +// objc_designated_initializer attribute diagnostics.
>> +def err_attr_objc_designated_not_init_family : Error<
>> +  "'objc_designated_initializer' only applies to methods of the init family">;
>> +def err_attr_objc_designated_not_interface : Error<
>> +  "'objc_designated_initializer' only applies to methods of interface declarations">;
> 
> Both of these should be attached to the existing diagnostics applying
> to attribute subjects (warn_attribute_wrong_decl_type and
> err_attribute_wrong_decl_type). When switching over to the subset
> subjects, you need to supply a custom error diagnostic argument
> anyway. Don't forget to update AttributeList.h with the new enumerant.

Is it worth adding all that stuff to the tablegen setup just for one attribute ?
I can see it more justified if there's at least another attribute that is going to use it.

> 
>> +
>> def err_ns_bridged_not_interface : Error<
>>   "parameter of 'ns_bridged' attribute does not name an Objective-C class">;
>> 
>> 
>> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=196314&r1=196313&r2=196314&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue Dec  3 15:11:25 2013
>> @@ -3713,6 +3713,28 @@ static void handleObjCBridgeMutableAttr(
>>                             Attr.getAttributeSpellingListIndex()));
>> }
>> 
>> +static void handleObjCDesignatedInitializer(Sema &S, Decl *D,
>> +                                            const AttributeList &Attr) {
>> +  SourceLocation Loc = Attr.getLoc();
>> +  ObjCMethodDecl *Method = cast<ObjCMethodDecl>(D);
>> +
>> +  if (Method->getMethodFamily() != OMF_init) {
>> +    S.Diag(D->getLocStart(), diag::err_attr_objc_designated_not_init_family)
>> +    << SourceRange(Loc, Loc);
>> +    return;
>> +  }
>> +  DeclContext *DC = Method->getDeclContext();
>> +  if (!isa<ObjCInterfaceDecl>(DC)) {
>> +    S.Diag(D->getLocStart(), diag::err_attr_objc_designated_not_interface)
>> +    << SourceRange(Loc, Loc);
>> +    return;
>> +  }
>> +
>> +  Method->addAttr(::new (S.Context)
>> +                  ObjCDesignatedInitializerAttr(Attr.getRange(), S.Context,
>> +                                         Attr.getAttributeSpellingListIndex()));
> 
> Once you use the SubsetSubjects, this entire method can go away.

I later added

	  IFace->setHasDesignatedInitializers();

to this method, not sure if it can still go away now.

-Argyrios

> 
>> +}
>> +
>> static void handleObjCOwnershipAttr(Sema &S, Decl *D,
>>                                     const AttributeList &Attr) {
>>   if (hasDeclarator(D)) return;
>> @@ -3963,6 +3985,9 @@ static void ProcessDeclAttribute(Sema &S
>>   case AttributeList::AT_ObjCBridgeMutable:
>>     handleObjCBridgeMutableAttr(S, scope, D, Attr); break;
>> 
>> +  case AttributeList::AT_ObjCDesignatedInitializer:
>> +    handleObjCDesignatedInitializer(S, D, Attr); break;
> 
> Once the handler method goes away, this can be replaced with:
> 
> handleSimpleAttribute<ObjCDesignatedInitializerAttr>(S, D, Attr); break;
> 
>> +
>>   case AttributeList::AT_CFAuditedTransfer:
>>     handleCFAuditedTransferAttr(S, D, Attr); break;
>>   case AttributeList::AT_CFUnknownTransfer:
>> 
>> Added: cfe/trunk/test/SemaObjC/attr-designated-init.m
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/attr-designated-init.m?rev=196314&view=auto
>> ==============================================================================
>> --- cfe/trunk/test/SemaObjC/attr-designated-init.m (added)
>> +++ cfe/trunk/test/SemaObjC/attr-designated-init.m Tue Dec  3 15:11:25 2013
>> @@ -0,0 +1,32 @@
>> +// RUN: %clang_cc1 -fsyntax-only -verify %s
>> +
>> +#define NS_DESIGNATED_INITIALIZER __attribute__((objc_designated_initializer))
>> +
>> +void fnfoo(void) NS_DESIGNATED_INITIALIZER; // expected-error {{only applies to methods}}
>> +
>> + at protocol P1
>> +-(id)init NS_DESIGNATED_INITIALIZER; // expected-error {{only applies to methods of interface declarations}}
>> + at end
>> +
>> +__attribute__((objc_root_class))
>> + at interface I1
>> +-(void)meth NS_DESIGNATED_INITIALIZER; // expected-error {{only applies to methods of the init family}}
>> +-(id)init NS_DESIGNATED_INITIALIZER;
>> ++(id)init NS_DESIGNATED_INITIALIZER; // expected-error {{only applies to methods of the init family}}
>> + at end
>> +
>> + at interface I1(cat)
>> +-(id)init2 NS_DESIGNATED_INITIALIZER; // expected-error {{only applies to methods of interface declarations}}
>> + at end
>> +
>> + at interface I1()
>> +-(id)init3 NS_DESIGNATED_INITIALIZER; // expected-error {{only applies to methods of interface declarations}}
>> + at end
>> +
>> + at implementation I1
>> +-(void)meth {}
>> +-(id)init NS_DESIGNATED_INITIALIZER { return 0; } // expected-error {{only applies to methods of interface declarations}}
>> ++(id)init { return 0; }
>> +-(id)init3 { return 0; }
>> +-(id)init4 NS_DESIGNATED_INITIALIZER { return 0; } // expected-error {{only applies to methods of interface declarations}}
>> + at end
> 
> ~Aaron

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131205/78ec1e2f/attachment.html>


More information about the cfe-commits mailing list