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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
