Thanks for reviewing! Comments inline. On Dec 3, 2013, at 1:38 PM, Aaron Ballman <[email protected]> wrote:
> On Tue, Dec 3, 2013 at 4:11 PM, Argyrios Kyrtzidis <[email protected]> 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}} >> + >> +@protocol P1 >> +-(id)init NS_DESIGNATED_INITIALIZER; // expected-error {{only applies to >> methods of interface declarations}} >> +@end >> + >> +__attribute__((objc_root_class)) >> +@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}} >> +@end >> + >> +@interface I1(cat) >> +-(id)init2 NS_DESIGNATED_INITIALIZER; // expected-error {{only applies to >> methods of interface declarations}} >> +@end >> + >> +@interface I1() >> +-(id)init3 NS_DESIGNATED_INITIALIZER; // expected-error {{only applies to >> methods of interface declarations}} >> +@end >> + >> +@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}} >> +@end > > ~Aaron
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
