erik.pilkington accepted this revision. erik.pilkington added a comment. This revision is now accepted and ready to land.
LGTM, thanks. ================ Comment at: clang/lib/Sema/SemaObjCProperty.cpp:2436 + if (!GetterMethod) { + if (const ObjCCategoryDecl *CatDecl = dyn_cast<ObjCCategoryDecl>(CD)) { + auto *ExistingGetter = CatDecl->getClassInterface()->lookupMethod( ---------------- MadCoder wrote: > erik.pilkington wrote: > > Just to be clear: so we need this check because the getter/setters of a > > property declared in a category aren't considered to override any > > getters/setters of the extended class, so the existing > > CheckObjCMethodOverrides checks below don't work? > not quite, it's because people do things like this: > > ``` > @interface Foo (Cat) > @property int bar; > @end > ``` > > But implement the property in the main `@implementation` (or the other way > around) and this is not considered as overrides today, or even worse, many > many times the Category doesn't even have a corresponding `@implementation` > anywhere. Tthere's one direction of this that has a possible optional warning > though today in the compiler. > > But direct method resolution requires to not cross streams else the incorrect > symbol is formed (because the category is an actual different namespace). > With dynamism it actually doesn't matter which is why the compiler doesn't > care today. Okay, thanks for clarifying. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73755/new/ https://reviews.llvm.org/D73755 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits