<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Thanks for reviewing! Comments inline.<div><br><div><div><div>On Dec 3, 2013, at 1:38 PM, Aaron Ballman <<a href="mailto:aaron@aaronballman.com">aaron@aaronballman.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">On Tue, Dec 3, 2013 at 4:11 PM, Argyrios Kyrtzidis <<a href="mailto:akyrtzi@gmail.com">akyrtzi@gmail.com</a>> wrote:<br><blockquote type="cite">Author: akirtzidis<br>Date: Tue Dec 3 15:11:25 2013<br>New Revision: 196314<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=196314&view=rev">http://llvm.org/viewvc/llvm-project?rev=196314&view=rev</a><br>Log:<br>[objc] Introduce attribute 'objc_designated_initializer'.<br><br>It only applies to methods of init family in an interface declaration.<br><br>Added:<br> cfe/trunk/test/SemaObjC/attr-designated-init.m<br>Modified:<br> cfe/trunk/include/clang/Basic/Attr.td<br> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br> cfe/trunk/lib/Sema/SemaDeclAttr.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=196314&r1=196313&r2=196314&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=196314&r1=196313&r2=196314&view=diff</a><br>==============================================================================<br>--- cfe/trunk/include/clang/Basic/Attr.td (original)<br>+++ cfe/trunk/include/clang/Basic/Attr.td Tue Dec 3 15:11:25 2013<br>@@ -684,6 +684,11 @@ def ObjCSuppressProtocol : InheritableAt<br> let Args = [IdentifierArgument<"Protocol">];<br>}<br><br>+def ObjCDesignatedInitializer : Attr {<br>+ let Spellings = [GNU<"objc_designated_initializer">];<br>+ let Subjects = SubjectList<[ObjCMethod], ErrorDiag>;<br></blockquote><br>Considering how simple the semantic checks are, I think a<br>SubsetSubject is more appropriate. Something like:<br><br>def ObjCInitFamily : SubsetSubject<ObjCMethod, [{S->getMethodFamily()<br>== OMF_init}]>;<br><br>def ObjCInterfaceDeclContext : SubsetSubject<ObjCMethod,<br>[{isa<ObjCInterfaceDecl>(S->getDeclContext())}]>;<br><br><blockquote type="cite">+}<br>+<br>def Overloadable : Attr {<br> let Spellings = [GNU<"overloadable">];<br> let Subjects = SubjectList<[Function], ErrorDiag>;<br><br>Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=196314&r1=196313&r2=196314&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=196314&r1=196313&r2=196314&view=diff</a><br>==============================================================================<br>--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)<br>+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Dec 3 15:11:25 2013<br>@@ -2427,6 +2427,12 @@ def warn_objc_requires_super_protocol :<br>def note_protocol_decl : Note<<br> "protocol is declared here">;<br><br>+// objc_designated_initializer attribute diagnostics.<br>+def err_attr_objc_designated_not_init_family : Error<<br>+ "'objc_designated_initializer' only applies to methods of the init family">;<br>+def err_attr_objc_designated_not_interface : Error<<br>+ "'objc_designated_initializer' only applies to methods of interface declarations">;<br></blockquote><br>Both of these should be attached to the existing diagnostics applying<br>to attribute subjects (warn_attribute_wrong_decl_type and<br>err_attribute_wrong_decl_type). When switching over to the subset<br>subjects, you need to supply a custom error diagnostic argument<br>anyway. Don't forget to update AttributeList.h with the new enumerant.<br></div></blockquote><div><br></div><div>Is it worth adding all that stuff to the tablegen setup just for one attribute ?</div><div>I can see it more justified if there's at least another attribute that is going to use it.</div><br><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br><blockquote type="cite">+<br>def err_ns_bridged_not_interface : Error<<br> "parameter of 'ns_bridged' attribute does not name an Objective-C class">;<br><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=196314&r1=196313&r2=196314&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=196314&r1=196313&r2=196314&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)<br>+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue Dec 3 15:11:25 2013<br>@@ -3713,6 +3713,28 @@ static void handleObjCBridgeMutableAttr(<br> Attr.getAttributeSpellingListIndex()));<br>}<br><br>+static void handleObjCDesignatedInitializer(Sema &S, Decl *D,<br>+ const AttributeList &Attr) {<br>+ SourceLocation Loc = Attr.getLoc();<br>+ ObjCMethodDecl *Method = cast<ObjCMethodDecl>(D);<br>+<br>+ if (Method->getMethodFamily() != OMF_init) {<br>+ S.Diag(D->getLocStart(), diag::err_attr_objc_designated_not_init_family)<br>+ << SourceRange(Loc, Loc);<br>+ return;<br>+ }<br>+ DeclContext *DC = Method->getDeclContext();<br>+ if (!isa<ObjCInterfaceDecl>(DC)) {<br>+ S.Diag(D->getLocStart(), diag::err_attr_objc_designated_not_interface)<br>+ << SourceRange(Loc, Loc);<br>+ return;<br>+ }<br>+<br>+ Method->addAttr(::new (S.Context)<br>+ ObjCDesignatedInitializerAttr(Attr.getRange(), S.Context,<br>+ Attr.getAttributeSpellingListIndex()));<br></blockquote><br>Once you use the SubsetSubjects, this entire method can go away.<br></div></blockquote><div><br></div><div>I later added</div><div><br></div><div><span class="Apple-tab-span" style="white-space:pre"> </span> IFace->setHasDesignatedInitializers();</div><br>to this method, not sure if it can still go away now.</div><div><br></div><div>-Argyrios</div><div><br><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br><blockquote type="cite">+}<br>+<br>static void handleObjCOwnershipAttr(Sema &S, Decl *D,<br> const AttributeList &Attr) {<br> if (hasDeclarator(D)) return;<br>@@ -3963,6 +3985,9 @@ static void ProcessDeclAttribute(Sema &S<br> case AttributeList::AT_ObjCBridgeMutable:<br> handleObjCBridgeMutableAttr(S, scope, D, Attr); break;<br><br>+ case AttributeList::AT_ObjCDesignatedInitializer:<br>+ handleObjCDesignatedInitializer(S, D, Attr); break;<br></blockquote><br>Once the handler method goes away, this can be replaced with:<br><br>handleSimpleAttribute<ObjCDesignatedInitializerAttr>(S, D, Attr); break;<br><br><blockquote type="cite">+<br> case AttributeList::AT_CFAuditedTransfer:<br> handleCFAuditedTransferAttr(S, D, Attr); break;<br> case AttributeList::AT_CFUnknownTransfer:<br><br>Added: cfe/trunk/test/SemaObjC/attr-designated-init.m<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/attr-designated-init.m?rev=196314&view=auto">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/attr-designated-init.m?rev=196314&view=auto</a><br>==============================================================================<br>--- cfe/trunk/test/SemaObjC/attr-designated-init.m (added)<br>+++ cfe/trunk/test/SemaObjC/attr-designated-init.m Tue Dec 3 15:11:25 2013<br>@@ -0,0 +1,32 @@<br>+// RUN: %clang_cc1 -fsyntax-only -verify %s<br>+<br>+#define NS_DESIGNATED_INITIALIZER __attribute__((objc_designated_initializer))<br>+<br>+void fnfoo(void) NS_DESIGNATED_INITIALIZER; // expected-error {{only applies to methods}}<br>+<br>+@protocol P1<br>+-(id)init NS_DESIGNATED_INITIALIZER; // expected-error {{only applies to methods of interface declarations}}<br>+@end<br>+<br>+__attribute__((objc_root_class))<br>+@interface I1<br>+-(void)meth NS_DESIGNATED_INITIALIZER; // expected-error {{only applies to methods of the init family}}<br>+-(id)init NS_DESIGNATED_INITIALIZER;<br>++(id)init NS_DESIGNATED_INITIALIZER; // expected-error {{only applies to methods of the init family}}<br>+@end<br>+<br>+@interface I1(cat)<br>+-(id)init2 NS_DESIGNATED_INITIALIZER; // expected-error {{only applies to methods of interface declarations}}<br>+@end<br>+<br>+@interface I1()<br>+-(id)init3 NS_DESIGNATED_INITIALIZER; // expected-error {{only applies to methods of interface declarations}}<br>+@end<br>+<br>+@implementation I1<br>+-(void)meth {}<br>+-(id)init NS_DESIGNATED_INITIALIZER { return 0; } // expected-error {{only applies to methods of interface declarations}}<br>++(id)init { return 0; }<br>+-(id)init3 { return 0; }<br>+-(id)init4 NS_DESIGNATED_INITIALIZER { return 0; } // expected-error {{only applies to methods of interface declarations}}<br>+@end<br></blockquote><br>~Aaron</div></blockquote></div><br></div></div></body></html>