Thanks everyone, merged in r354015.
On Wed, Feb 13, 2019 at 9:48 PM Aaron Ballman <aa...@aaronballman.com> wrote: > > I'm fine with rolling it into 8.0. > > ~Aaron > > On Wed, Feb 13, 2019 at 3:47 PM Erik Pilkington via cfe-commits > <cfe-commits@lists.llvm.org> wrote: > > > > It isn’t a super common issue, but this is a pretty low-risk fix so I think > > it would be nice to have in 8.0. > > > > On Feb 13, 2019, at 12:40 PM, Shoaib Meenai <smee...@fb.com> wrote: > > > > Should this be considered for 8.0? It's late in the branch and I don't know > > how prevalent the issue being fixed is, but it caught my eye. > > > > From: cfe-commits <cfe-commits-boun...@lists.llvm.org> on behalf of Erik > > Pilkington via cfe-commits <cfe-commits@lists.llvm.org> > > Reply-To: Erik Pilkington <erik.pilking...@gmail.com> > > Date: Wednesday, February 13, 2019 at 12:32 PM > > To: "cfe-commits@lists.llvm.org" <cfe-commits@lists.llvm.org> > > Subject: r353976 - [Sema] Delay checking whether > > objc_designated_initializer is being applied to an init method > > > > Author: epilk > > Date: Wed Feb 13 12:32:37 2019 > > New Revision: 353976 > > > > URL: > > https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D353976-26view-3Drev&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=SS1gIdRrXBd-rCKs5rBEEPmEevgsHAkkERh1ko2vPEc&s=W245MwgFUCCNz9Ug2Tc0dY5lJ35n855YKxo_fwmicxM&e= > > Log: > > [Sema] Delay checking whether objc_designated_initializer is being applied > > to an init method > > > > This fixes a regression that was caused by r335084, which reversed > > the order that attributes are applied. objc_method_family can change > > whether a method is an init method, so the order that these > > attributes are applied matters. The commit fixes this by delaying the > > init check until after all attributes have been applied. > > > > rdar://47829358 > > > > Differential revision: > > https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D58152&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=SS1gIdRrXBd-rCKs5rBEEPmEevgsHAkkERh1ko2vPEc&s=-4Nd4QBW0KxmFdmqS-2w90_5AoHoRQbdwd8K3Z4JzuY&e= > > > > Modified: > > cfe/trunk/include/clang/Basic/Attr.td > > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > > cfe/trunk/lib/Sema/SemaDeclAttr.cpp > > cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test > > cfe/trunk/test/SemaObjC/attr-designated-init.m > > > > Modified: cfe/trunk/include/clang/Basic/Attr.td > > URL: > > https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_include_clang_Basic_Attr.td-3Frev-3D353976-26r1-3D353975-26r2-3D353976-26view-3Ddiff&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=SS1gIdRrXBd-rCKs5rBEEPmEevgsHAkkERh1ko2vPEc&s=tgqq2EylVgj2FgGry6ZO46KA6teeO-xJ1TlvJWvEy6Q&e= > > ============================================================================== > > --- cfe/trunk/include/clang/Basic/Attr.td (original) > > +++ cfe/trunk/include/clang/Basic/Attr.td Wed Feb 13 12:32:37 2019 > > @@ -102,13 +102,6 @@ def ObjCInstanceMethod : SubsetSubject<O > > [{S->isInstanceMethod()}], > > "Objective-C instance methods">; > > -def ObjCInterfaceDeclInitMethod : SubsetSubject<ObjCMethod, > > - [{S->getMethodFamily() == OMF_init && > > - > > (isa<ObjCInterfaceDecl>(S->getDeclContext()) || > > - > > (isa<ObjCCategoryDecl>(S->getDeclContext()) && > > - > > cast<ObjCCategoryDecl>(S->getDeclContext())->IsClassExtension()))}], > > - "init methods of interface or class extension declarations">; > > - > > def Struct : SubsetSubject<Record, > > [{!S->isUnion()}], "structs">; > > @@ -1786,7 +1779,7 @@ def ObjCExplicitProtocolImpl : Inheritab > > def ObjCDesignatedInitializer : Attr { > > let Spellings = [Clang<"objc_designated_initializer">]; > > - let Subjects = SubjectList<[ObjCInterfaceDeclInitMethod], ErrorDiag>; > > + let Subjects = SubjectList<[ObjCMethod], ErrorDiag>; > > let Documentation = [Undocumented]; > > } > > > > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td > > URL: > > https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_include_clang_Basic_DiagnosticSemaKinds.td-3Frev-3D353976-26r1-3D353975-26r2-3D353976-26view-3Ddiff&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=SS1gIdRrXBd-rCKs5rBEEPmEevgsHAkkERh1ko2vPEc&s=d6MOBmrgs96qcCuR_Q5j82acKFzDnHxqDkcDiUM_IIk&e= > > ============================================================================== > > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) > > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Feb 13 > > 12:32:37 2019 > > @@ -3481,6 +3481,9 @@ def warn_objc_secondary_init_missing_ini > > def warn_objc_implementation_missing_designated_init_override : Warning< > > "method override for the designated initializer of the superclass > > %objcinstance0 not found">, > > InGroup<ObjCDesignatedInit>; > > +def err_designated_init_attr_non_init : Error< > > + "'objc_designated_initializer' attribute only applies to init methods " > > + "of interface or class extension declarations">; > > // objc_bridge attribute diagnostics. > > def err_objc_attr_not_id : Error< > > > > Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp > > URL: > > https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_lib_Sema_SemaDeclAttr.cpp-3Frev-3D353976-26r1-3D353975-26r2-3D353976-26view-3Ddiff&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=SS1gIdRrXBd-rCKs5rBEEPmEevgsHAkkERh1ko2vPEc&s=n3Lrsayy7oQTOdjbionMiki15N4Z4kxZaPSw5Fn4uww&e= > > ============================================================================== > > --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original) > > +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed Feb 13 12:32:37 2019 > > @@ -5249,11 +5249,22 @@ static void handleObjCBridgeRelatedAttr( > > static void handleObjCDesignatedInitializer(Sema &S, Decl *D, > > const ParsedAttr &AL) { > > + DeclContext *Ctx = D->getDeclContext(); > > + > > + // This attribute can only be applied to methods in interfaces or class > > + // extensions. > > + if (!isa<ObjCInterfaceDecl>(Ctx) && > > + !(isa<ObjCCategoryDecl>(Ctx) && > > + cast<ObjCCategoryDecl>(Ctx)->IsClassExtension())) { > > + S.Diag(D->getLocation(), diag::err_designated_init_attr_non_init); > > + return; > > + } > > + > > ObjCInterfaceDecl *IFace; > > - if (auto *CatDecl = dyn_cast<ObjCCategoryDecl>(D->getDeclContext())) > > + if (auto *CatDecl = dyn_cast<ObjCCategoryDecl>(Ctx)) > > IFace = CatDecl->getClassInterface(); > > else > > - IFace = cast<ObjCInterfaceDecl>(D->getDeclContext()); > > + IFace = cast<ObjCInterfaceDecl>(Ctx); > > if (!IFace) > > return; > > @@ -7243,6 +7254,17 @@ void Sema::ProcessDeclAttributeList(Scop > > } > > } > > } > > + > > + // Do this check after processing D's attributes because the attribute > > + // objc_method_family can change whether the given method is in the init > > + // family, and it can be applied after objc_designated_initializer. This > > is a > > + // bit of a hack, but we need it to be compatible with versions of clang > > that > > + // processed the attribute list in the wrong order. > > + if (D->hasAttr<ObjCDesignatedInitializerAttr>() && > > + cast<ObjCMethodDecl>(D)->getMethodFamily() != OMF_init) { > > + Diag(D->getLocation(), diag::err_designated_init_attr_non_init); > > + D->dropAttr<ObjCDesignatedInitializerAttr>(); > > + } > > } > > // Helper for delayed processing TransparentUnion attribute. > > > > Modified: > > cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test > > URL: > > https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_test_Misc_pragma-2Dattribute-2Dsupported-2Dattributes-2Dlist.test-3Frev-3D353976-26r1-3D353975-26r2-3D353976-26view-3Ddiff&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=SS1gIdRrXBd-rCKs5rBEEPmEevgsHAkkERh1ko2vPEc&s=WG3dj5MItvLs5XeEzU66Ghla6jowRBTj94bOG49Bz0g&e= > > ============================================================================== > > --- cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test > > (original) > > +++ cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test Wed > > Feb 13 12:32:37 2019 > > @@ -97,6 +97,7 @@ > > // CHECK-NEXT: ObjCBridge (SubjectMatchRule_record, > > SubjectMatchRule_type_alias) > > // CHECK-NEXT: ObjCBridgeMutable (SubjectMatchRule_record) > > // CHECK-NEXT: ObjCBridgeRelated (SubjectMatchRule_record) > > +// CHECK-NEXT: ObjCDesignatedInitializer (SubjectMatchRule_objc_method) > > // CHECK-NEXT: ObjCException (SubjectMatchRule_objc_interface) > > // CHECK-NEXT: ObjCExplicitProtocolImpl (SubjectMatchRule_objc_protocol) > > // CHECK-NEXT: ObjCExternallyRetained > > (SubjectMatchRule_variable_not_is_parameter, SubjectMatchRule_function, > > SubjectMatchRule_block, SubjectMatchRule_objc_method) > > > > Modified: cfe/trunk/test/SemaObjC/attr-designated-init.m > > URL: > > https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_test_SemaObjC_attr-2Ddesignated-2Dinit.m-3Frev-3D353976-26r1-3D353975-26r2-3D353976-26view-3Ddiff&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=SS1gIdRrXBd-rCKs5rBEEPmEevgsHAkkERh1ko2vPEc&s=qFGKSW2h6UsOH36D_A3kkAWh1P-zIl2zj0dNdzK5fZU&e= > > ============================================================================== > > --- cfe/trunk/test/SemaObjC/attr-designated-init.m (original) > > +++ cfe/trunk/test/SemaObjC/attr-designated-init.m Wed Feb 13 12:32:37 2019 > > @@ -3,7 +3,7 @@ > > #define NS_DESIGNATED_INITIALIZER > > __attribute__((objc_designated_initializer)) > > #define NS_UNAVAILABLE __attribute__((unavailable)) > > -void fnfoo(void) NS_DESIGNATED_INITIALIZER; // expected-error {{only > > applies to init methods of interface or class extension declarations}} > > +void fnfoo(void) NS_DESIGNATED_INITIALIZER; // expected-error > > {{'objc_designated_initializer' attribute only applies to Objective-C > > methods}} > > @protocol P1 > > -(id)init NS_DESIGNATED_INITIALIZER; // expected-error {{only applies to > > init methods of interface or class extension declarations}} > > @@ -428,3 +428,16 @@ __attribute__((objc_root_class)) > > @interface CategoryForMissingInterface(Cat) // expected-error{{cannot find > > interface declaration}} > > - (instancetype)init NS_DESIGNATED_INITIALIZER; // expected-error{{only > > applies to init methods of interface or class extension declarations}} > > @end > > + > > +@interface TwoAttrs > > +-(instancetype)foo > > + __attribute__((objc_designated_initializer)) > > + __attribute__((objc_method_family(init))); > > +-(instancetype)bar > > + __attribute__((objc_method_family(init))) > > + __attribute__((objc_designated_initializer)); > > +-(instancetype)baz > > + __attribute__((objc_designated_initializer, objc_method_family(init))); > > +-(instancetype)quux > > + __attribute__((objc_method_family(init), objc_designated_initializer)); > > +@end > > > > > > _______________________________________________ > > cfe-commits mailing list > > cfe-commits@lists.llvm.org > > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_cfe-2Dcommits&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=SS1gIdRrXBd-rCKs5rBEEPmEevgsHAkkERh1ko2vPEc&s=CUgh_2TLwbP045tLWSjPb2ggagK8hronounsArE9dbQ&e= > > > > > > _______________________________________________ > > cfe-commits mailing list > > cfe-commits@lists.llvm.org > > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits