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

[Pierre Habouzit via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3adcc78a8071: [objc_direct] Small updates to help with 
adoption. (authored by MadCoder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73755

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/FixIt/fixit-objc-direct.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/category-direct-properties.m
  clang/test/SemaObjC/dynamic-direct-properties.m
  clang/test/SemaObjC/method-direct.m

Index: clang/test/SemaObjC/method-direct.m
===
--- clang/test/SemaObjC/method-direct.m
+++ clang/test/SemaObjC/method-direct.m
@@ -18,6 +18,7 @@
 + (void)classRootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
 - (void)otherRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherRootDirect' declared here}}
 + (void)otherClassRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherClassRootDirect' declared here}}
++ (void)otherOtherClassRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherOtherClassRootDirect' declared here}}
 - (void)notDirectInIface; // expected-note {{previous declaration is here}}
 + (void)classNotDirectInIface;// expected-note {{previous declaration is here}}
 @end
@@ -48,8 +49,9 @@
 + (void)classRootCategoryDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
 @end
 
-__attribute__((objc_root_class, objc_direct_members)) // expected-error {{'objc_direct_members' attribute only applies to Objective-C implementation declarations and Objective-C containers}}
-@interface SubDirectFail : Root
+__attribute__((objc_direct_members))
+@interface SubDirectMembers : Root
+@property int foo; // expected-note {{previous declaration is here}}
 - (instancetype)init;
 @end
 
@@ -94,6 +96,8 @@
 + (void)otherClassRootDirect {
   [self someRootDirectMethod]; // expected-error {{messaging a Class with a method that is possibly direct}}
 }
++ (void)otherOtherClassRootDirect {
+}
 - (void)rootExtensionDirect {
 }
 + (void)classRootExtensionDirect {
@@ -135,6 +139,16 @@
 - (void)someValidSubMethod {
   [super otherRootDirect]; // expected-error {{messaging super with a direct method}}
 }
++ (void)someValidSubMethod {
+  [super otherOtherClassRootDirect]; // expected-error {{messaging super with a direct method}}
+}
+@end
+
+@implementation SubDirectMembers
+@dynamic foo; // expected-error {{direct property cannot be @dynamic}}
+- (instancetype)init {
+  return self;
+}
 @end
 
 extern void callMethod(id obj, Class cls);
Index: clang/test/SemaObjC/dynamic-direct-properties.m
===
--- /dev/null
+++ clang/test/SemaObjC/dynamic-direct-properties.m
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+__attribute__((objc_root_class))
+@interface Foo
+@property() int dynamic_property;
+@property(direct) int direct_property; // expected-note {{previous declaration is here}}
+@end
+
+@implementation Foo
+@dynamic dynamic_property;
+@dynamic direct_property; // expected-error {{direct property cannot be @dynamic}}
+@end
+
+@interface Foo (Bar)
+@property() int dynamic_category_property;
+@property(direct) int direct_category_property; // expected-note {{previous declaration is here}}
+@end
+
+@implementation Foo (Bar)
+@dynamic dynamic_category_property;
+@dynamic direct_category_property; // expected-error {{direct property cannot be @dynamic}}
+@end
Index: clang/test/SemaObjC/category-direct-properties.m
===
--- /dev/null
+++ clang/test/SemaObjC/category-direct-properties.m
@@ -0,0 +1,273 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+__attribute__((objc_root_class))
+@interface Inteface_Implementation
+@property(nonatomic, readonly) int normal_normal;
+@property(nonatomic, readonly, direct) int direct_normal;
+@property(nonatomic, readonly) int normal_direct; // expected-note {{previous declaration is here}}
+@property(nonatomic, readonly, direct) int direct_direct;
+@end
+
+@implementation Inteface_Implementation
+- (int)normal_normal {
+  return 42;
+}
+- (int)direct_normal {
+  return 42;
+}
+- (int)normal_direct __attribute__((objc_direct)) { // expected-error {{direct method implementation was previously declared not direct}}
+  return 42;
+}
+- (int)direct_direct __attribute__((objc_direct)) {
+  return 42;
+}
+@end
+
+__attribute__((objc_root_class))
+@interface 

[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(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

[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(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

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

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 244773.
MadCoder marked 10 inline comments as done.
MadCoder added a comment.

@erik.pilkington review feedback


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

https://reviews.llvm.org/D73755

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/FixIt/fixit-objc-direct.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/category-direct-properties.m
  clang/test/SemaObjC/dynamic-direct-properties.m
  clang/test/SemaObjC/method-direct.m

Index: clang/test/SemaObjC/method-direct.m
===
--- clang/test/SemaObjC/method-direct.m
+++ clang/test/SemaObjC/method-direct.m
@@ -18,6 +18,7 @@
 + (void)classRootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
 - (void)otherRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherRootDirect' declared here}}
 + (void)otherClassRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherClassRootDirect' declared here}}
++ (void)otherOtherClassRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherOtherClassRootDirect' declared here}}
 - (void)notDirectInIface; // expected-note {{previous declaration is here}}
 + (void)classNotDirectInIface;// expected-note {{previous declaration is here}}
 @end
@@ -48,8 +49,9 @@
 + (void)classRootCategoryDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
 @end
 
-__attribute__((objc_root_class, objc_direct_members)) // expected-error {{'objc_direct_members' attribute only applies to Objective-C implementation declarations and Objective-C containers}}
-@interface SubDirectFail : Root
+__attribute__((objc_direct_members))
+@interface SubDirectMembers : Root
+@property int foo; // expected-note {{previous declaration is here}}
 - (instancetype)init;
 @end
 
@@ -94,6 +96,8 @@
 + (void)otherClassRootDirect {
   [self someRootDirectMethod]; // expected-error {{messaging a Class with a method that is possibly direct}}
 }
++ (void)otherOtherClassRootDirect {
+}
 - (void)rootExtensionDirect {
 }
 + (void)classRootExtensionDirect {
@@ -135,6 +139,16 @@
 - (void)someValidSubMethod {
   [super otherRootDirect]; // expected-error {{messaging super with a direct method}}
 }
++ (void)someValidSubMethod {
+  [super otherOtherClassRootDirect]; // expected-error {{messaging super with a direct method}}
+}
+@end
+
+@implementation SubDirectMembers
+@dynamic foo; // expected-error {{direct property cannot be @dynamic}}
+- (instancetype)init {
+  return self;
+}
 @end
 
 extern void callMethod(id obj, Class cls);
Index: clang/test/SemaObjC/dynamic-direct-properties.m
===
--- /dev/null
+++ clang/test/SemaObjC/dynamic-direct-properties.m
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+__attribute__((objc_root_class))
+@interface Foo
+@property() int dynamic_property;
+@property(direct) int direct_property; // expected-note {{previous declaration is here}}
+@end
+
+@implementation Foo
+@dynamic dynamic_property;
+@dynamic direct_property; // expected-error {{direct property cannot be @dynamic}}
+@end
+
+@interface Foo (Bar)
+@property() int dynamic_category_property;
+@property(direct) int direct_category_property; // expected-note {{previous declaration is here}}
+@end
+
+@implementation Foo (Bar)
+@dynamic dynamic_category_property;
+@dynamic direct_category_property; // expected-error {{direct property cannot be @dynamic}}
+@end
Index: clang/test/SemaObjC/category-direct-properties.m
===
--- /dev/null
+++ clang/test/SemaObjC/category-direct-properties.m
@@ -0,0 +1,273 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+__attribute__((objc_root_class))
+@interface Inteface_Implementation
+@property(nonatomic, readonly) int normal_normal;
+@property(nonatomic, readonly, direct) int direct_normal;
+@property(nonatomic, readonly) int normal_direct; // expected-note {{previous declaration is here}}
+@property(nonatomic, readonly, direct) int direct_direct;
+@end
+
+@implementation Inteface_Implementation
+- (int)normal_normal {
+  return 42;
+}
+- (int)direct_normal {
+  return 42;
+}
+- (int)normal_direct __attribute__((objc_direct)) { // expected-error {{direct method implementation was previously declared not direct}}
+  return 42;
+}
+- (int)direct_direct __attribute__((objc_direct)) {
+  return 42;
+}
+@end
+
+__attribute__((objc_root_class))
+@interface Inteface_Extension
+@property(nonatomic, readonly) int normal_normal;

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

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

In D73755#1874962 , @plotfi wrote:

> @MadCoder Could there be a way to mark objc_direct methods as something like 
> __attribute__((visibility("default"))) or __declspec(dllexport) to denote 
> that a given method should be directly callable but still retain the metadata 
> to be called through an objc_msgSend from an external dylib? Could that help 
> to boost adoption?


I'm not sure how that is relevant to this review. secondly this kind of defeats 
the entire point of `objc_direct` which is to _not_ generate all that metadata.


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

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

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

@MadCoder Could there be a way to mark objc_direct methods as something like 
__attribute__((visibility("default"))) or __declspec(dllexport) to denote that 
a given method should be directly callable but still retain the metadata to be 
called through an objc_msgSend from an external dylib? Could that help to boost 
adoption?


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

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

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:1005-1006
 def err_objc_direct_duplicate_decl : Error<
-  "%select{|direct }0method declaration conflicts "
-  "with previous %select{|direct }1declaration of method %2">;
+  "%select{|direct }0%select{method|property}1 declaration conflicts "
+  "with previous %select{|direct }2declaration of method %3">;
 def err_objc_direct_impl_decl_mismatch : Error<

This diagnostics reads a bit awkward when a property is conflicting with a 
property, since the the first half of the diagnostic is talking about the 
property declaration, but the second half is implicitly referring to the 
generated getter/setter.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:1023
+def err_objc_direct_dynamic_property : Error<
+  "property was previously declared to be direct">;
 

I think this might be more clear as: "direct property cannot be @dynamic"



Comment at: clang/lib/Sema/SemaExprObjC.cpp:3032
+  }
+  Builder.~DiagnosticBuilder();
   Diag(Method->getLocation(), diag::note_direct_method_declared_at)

I believe its UB to call a destructor twice. Please use a compound statement to 
control where the dtor will be called here.



Comment at: clang/lib/Sema/SemaExprObjC.cpp:3046
+  }
+  Builder.~DiagnosticBuilder();
   Diag(Method->getLocation(), diag::note_direct_method_declared_at)

ditto



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;

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.



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

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?



Comment at: clang/test/SemaObjC/method-direct.m:51
 
-__attribute__((objc_root_class, objc_direct_members)) // expected-error 
{{'objc_direct_members' attribute only applies to Objective-C implementation 
declarations and Objective-C containers}}
-@interface SubDirectFail : Root

Can you leave this in so we have a test that has this attribute on an 
`@interface`?


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

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

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 243649.
MadCoder added a comment.

  Add some errors when direct properties are marked @dynamic.


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

https://reviews.llvm.org/D73755

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/FixIt/fixit-objc-direct.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/category-direct-properties.m
  clang/test/SemaObjC/method-direct.m

Index: clang/test/SemaObjC/method-direct.m
===
--- clang/test/SemaObjC/method-direct.m
+++ clang/test/SemaObjC/method-direct.m
@@ -18,6 +18,7 @@
 + (void)classRootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
 - (void)otherRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherRootDirect' declared here}}
 + (void)otherClassRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherClassRootDirect' declared here}}
++ (void)otherOtherClassRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherOtherClassRootDirect' declared here}}
 - (void)notDirectInIface; // expected-note {{previous declaration is here}}
 + (void)classNotDirectInIface;// expected-note {{previous declaration is here}}
 @end
@@ -48,11 +49,6 @@
 + (void)classRootCategoryDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
 @end
 
-__attribute__((objc_root_class, objc_direct_members)) // expected-error {{'objc_direct_members' attribute only applies to Objective-C implementation declarations and Objective-C containers}}
-@interface SubDirectFail : Root
-- (instancetype)init;
-@end
-
 @interface Sub : Root 
 /* invalid overrides with directs */
 - (void)rootRegular __attribute__((objc_direct));   // expected-error {{methods that override superclass methods cannot be direct}}
@@ -94,6 +90,8 @@
 + (void)otherClassRootDirect {
   [self someRootDirectMethod]; // expected-error {{messaging a Class with a method that is possibly direct}}
 }
++ (void)otherOtherClassRootDirect {
+}
 - (void)rootExtensionDirect {
 }
 + (void)classRootExtensionDirect {
@@ -135,6 +133,9 @@
 - (void)someValidSubMethod {
   [super otherRootDirect]; // expected-error {{messaging super with a direct method}}
 }
++ (void)someValidSubMethod {
+  [super otherOtherClassRootDirect]; // expected-error {{messaging super with a direct method}}
+}
 @end
 
 extern void callMethod(id obj, Class cls);
Index: clang/test/SemaObjC/category-direct-properties.m
===
--- /dev/null
+++ clang/test/SemaObjC/category-direct-properties.m
@@ -0,0 +1,273 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+__attribute__((objc_root_class))
+@interface Inteface_Implementation
+@property(nonatomic, readonly) int normal_normal;
+@property(nonatomic, readonly, direct) int direct_normal;
+@property(nonatomic, readonly) int normal_direct; // expected-note {{previous declaration is here}}
+@property(nonatomic, readonly, direct) int direct_direct;
+@end
+
+@implementation Inteface_Implementation
+- (int)normal_normal {
+  return 42;
+}
+- (int)direct_normal {
+  return 42;
+}
+- (int)normal_direct __attribute__((objc_direct)) { // expected-error {{direct method implementation was previously declared not direct}}
+  return 42;
+}
+- (int)direct_direct __attribute__((objc_direct)) {
+  return 42;
+}
+@end
+
+__attribute__((objc_root_class))
+@interface Inteface_Extension
+@property(nonatomic, readonly) int normal_normal;
+@property(nonatomic, readonly, direct) int direct_normal;
+@property(nonatomic, readonly) int normal_direct;
+@property(nonatomic, readonly, direct) int direct_direct;
+@end
+
+@interface Inteface_Extension ()
+@property(nonatomic, readwrite) int normal_normal;
+@property(nonatomic, readwrite) int direct_normal;
+@property(nonatomic, readwrite, direct) int normal_direct;
+@property(nonatomic, readwrite, direct) int direct_direct;
+@end
+
+@implementation Inteface_Extension
+@end
+
+__attribute__((objc_root_class))
+@interface Extension_Implementation
+@end
+
+@interface Extension_Implementation ()
+@property(nonatomic, readwrite) int normal_normal;
+@property(nonatomic, readwrite, direct) int direct_normal;
+@property(nonatomic, readwrite) int normal_direct; // expected-note {{previous declaration is here}}
+@property(nonatomic, readwrite, direct) int direct_direct;
+@end
+
+@implementation Extension_Implementation
+- (int)normal_normal {
+  return 42;
+}
+- (int)direct_normal {
+  return 42;
+}
+- (int)normal_direct __attribute__((objc_direct)) { // expected-error {{direct method 

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

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder created this revision.
MadCoder added reviewers: ahatanak, dexonsmith, arphaman, erik.pilkington.
MadCoder added a project: clang.
Herald added subscribers: cfe-commits, jfb.

Add fixits for messaging self in MRR or using super, as the intent is
clear, and it turns out people do that a lot more than expected.

Allow for objc_direct_members on main interfaces, it's extremely useful
for internal only classes, and proves to be quite annoying for adoption.

Add some better warnings around properties direct/non-direct clashes (it
was done for methods but properties were a miss).

Radar-Id: rdar://problem/58355212


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73755

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/FixIt/fixit-objc-direct.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/category-direct-properties.m
  clang/test/SemaObjC/method-direct.m

Index: clang/test/SemaObjC/method-direct.m
===
--- clang/test/SemaObjC/method-direct.m
+++ clang/test/SemaObjC/method-direct.m
@@ -18,6 +18,7 @@
 + (void)classRootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
 - (void)otherRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherRootDirect' declared here}}
 + (void)otherClassRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherClassRootDirect' declared here}}
++ (void)otherOtherClassRootDirect __attribute__((objc_direct)); // expected-note {{direct method 'otherOtherClassRootDirect' declared here}}
 - (void)notDirectInIface; // expected-note {{previous declaration is here}}
 + (void)classNotDirectInIface;// expected-note {{previous declaration is here}}
 @end
@@ -48,11 +49,6 @@
 + (void)classRootCategoryDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
 @end
 
-__attribute__((objc_root_class, objc_direct_members)) // expected-error {{'objc_direct_members' attribute only applies to Objective-C implementation declarations and Objective-C containers}}
-@interface SubDirectFail : Root
-- (instancetype)init;
-@end
-
 @interface Sub : Root 
 /* invalid overrides with directs */
 - (void)rootRegular __attribute__((objc_direct));   // expected-error {{methods that override superclass methods cannot be direct}}
@@ -94,6 +90,8 @@
 + (void)otherClassRootDirect {
   [self someRootDirectMethod]; // expected-error {{messaging a Class with a method that is possibly direct}}
 }
++ (void)otherOtherClassRootDirect {
+}
 - (void)rootExtensionDirect {
 }
 + (void)classRootExtensionDirect {
@@ -135,6 +133,9 @@
 - (void)someValidSubMethod {
   [super otherRootDirect]; // expected-error {{messaging super with a direct method}}
 }
++ (void)someValidSubMethod {
+  [super otherOtherClassRootDirect]; // expected-error {{messaging super with a direct method}}
+}
 @end
 
 extern void callMethod(id obj, Class cls);
Index: clang/test/SemaObjC/category-direct-properties.m
===
--- /dev/null
+++ clang/test/SemaObjC/category-direct-properties.m
@@ -0,0 +1,273 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+__attribute__((objc_root_class))
+@interface Inteface_Implementation
+@property(nonatomic, readonly) int normal_normal;
+@property(nonatomic, readonly, direct) int direct_normal;
+@property(nonatomic, readonly) int normal_direct; // expected-note {{previous declaration is here}}
+@property(nonatomic, readonly, direct) int direct_direct;
+@end
+
+@implementation Inteface_Implementation
+- (int)normal_normal {
+  return 42;
+}
+- (int)direct_normal {
+  return 42;
+}
+- (int)normal_direct __attribute__((objc_direct)) { // expected-error {{direct method implementation was previously declared not direct}}
+  return 42;
+}
+- (int)direct_direct __attribute__((objc_direct)) {
+  return 42;
+}
+@end
+
+__attribute__((objc_root_class))
+@interface Inteface_Extension
+@property(nonatomic, readonly) int normal_normal;
+@property(nonatomic, readonly, direct) int direct_normal;
+@property(nonatomic, readonly) int normal_direct;
+@property(nonatomic, readonly, direct) int direct_direct;
+@end
+
+@interface Inteface_Extension ()
+@property(nonatomic, readwrite) int normal_normal;
+@property(nonatomic, readwrite) int direct_normal;
+@property(nonatomic, readwrite, direct) int normal_direct;
+@property(nonatomic, readwrite, direct) int direct_direct;
+@end
+
+@implementation Inteface_Extension
+@end
+
+__attribute__((objc_root_class))
+@interface Extension_Implementation
+@end
+
+@interface Extension_Implementation ()
+@property(nonatomic,