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

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added inline comments.


================
Comment at: clang/lib/Sema/SemaObjCProperty.cpp:1630-1637
+  if (PIDecl->getPropertyImplementation() == ObjCPropertyImplDecl::Dynamic &&
+      PIDecl->getPropertyDecl() &&
+      PIDecl->getPropertyDecl()->isDirectProperty()) {
+    Diag(PropertyLoc, diag::err_objc_direct_dynamic_property) << PropertyId;
+    Diag(PIDecl->getPropertyDecl()->getLocation(),
+         diag::note_previous_declaration);
+    return nullptr;
----------------
erik.pilkington wrote:
> Would you mind adding a test for this diagnostic? It looks like err_objc_direct_dynamic_property doesn't take a parameter too, so there isn't any reason to do `<< PropertyId` unless you add one.
there were some, I forgot to add the test case to the commit ...


================
Comment at: clang/lib/Sema/SemaObjCProperty.cpp:2436
+  if (!GetterMethod) {
+    if (const ObjCCategoryDecl *CatDecl = dyn_cast<ObjCCategoryDecl>(CD)) {
+      auto *ExistingGetter = CatDecl->getClassInterface()->lookupMethod(
----------------
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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73755/new/

https://reviews.llvm.org/D73755