[PATCH] D73755: [objc_direct] Small updates to help with adoption.

[Erik Pilkington via Phabricator via cfe-commits]

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