[PATCH] D82611: [SemaObjC] Add a warning for @selector expressions that potentially refer to objc_direct methods

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

In D82611#2133521 , @erik.pilkington 
wrote:

> In D82611#2125868 , @MadCoder wrote:
>
> >
>




>> I would suggest something like `-Wstrict-direct-dispatch` or something.
> 
> I kinda prefer `-Wpotentially-direct-selector`, since that seems to more 
> closely correspond to what the compiler is complaining about. WDYT?

Yes, that is better, I wasn't dead-set on the name.


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

https://reviews.llvm.org/D82611



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82611: [SemaObjC] Add a warning for @selector expressions that potentially refer to objc_direct methods

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

So the risk with that one is that if someone had say the `-didSomething` direct 
selector and that some UIKit/Apple SDK API now adds this as a thing that you 
use with CFNotification center for example, where you _need_ dynamism, then the 
uses of the API would warn all the time when the internal to the client project 
`-didSomething` is in scope.

So we ideally need a way to silence this warning, so at the very least this 
needs to be a new `-W` flag (on by default (?)) so that callers that have an 
issue can use a `_Pragma` at the call site to ignore the error for when it's 
legal.

I would suggest something like `-Wstrict-direct-dispatch` or something.


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

https://reviews.llvm.org/D82611



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82611: [SemaObjC] Add a warning for @selector expressions that potentially refer to objc_direct methods

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added inline comments.



Comment at: clang/lib/Sema/SemaExprObjC.cpp:1230
 }
-
-static void HelperToDiagnoseDirectSelectorsExpr(Sema &S, SourceLocation AtLoc,
-Selector Sel,
+static void HelperToDiagnoseDirectSelectorsExpr(Sema &S, Selector Sel,
 ObjCMethodList &MethList,

missing line between the functions?


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

https://reviews.llvm.org/D82611



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D82611: [SemaObjC] Add a warning for @selector expressions that potentially refer to objc_direct methods

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added inline comments.



Comment at: clang/test/SemaObjC/potentially-direct-selector.m:84
+  (void)@selector(inBaseCat);
+  (void)@selector(inBaseCatImpl);
+  (void)@selector(inDerived);

I think this might be too weak, if we add a warning then maybe what we want is 
a tri-state:

`-Wstrict-direct-dispatch=none|self|all`

because I can see cases where the heuristic here would be too weak


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

https://reviews.llvm.org/D82611



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76643: [objc_direct] also go through implementations when looking for clashes

[Pierre Habouzit via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG20d704a75ed5: [objc_direct] also go through implementations 
when looking for clashes (authored by MadCoder).

Changed prior to commit:
  https://reviews.llvm.org/D76643?vs=252162&id=252214#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76643

Files:
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/SemaObjC/method-direct-one-definition.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
@@ -12,6 +12,7 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (void)unavailableInChild;
 - (void)rootRegular;  // expected-note {{previous declaration is here}}
 + (void)classRootRegular; // expected-note {{previous declaration is here}}
 - (void)rootDirect __attribute__((objc_direct));  // expected-note {{previous declaration is here}};
@@ -52,6 +53,7 @@
 __attribute__((objc_direct_members))
 @interface SubDirectMembers : Root
 @property int foo; // expected-note {{previous declaration is here}}
+- (void)unavailableInChild __attribute__((unavailable)); // should not warn
 - (instancetype)init;
 @end
 
@@ -81,6 +83,8 @@
 
 __attribute__((objc_direct_members))
 @implementation Root
+- (void)unavailableInChild {
+}
 - (void)rootRegular {
 }
 + (void)classRootRegular {
Index: clang/test/SemaObjC/method-direct-one-definition.m
===
--- clang/test/SemaObjC/method-direct-one-definition.m
+++ clang/test/SemaObjC/method-direct-one-definition.m
@@ -30,6 +30,15 @@
 - (void)B_OtherCat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
 @end
 
+@implementation B
+- (void)B_primary {
+}
+- (void)B_extension {
+}
+- (void)B_implOnly __attribute__((objc_direct)) { // expected-note {{previous declaration is here}}
+}
+@end
+
 @implementation B (Cat)
 - (void)B_primary { // expected-error {{direct method was declared in the primary interface but is implemented in a category}}
 }
@@ -39,6 +48,8 @@
 }
 - (void)B_OtherCat { // expected-error {{direct method was declared in a category but is implemented in a different category}}
 }
+- (void)B_implOnly __attribute__((objc_direct)) { // expected-error {{direct method declaration conflicts with previous direct declaration of method 'B_implOnly'}}
+}
 @end
 
 __attribute__((objc_root_class))
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4581,6 +4581,62 @@
   << (Triple.isMacOSX() ? "macOS 10.11" : "iOS 9");
 }
 
+static void mergeObjCDirectMembers(Sema &S, Decl *CD, ObjCMethodDecl *Method) {
+  if (!Method->isDirectMethod() && !Method->hasAttr() &&
+  CD->hasAttr()) {
+Method->addAttr(
+ObjCDirectAttr::CreateImplicit(S.Context, Method->getLocation()));
+  }
+}
+
+static void checkObjCDirectMethodClashes(Sema &S, ObjCInterfaceDecl *IDecl,
+ ObjCMethodDecl *Method,
+ ObjCImplDecl *ImpDecl = nullptr) {
+  auto Sel = Method->getSelector();
+  bool isInstance = Method->isInstanceMethod();
+  bool diagnosed = false;
+
+  auto diagClash = [&](const ObjCMethodDecl *IMD) {
+if (diagnosed || IMD->isImplicit())
+  return;
+if (Method->isDirectMethod() || IMD->isDirectMethod()) {
+  S.Diag(Method->getLocation(), diag::err_objc_direct_duplicate_decl)
+  << Method->isDirectMethod() << /* method */ 0 << IMD->isDirectMethod()
+  << Method->getDeclName();
+  S.Diag(IMD->getLocation(), diag::note_previous_declaration);
+  diagnosed = true;
+}
+  };
+
+  // Look for any other declaration of this method anywhere we can see in this
+  // compilation unit.
+  //
+  // We do not use IDecl->lookupMethod() because we have specific needs:
+  //
+  // - we absolutely do not need to walk protocols, because
+  //   diag::err_objc_direct_on_protocol has already been emitted
+  //   during parsing if there's a conflict,
+  //
+  // - when we do not find a match in a given @interface container,
+  //   we need to attempt looking it up in the @implementation block if the
+  //   translation unit sees it to find more clashes.
+
+  if (auto *IMD = IDecl->getMethod(Sel, isInstance))
+diagClash(IMD);
+  else if (auto *Impl = IDecl->getImplementation())
+if (Impl != ImpDecl)
+  if (auto *IMD = IDecl->getImplementation()->getMethod(Sel, isInstance))
+diagClash(IMD);
+
+  for (const auto *Cat : IDecl->visible_categories())
+if (auto *IMD = Cat->getMethod(Sel, isInstance))
+  diagClash(IMD);
+else if (auto CatImpl

[PATCH] D76643: [objc_direct] also go through implementations when looking for clashes

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 252162.
MadCoder edited the summary of this revision.

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

https://reviews.llvm.org/D76643

Files:
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/SemaObjC/method-direct-one-definition.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
@@ -12,6 +12,7 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (void)unavailableInChild;
 - (void)rootRegular;  // expected-note {{previous declaration is here}}
 + (void)classRootRegular; // expected-note {{previous declaration is here}}
 - (void)rootDirect __attribute__((objc_direct));  // expected-note {{previous declaration is here}};
@@ -52,6 +53,7 @@
 __attribute__((objc_direct_members))
 @interface SubDirectMembers : Root
 @property int foo; // expected-note {{previous declaration is here}}
+- (void)unavailableInChild __attribute__((unavailable)); // should not warn
 - (instancetype)init;
 @end
 
@@ -81,6 +83,8 @@
 
 __attribute__((objc_direct_members))
 @implementation Root
+- (void)unavailableInChild {
+}
 - (void)rootRegular {
 }
 + (void)classRootRegular {
Index: clang/test/SemaObjC/method-direct-one-definition.m
===
--- clang/test/SemaObjC/method-direct-one-definition.m
+++ clang/test/SemaObjC/method-direct-one-definition.m
@@ -30,6 +30,15 @@
 - (void)B_OtherCat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
 @end
 
+@implementation B
+- (void)B_primary {
+}
+- (void)B_extension {
+}
+- (void)B_implOnly __attribute__((objc_direct)) { // expected-note {{previous declaration is here}}
+}
+@end
+
 @implementation B (Cat)
 - (void)B_primary { // expected-error {{direct method was declared in the primary interface but is implemented in a category}}
 }
@@ -39,6 +48,8 @@
 }
 - (void)B_OtherCat { // expected-error {{direct method was declared in a category but is implemented in a different category}}
 }
+- (void)B_implOnly __attribute__((objc_direct)) { // expected-error {{direct method declaration conflicts with previous direct declaration of method 'B_implOnly'}}
+}
 @end
 
 __attribute__((objc_root_class))
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4581,6 +4581,62 @@
   << (Triple.isMacOSX() ? "macOS 10.11" : "iOS 9");
 }
 
+static void mergeObjCDirectMembers(Sema &S, Decl *CD, ObjCMethodDecl *Method) {
+  if (!Method->isDirectMethod() && !Method->hasAttr() &&
+  CD->hasAttr()) {
+Method->addAttr(
+ObjCDirectAttr::CreateImplicit(S.Context, Method->getLocation()));
+  }
+}
+
+static void checkObjCDirectMethodClashes(Sema &S, ObjCInterfaceDecl *IDecl,
+ ObjCMethodDecl *Method,
+ ObjCImplDecl *ImpDecl = nullptr) {
+  auto Sel = Method->getSelector();
+  bool isInstance = Method->isInstanceMethod();
+  bool diagnosed = false;
+
+  auto diagClash = [&](const ObjCMethodDecl *IMD) {
+if (diagnosed || IMD->isImplicit())
+  return;
+if (Method->isDirectMethod() || IMD->isDirectMethod()) {
+  S.Diag(Method->getLocation(), diag::err_objc_direct_duplicate_decl)
+  << Method->isDirectMethod() << /* method */ 0 << IMD->isDirectMethod()
+  << Method->getDeclName();
+  S.Diag(IMD->getLocation(), diag::note_previous_declaration);
+  diagnosed = true;
+}
+  };
+
+  // Look for any other declaration of this method anywhere we can see in this
+  // compilation unit.
+  //
+  // We do not use IDecl->lookupMethod() because we have specific needs:
+  //
+  // - we absolutely do not need to walk protocols, because
+  //   diag::err_objc_direct_on_protocol has already been emitted
+  //   during parsing if there's a conflict,
+  //
+  // - when we do not find a match in a given @interface container,
+  //   we need to attempt looking up int he @implementation block if the
+  //   translation unit sees it to find more clashes.
+
+  if (auto *IMD = IDecl->getMethod(Sel, isInstance))
+diagClash(IMD);
+  else if (auto *Impl = IDecl->getImplementation())
+if (Impl != ImpDecl)
+  if (auto *IMD = IDecl->getImplementation()->getMethod(Sel, isInstance))
+diagClash(IMD);
+
+  for (const auto *Cat : IDecl->visible_categories())
+if (auto *IMD = Cat->getMethod(Sel, isInstance))
+  diagClash(IMD);
+else if (auto CatImpl = Cat->getImplementation())
+  if (CatImpl != ImpDecl)
+if (auto *IMD = Cat->getMethod(Sel, isInstance))
+  diagClash(IMD);
+}
+
 Decl *Sema::ActOnMethodDeclaration(
 Scope *S, SourceLocation MethodLoc, SourceLo

[PATCH] D76643: [objc_direct] also go through implementations when looking for clashes

[Pierre Habouzit via Phabricator via cfe-commits]

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

Some methods are sometimes declared in the @implementation blocks which can 
cause undiagnosed clashes.

Just write a checkObjCDirectMethodClashes() for this purpose.

Signed-off-by: Pierre Habouzit 
Radar-ID: rdar://problem/59332804


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76643

Files:
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/SemaObjC/method-direct-one-definition.m

Index: clang/test/SemaObjC/method-direct-one-definition.m
===
--- clang/test/SemaObjC/method-direct-one-definition.m
+++ clang/test/SemaObjC/method-direct-one-definition.m
@@ -30,6 +30,15 @@
 - (void)B_OtherCat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
 @end
 
+@implementation B
+- (void)B_primary {
+}
+- (void)B_extension {
+}
+- (void)B_implOnly __attribute__((objc_direct)) { // expected-note {{previous declaration is here}}
+}
+@end
+
 @implementation B (Cat)
 - (void)B_primary { // expected-error {{direct method was declared in the primary interface but is implemented in a category}}
 }
@@ -39,6 +48,8 @@
 }
 - (void)B_OtherCat { // expected-error {{direct method was declared in a category but is implemented in a different category}}
 }
+- (void)B_implOnly __attribute__((objc_direct)) { // expected-error {{direct method declaration conflicts with previous direct declaration of method 'B_implOnly'}}
+}
 @end
 
 __attribute__((objc_root_class))
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4581,6 +4581,61 @@
   << (Triple.isMacOSX() ? "macOS 10.11" : "iOS 9");
 }
 
+static void mergeObjCDirectMembers(Sema &S, Decl *CD, ObjCMethodDecl *Method) {
+  if (!Method->isDirectMethod() && CD->hasAttr()) {
+Method->addAttr(
+ObjCDirectAttr::CreateImplicit(S.Context, Method->getLocation()));
+  }
+}
+
+static void checkObjCDirectMethodClashes(Sema &S, ObjCInterfaceDecl *IDecl,
+ ObjCMethodDecl *Method,
+ ObjCImplDecl *ImpDecl = nullptr) {
+  auto Sel = Method->getSelector();
+  bool isInstance = Method->isInstanceMethod();
+  bool diagnosed = false;
+
+  auto diagClash = [&](const ObjCMethodDecl *IMD) {
+if (diagnosed || IMD->isImplicit())
+  return;
+if (Method->isDirectMethod() || IMD->isDirectMethod()) {
+  S.Diag(Method->getLocation(), diag::err_objc_direct_duplicate_decl)
+  << Method->isDirectMethod() << /* method */ 0 << IMD->isDirectMethod()
+  << Method->getDeclName();
+  S.Diag(IMD->getLocation(), diag::note_previous_declaration);
+  diagnosed = true;
+}
+  };
+
+  // Look for any other declaration of this method anywhere we can see in this
+  // compilation unit.
+  //
+  // We do not use IDecl->lookupMethod() because we have specific needs:
+  //
+  // - we absolutely do not need to walk protocols, because
+  //   diag::err_objc_direct_on_protocol has already been emitted
+  //   during parsing if there's a conflict,
+  //
+  // - when we do not find a match in a given @interface container,
+  //   we need to attempt looking up int he @implementation block if the
+  //   translation unit sees it to find more clashes.
+
+  if (auto *IMD = IDecl->getMethod(Sel, isInstance))
+diagClash(IMD);
+  else if (auto *Impl = IDecl->getImplementation())
+if (Impl != ImpDecl)
+  if (auto *IMD = IDecl->getImplementation()->getMethod(Sel, isInstance))
+diagClash(IMD);
+
+  for (const auto *Cat : IDecl->visible_categories())
+if (auto *IMD = Cat->getMethod(Sel, isInstance))
+  diagClash(IMD);
+else if (auto CatImpl = Cat->getImplementation())
+  if (CatImpl != ImpDecl)
+if (auto *IMD = Cat->getMethod(Sel, isInstance))
+  diagClash(IMD);
+}
+
 Decl *Sema::ActOnMethodDeclaration(
 Scope *S, SourceLocation MethodLoc, SourceLocation EndLoc,
 tok::TokenKind MethodType, ObjCDeclSpec &ReturnQT, ParsedType ReturnType,
@@ -4809,9 +4864,9 @@
   Diag(ObjCMethod->getLocation(), diag::warn_dealloc_in_category)
 << ObjCMethod->getDeclName();
 }
-  } else if (ImpDecl->hasAttr()) {
-ObjCMethod->addAttr(
-ObjCDirectAttr::CreateImplicit(Context, ObjCMethod->getLocation()));
+  } else {
+mergeObjCDirectMembers(*this, ClassDecl, ObjCMethod);
+checkObjCDirectMethodClashes(*this, IDecl, ObjCMethod, ImpDecl);
   }
 
   // Warn if a method declared in a protocol to which a category or
@@ -4832,39 +4887,16 @@
 }
   } else {
 if (!isa(ClassDecl)) {
-  if (!ObjCMethod->isDirectMethod() &&
-  ClassDecl->hasAttr()) {
-ObjCMethod->addAttr(
-  

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

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

[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] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Pierre Habouzit via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6eb969b7c5b5: [objc_direct] fix codegen for mismatched 
Decl/Impl return types (authored by MadCoder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73208

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/test/CodeGenObjC/direct-method-ret-mismatch.m


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - 
| FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4032,22 +4032,49 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
-  if (I != DirectMethodDefinitions.end())
-return I->second;
+  auto *COMD = OMD->getCanonicalDecl();
+  auto I = DirectMethodDefinitions.find(COMD);
+  llvm::Function *OldFn = nullptr, *Fn = nullptr;
 
-  SmallString<256> Name;
-  GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/true);
+  if (I != DirectMethodDefinitions.end()) {
+// Objective-C allows for the declaration and implementation types
+// to differ slightly.
+//
+// If we're being asked for the Function associated for a method
+// implementation, a previous value might have been cached
+// based on the type of the canonical declaration.
+//
+// If these do not match, then we'll replace this function with
+// a new one that has the proper type below.
+if (!OMD->getBody() || COMD->getReturnType() == OMD->getReturnType())
+  return I->second;
+OldFn = I->second;
+  }
 
   CodeGenTypes &Types = CGM.getTypes();
   llvm::FunctionType *MethodTy =
 Types.GetFunctionType(Types.arrangeObjCMethodDeclaration(OMD));
-  llvm::Function *Method =
-  llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
- Name.str(), &CGM.getModule());
-  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), 
Method));
 
-  return Method;
+  if (OldFn) {
+Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
+"", &CGM.getModule());
+Fn->takeName(OldFn);
+OldFn->replaceAllUsesWith(
+llvm::ConstantExpr::getBitCast(Fn, OldFn->getType()));
+OldFn->eraseFromParent();
+
+// Replace the cached function in the map.
+I->second = Fn;
+  } else {
+SmallString<256> Name;
+GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/ true);
+
+Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
+Name.str(), &CGM.getModule());
+DirectMethodDefinitions.insert(std::make_pair(COMD, Fn));
+  }
+
+  return Fn;
 }
 
 void CGObjCCommonMac::GenerateDirectMethodPrologue(


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4032,22 +4032,49 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
-  if (I != DirectMethodDefinitions.end())
-return I->second;
+  auto *COMD = OMD->getCanonicalDecl();
+  auto I = DirectMethodDefinitions.find(COMD);
+  llvm::Function *OldFn = nullptr, *Fn = nullptr;
 

[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, readwrite

[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 241613.
MadCoder marked an inline comment as done.

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

https://reviews.llvm.org/D73208

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/test/CodeGenObjC/direct-method-ret-mismatch.m


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - 
| FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4032,22 +4032,49 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
-  if (I != DirectMethodDefinitions.end())
-return I->second;
+  auto *COMD = OMD->getCanonicalDecl();
+  auto I = DirectMethodDefinitions.find(COMD);
+  llvm::Function *OldFn = nullptr, *Fn = nullptr;
 
-  SmallString<256> Name;
-  GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/true);
+  if (I != DirectMethodDefinitions.end()) {
+// Objective-C allows for the declaration and implementation types
+// to differ slightly.
+//
+// If we're being asked for the Function associated for a method
+// implementation, a previous value might have been cached
+// based on the type of the canonical declaration.
+//
+// If these do not match, then we'll replace this function with
+// a new one that has the proper type below.
+if (!OMD->getBody() || COMD->getReturnType() == OMD->getReturnType())
+  return I->second;
+OldFn = I->second;
+  }
 
   CodeGenTypes &Types = CGM.getTypes();
   llvm::FunctionType *MethodTy =
 Types.GetFunctionType(Types.arrangeObjCMethodDeclaration(OMD));
-  llvm::Function *Method =
-  llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
- Name.str(), &CGM.getModule());
-  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), 
Method));
 
-  return Method;
+  if (OldFn) {
+Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
+"", &CGM.getModule());
+Fn->takeName(OldFn);
+OldFn->replaceAllUsesWith(
+llvm::ConstantExpr::getBitCast(Fn, OldFn->getType()));
+OldFn->eraseFromParent();
+
+// Replace the cached function in the map.
+I->second = Fn;
+  } else {
+SmallString<256> Name;
+GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/ true);
+
+Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
+Name.str(), &CGM.getModule());
+DirectMethodDefinitions.insert(std::make_pair(COMD, Fn));
+  }
+
+  return Fn;
 }
 
 void CGObjCCommonMac::GenerateDirectMethodPrologue(


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4032,22 +4032,49 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
-  if (I != DirectMethodDefinitions.end())
-return I->second;
+  auto *COMD = OMD->getCanonicalDecl();
+  auto I = DirectMethodDefinitions.find(COMD);
+  llvm::Function *OldFn = nullptr, *Fn = nullptr;
 
-  SmallString<256> Name;
-  GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/true);
+  if (I != DirectMethodDefinitions.end()) {
+

[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Pierre Habouzit via Phabricator via cfe-commits]

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

@dexonsmith here, I still hook the same method but do it in a more 
LLVM-approved way ;)

It works with minimap perf impect because the only case we call 
GenerateDirectMethod with an Implementation is if we're about to codegen the 
body, so the case when you build against a header without seing the 
@implementation will not be affected perf-wise.


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

https://reviews.llvm.org/D73208

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/test/CodeGenObjC/direct-method-ret-mismatch.m


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - 
| FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4032,22 +4032,50 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
-  if (I != DirectMethodDefinitions.end())
-return I->second;
+  auto *COMD = OMD->getCanonicalDecl();
+  auto I = DirectMethodDefinitions.find(COMD);
+  llvm::Function *OldFn = nullptr, *Fn = nullptr;
 
-  SmallString<256> Name;
-  GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/true);
+  if (I != DirectMethodDefinitions.end()) {
+// Objective-C allows for the declaration and implementation types
+// to differ slightly.
+   //
+   // If we're being asked for the Function associated for a method
+   // implementation, a previous value might have been cached
+   // based on the type of the canonical declaration.
+   //
+   // If these do not match, then we'll replace this function with
+   // a new one that has the proper type below.
+if (!OMD->getBody() || COMD->getReturnType() == OMD->getReturnType()) {
+  return I->second;
+}
+OldFn = I->second;
+  }
 
   CodeGenTypes &Types = CGM.getTypes();
   llvm::FunctionType *MethodTy =
 Types.GetFunctionType(Types.arrangeObjCMethodDeclaration(OMD));
-  llvm::Function *Method =
-  llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
- Name.str(), &CGM.getModule());
-  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), 
Method));
 
-  return Method;
+  if (OldFn) {
+Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
+"", &CGM.getModule());
+Fn->takeName(OldFn);
+OldFn->replaceAllUsesWith(
+llvm::ConstantExpr::getBitCast(Fn, OldFn->getType()));
+OldFn->eraseFromParent();
+
+   // Replace the cached function in the map.
+I->second = Fn;
+  } else {
+SmallString<256> Name;
+GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/ true);
+
+Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
+Name.str(), &CGM.getModule());
+DirectMethodDefinitions.insert(std::make_pair(COMD, Fn));
+  }
+
+  return Fn;
 }
 
 void CGObjCCommonMac::GenerateDirectMethodPrologue(


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4032,22 +4032,50 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCan

[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Pierre Habouzit via Phabricator via cfe-commits]

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

damn you tabs!


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

https://reviews.llvm.org/D73208

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/test/CodeGenObjC/direct-method-ret-mismatch.m


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - 
| FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4032,22 +4032,50 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
-  if (I != DirectMethodDefinitions.end())
-return I->second;
+  auto *COMD = OMD->getCanonicalDecl();
+  auto I = DirectMethodDefinitions.find(COMD);
+  llvm::Function *OldFn = nullptr, *Fn = nullptr;
 
-  SmallString<256> Name;
-  GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/true);
+  if (I != DirectMethodDefinitions.end()) {
+// Objective-C allows for the declaration and implementation types
+// to differ slightly.
+//
+// If we're being asked for the Function associated for a method
+// implementation, a previous value might have been cached
+// based on the type of the canonical declaration.
+//
+// If these do not match, then we'll replace this function with
+// a new one that has the proper type below.
+if (!OMD->getBody() || COMD->getReturnType() == OMD->getReturnType()) {
+  return I->second;
+}
+OldFn = I->second;
+  }
 
   CodeGenTypes &Types = CGM.getTypes();
   llvm::FunctionType *MethodTy =
 Types.GetFunctionType(Types.arrangeObjCMethodDeclaration(OMD));
-  llvm::Function *Method =
-  llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
- Name.str(), &CGM.getModule());
-  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), 
Method));
 
-  return Method;
+  if (OldFn) {
+Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
+"", &CGM.getModule());
+Fn->takeName(OldFn);
+OldFn->replaceAllUsesWith(
+llvm::ConstantExpr::getBitCast(Fn, OldFn->getType()));
+OldFn->eraseFromParent();
+
+// Replace the cached function in the map.
+I->second = Fn;
+  } else {
+SmallString<256> Name;
+GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/ true);
+
+Fn = llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
+Name.str(), &CGM.getModule());
+DirectMethodDefinitions.insert(std::make_pair(COMD, Fn));
+  }
+
+  return Fn;
 }
 
 void CGObjCCommonMac::GenerateDirectMethodPrologue(


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4032,22 +4032,50 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
-  if (I != DirectMethodDefinitions.end())
-return I->second;
+  auto *COMD = OMD->getCanonicalDecl();
+  auto I = DirectMethodDefinitions.find(COMD);
+  llvm::Function *OldFn = nullptr, *Fn = nullptr;
 
-  SmallString<256> Name;
-  GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/true);
+  if (I != DirectMethodDefinitions.end()

[PATCH] D73219: [objc_direct] do not add direct properties to the serialization array

[Pierre Habouzit via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
MadCoder marked an inline comment as done.
Closed by commit rG52311d0483ee: [objc_direct] do not add direct properties to 
the serialization array (authored by MadCoder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73219

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/test/CodeGenObjC/direct-properties.m


Index: clang/test/CodeGenObjC/direct-properties.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-properties.m
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - 
| FileCheck %s
+
+__attribute__((objc_root_class))
+@interface A
+@property(direct, readonly) int i;
+@end
+
+__attribute__((objc_root_class))
+@interface B
+@property(direct, readonly) int i;
+@property(readonly) int j;
+@end
+
+// CHECK-NOT: @"__OBJC_$_PROP_LIST_A"
+@implementation A
+@synthesize i = _i;
+@end
+
+// CHECK: @"_OBJC_$_PROP_LIST_B" = internal global { i32, i32, [1 x 
%struct._prop_t] } { i32 16, i32 1
+@implementation B
+@synthesize i = _i;
+@synthesize j = _j;
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -3291,6 +3291,8 @@
   for (auto *PD : ClassExt->properties()) {
 if (IsClassProperty != PD->isClassProperty())
   continue;
+if (PD->isDirectProperty())
+  continue;
 PropertySet.insert(PD->getIdentifier());
 Properties.push_back(PD);
   }
@@ -3302,6 +3304,8 @@
 // class extension.
 if (!PropertySet.insert(PD->getIdentifier()).second)
   continue;
+if (PD->isDirectProperty())
+  continue;
 Properties.push_back(PD);
   }
 
@@ -3327,8 +3331,6 @@
   values.addInt(ObjCTypes.IntTy, Properties.size());
   auto propertiesArray = values.beginArray(ObjCTypes.PropertyTy);
   for (auto PD : Properties) {
-if (PD->isDirectProperty())
-  continue;
 auto property = propertiesArray.beginStruct(ObjCTypes.PropertyTy);
 property.add(GetPropertyName(PD->getIdentifier()));
 property.add(GetPropertyTypeString(PD, Container));


Index: clang/test/CodeGenObjC/direct-properties.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-properties.m
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
+
+__attribute__((objc_root_class))
+@interface A
+@property(direct, readonly) int i;
+@end
+
+__attribute__((objc_root_class))
+@interface B
+@property(direct, readonly) int i;
+@property(readonly) int j;
+@end
+
+// CHECK-NOT: @"__OBJC_$_PROP_LIST_A"
+@implementation A
+@synthesize i = _i;
+@end
+
+// CHECK: @"_OBJC_$_PROP_LIST_B" = internal global { i32, i32, [1 x %struct._prop_t] } { i32 16, i32 1
+@implementation B
+@synthesize i = _i;
+@synthesize j = _j;
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -3291,6 +3291,8 @@
   for (auto *PD : ClassExt->properties()) {
 if (IsClassProperty != PD->isClassProperty())
   continue;
+if (PD->isDirectProperty())
+  continue;
 PropertySet.insert(PD->getIdentifier());
 Properties.push_back(PD);
   }
@@ -3302,6 +3304,8 @@
 // class extension.
 if (!PropertySet.insert(PD->getIdentifier()).second)
   continue;
+if (PD->isDirectProperty())
+  continue;
 Properties.push_back(PD);
   }
 
@@ -3327,8 +3331,6 @@
   values.addInt(ObjCTypes.IntTy, Properties.size());
   auto propertiesArray = values.beginArray(ObjCTypes.PropertyTy);
   for (auto PD : Properties) {
-if (PD->isDirectProperty())
-  continue;
 auto property = propertiesArray.beginStruct(ObjCTypes.PropertyTy);
 property.add(GetPropertyName(PD->getIdentifier()));
 property.add(GetPropertyTypeString(PD, Container));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72747: [objc_direct] Allow for direct messages be sent to `self` when it is a Class

[Pierre Habouzit via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7596d3c50c4b: [objc_direct] Allow for direct messages be 
sent to `self` when it is a Class (authored by MadCoder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72747

Files:
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/SemaObjC/method-direct-arc.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
@@ -89,7 +89,10 @@
 }
 - (void)otherRootDirect {
 }
++ (void)someRootDirectMethod { // expected-note {{direct method 
'someRootDirectMethod' declared here}}
+}
 + (void)otherClassRootDirect {
+  [self someRootDirectMethod]; // expected-error {{messaging a Class with a 
method that is possibly direct}}
 }
 - (void)rootExtensionDirect {
 }
Index: clang/test/SemaObjC/method-direct-arc.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct-arc.m
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -fobjc-arc -fsyntax-only -verify -Wselector-type-mismatch %s
+
+extern Class object_getClass(id);
+
+__attribute__((objc_root_class))
+@interface Root
+- (Class)class;
++ (void)directMethod __attribute__((objc_direct)); // expected-note {{direct 
method 'directMethod' declared here}}
++ (void)anotherDirectMethod __attribute__((objc_direct));
+@end
+
+@implementation Root
+- (Class)class
+{
+  return object_getClass(self);
+}
++ (void)directMethod {
+}
++ (void)anotherDirectMethod {
+  [self directMethod]; // this should not warn
+}
++ (void)regularMethod {
+  [self directMethod];// this should not warn
+  [self anotherDirectMethod]; // this should not warn
+}
+- (void)regularInstanceMethod {
+  [[self class] directMethod]; // expected-error {{messaging a Class with a 
method that is possibly direct}}
+}
+@end
+
+@interface Sub : Root
+@end
+
+@implementation Sub
++ (void)foo {
+  [self directMethod]; // this should not warn
+}
+@end
+
+__attribute__((objc_root_class))
+@interface Other
+@end
+
+@implementation Other
++ (void)bar {
+  [self directMethod]; // expected-error {{no known class method for selector 
'directMethod'}}
+}
+@end
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -3012,7 +3012,11 @@
   << Method->getDeclName();
 }
 
-if (ReceiverType->isObjCClassType() && !isImplicit) {
+// Under ARC, self can't be assigned, and doing a direct call to `self`
+// when it's a Class is hence safe.  For other cases, we can't trust `self`
+// is what we think it is, so we reject it.
+if (ReceiverType->isObjCClassType() && !isImplicit &&
+!(Receiver->isObjCSelfExpr() && getLangOpts().ObjCAutoRefCount)) {
   Diag(Receiver->getExprLoc(),
diag::err_messaging_class_with_direct_method);
   Diag(Method->getLocation(), diag::note_direct_method_declared_at)


Index: clang/test/SemaObjC/method-direct.m
===
--- clang/test/SemaObjC/method-direct.m
+++ clang/test/SemaObjC/method-direct.m
@@ -89,7 +89,10 @@
 }
 - (void)otherRootDirect {
 }
++ (void)someRootDirectMethod { // expected-note {{direct method 'someRootDirectMethod' declared here}}
+}
 + (void)otherClassRootDirect {
+  [self someRootDirectMethod]; // expected-error {{messaging a Class with a method that is possibly direct}}
 }
 - (void)rootExtensionDirect {
 }
Index: clang/test/SemaObjC/method-direct-arc.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct-arc.m
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -fobjc-arc -fsyntax-only -verify -Wselector-type-mismatch %s
+
+extern Class object_getClass(id);
+
+__attribute__((objc_root_class))
+@interface Root
+- (Class)class;
++ (void)directMethod __attribute__((objc_direct)); // expected-note {{direct method 'directMethod' declared here}}
++ (void)anotherDirectMethod __attribute__((objc_direct));
+@end
+
+@implementation Root
+- (Class)class
+{
+  return object_getClass(self);
+}
++ (void)directMethod {
+}
++ (void)anotherDirectMethod {
+  [self directMethod]; // this should not warn
+}
++ (void)regularMethod {
+  [self directMethod];// this should not warn
+  [self anotherDirectMethod]; // this should not warn
+}
+- (void)regularInstanceMethod {
+  [[self class] directMethod]; // expected-error {{messaging a Class with a method that is possibly direct}}
+}
+@end
+
+@interface Sub : Root
+@end
+
+@implementation Sub
++ (void)foo {
+  [self directMethod]; // this should not warn
+}
+@end
+
+__attribute__((objc_root_class))
+@interface Other
+@end
+
+@implementation Other
++ (void)bar {
+  [self direct

[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

In D73208#1836789 , @dexonsmith wrote:

> In D73208#1836722 , @MadCoder wrote:
>
> > In D73208#1836704 , @dexonsmith 
> > wrote:
> >
> > > In D73208#1835264 , @MadCoder 
> > > wrote:
> > >
> > > > In D73208#1835051 , 
> > > > @dexonsmith wrote:
> > > >
> > > > > Why isn't a similar dance needed for non-direct methods?
> > > >
> > > >
> > > > because non direct methods do not need an `llvm::Function` to be 
> > > > synthesized at the call-site. direct methods do, and they form one with 
> > > > the type of the declaration they see. Then that same `llvm::Function` 
> > > > is used when you CodeGen the Implementation, so if there's a mismatch, 
> > > > sadness ensues because the LLVM IR verifier will notice the discrepancy 
> > > > between the declared return type of the function and the actual types 
> > > > coming out of the `ret` codepaths.
> > > >
> > > > Regular obj-C methods use the _implementation_ types for the codegen 
> > > > (the declaration(s) aren't even consulted) and I want to stick at what 
> > > > obj-c does as much as I can.
> > > >
> > > > (as a data point: If you use obj-C types with C functions, the type of 
> > > > the first declaration seen is used instead).
> > >
> > >
> > > Okay, that makes sense to me.
> > >
> > > Another solution would be to change IRGen for the implementation: if the 
> > > declaration already exists (`getFunction`), do a bitcast + RAUW dance to 
> > > fix it up (and update the `DirectMethodDefinitions` table).  WDYT?
> >
> >
> > I didn't want to do that because that would mean that the type used for the 
> > implementation would depart from dynamic Objective-C methods, and it feels 
> > that it shouldn't. hence I took this option.
>
>
> I think we're talking across each other.  The idea is check the type when 
> generating the implementation; if it's not correct, you fix it (need to 
> update existing uses to bitcast).  So the type used for the implementation 
> would match dynamic methods.


ah I see, I don't know how to do that, I couldn't figure out how to fix the 
function declaration, if you want to give it a stab I'd love it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73208



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

In D73208#1836704 , @dexonsmith wrote:

> In D73208#1835264 , @MadCoder wrote:
>
> > In D73208#1835051 , @dexonsmith 
> > wrote:
> >
> > > Why isn't a similar dance needed for non-direct methods?
> >
> >
> > because non direct methods do not need an `llvm::Function` to be 
> > synthesized at the call-site. direct methods do, and they form one with the 
> > type of the declaration they see. Then that same `llvm::Function` is used 
> > when you CodeGen the Implementation, so if there's a mismatch, sadness 
> > ensues because the LLVM IR verifier will notice the discrepancy between the 
> > declared return type of the function and the actual types coming out of the 
> > `ret` codepaths.
> >
> > Regular obj-C methods use the _implementation_ types for the codegen (the 
> > declaration(s) aren't even consulted) and I want to stick at what obj-c 
> > does as much as I can.
> >
> > (as a data point: If you use obj-C types with C functions, the type of the 
> > first declaration seen is used instead).
>
>
> Okay, that makes sense to me.
>
> Another solution would be to change IRGen for the implementation: if the 
> declaration already exists (`getFunction`), do a bitcast + RAUW dance to fix 
> it up (and update the `DirectMethodDefinitions` table).  WDYT?


I didn't want to do that because that would mean that the type used for the 
implementation would depart from dynamic Objective-C methods, and it feels that 
it shouldn't. hence I took this option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73208



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73219: [objc_direct] do not add direct properties to the serialization array

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder marked 2 inline comments as done.
MadCoder added inline comments.



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:3316-3319
   else if (const ObjCCategoryDecl *CD = dyn_cast(OCD)) {
 for (const auto *P : CD->protocols())
   PushProtocolProperties(PropertySet, Properties, P, IsClassProperty);
   }

dexonsmith wrote:
> Do you need similar logic in `PushProtocolProperties`?
no: you can't have anything direct in a Protocol.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73219



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

In D73208#1835051 , @dexonsmith wrote:

> Why isn't a similar dance needed for non-direct methods?


because non direct methods do not need an `llvm::Function` to be synthesized at 
the call-site. direct methods do, and they form one with the type of the 
declaration they see. Then that same `llvm::Function` is used when you CodeGen 
the Implementation, so if there's a mismatch, sadness ensues because the LLVM 
IR verifier will notice the discrepancy between the declared return type of the 
function and the actual types coming out of the `ret` codepaths.

Regular obj-C methods use the _implementation_ types for the codegen (the 
declaration(s) aren't even consulted) and I want to stick at what obj-c does as 
much as I can.

(as a data point: If you use obj-C types with C functions, the type of the 
first declaration seen is used instead).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73208



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73219: [objc_direct] do not add direct properties to the serialization array

[Pierre Habouzit via Phabricator via cfe-commits]

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

If we do, then the property_list_t length is wrong
and class_getProperty gets very sad.

Radar-Id: rdar://problem/58804805


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73219

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/test/CodeGenObjC/direct-properties.m


Index: clang/test/CodeGenObjC/direct-properties.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-properties.m
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - 
| FileCheck %s
+
+__attribute__((objc_root_class))
+@interface A
+@property(direct, readonly) int i;
+@end
+
+__attribute__((objc_root_class))
+@interface B
+@property(direct, readonly) int i;
+@property(readonly) int j;
+@end
+
+// CHECK-NOT: @"__OBJC_$_PROP_LIST_A"
+@implementation A
+@synthesize i = _i;
+@end
+
+// CHECK: @"_OBJC_$_PROP_LIST_B" = internal global { i32, i32, [1 x 
%struct._prop_t] } { i32 16, i32 1
+@implementation B
+@synthesize i = _i;
+@synthesize j = _j;
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -3291,6 +3291,8 @@
   for (auto *PD : ClassExt->properties()) {
 if (IsClassProperty != PD->isClassProperty())
   continue;
+if (PD->isDirectProperty())
+  continue;
 PropertySet.insert(PD->getIdentifier());
 Properties.push_back(PD);
   }
@@ -3302,6 +3304,8 @@
 // class extension.
 if (!PropertySet.insert(PD->getIdentifier()).second)
   continue;
+if (PD->isDirectProperty())
+  continue;
 Properties.push_back(PD);
   }
 
@@ -3327,8 +3331,6 @@
   values.addInt(ObjCTypes.IntTy, Properties.size());
   auto propertiesArray = values.beginArray(ObjCTypes.PropertyTy);
   for (auto PD : Properties) {
-if (PD->isDirectProperty())
-  continue;
 auto property = propertiesArray.beginStruct(ObjCTypes.PropertyTy);
 property.add(GetPropertyName(PD->getIdentifier()));
 property.add(GetPropertyTypeString(PD, Container));


Index: clang/test/CodeGenObjC/direct-properties.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-properties.m
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
+
+__attribute__((objc_root_class))
+@interface A
+@property(direct, readonly) int i;
+@end
+
+__attribute__((objc_root_class))
+@interface B
+@property(direct, readonly) int i;
+@property(readonly) int j;
+@end
+
+// CHECK-NOT: @"__OBJC_$_PROP_LIST_A"
+@implementation A
+@synthesize i = _i;
+@end
+
+// CHECK: @"_OBJC_$_PROP_LIST_B" = internal global { i32, i32, [1 x %struct._prop_t] } { i32 16, i32 1
+@implementation B
+@synthesize i = _i;
+@synthesize j = _j;
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -3291,6 +3291,8 @@
   for (auto *PD : ClassExt->properties()) {
 if (IsClassProperty != PD->isClassProperty())
   continue;
+if (PD->isDirectProperty())
+  continue;
 PropertySet.insert(PD->getIdentifier());
 Properties.push_back(PD);
   }
@@ -3302,6 +3304,8 @@
 // class extension.
 if (!PropertySet.insert(PD->getIdentifier()).second)
   continue;
+if (PD->isDirectProperty())
+  continue;
 Properties.push_back(PD);
   }
 
@@ -3327,8 +3331,6 @@
   values.addInt(ObjCTypes.IntTy, Properties.size());
   auto propertiesArray = values.beginArray(ObjCTypes.PropertyTy);
   for (auto PD : Properties) {
-if (PD->isDirectProperty())
-  continue;
 auto property = propertiesArray.beginStruct(ObjCTypes.PropertyTy);
 property.add(GetPropertyName(PD->getIdentifier()));
 property.add(GetPropertyTypeString(PD, Container));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73208: [objc_direct] fix codegen for mismatched Decl/Impl return types

[Pierre Habouzit via Phabricator via cfe-commits]

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

For non direct methods, the codegen uses the type of the Implementation.
Because Objective-C rules allow some differences between the Declaration
and Implementation return types, when the Implementation is in this
translation unit, the type of the Implementation should be preferred to
emit the Function over the Declaration.

Radar-Id: rdar://problem/58797748


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D73208

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/test/CodeGenObjC/direct-method-ret-mismatch.m


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - 
| FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4029,10 +4029,28 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
+  auto *COMD = OMD->getCanonicalDecl();
+  auto I = DirectMethodDefinitions.find(COMD);
   if (I != DirectMethodDefinitions.end())
 return I->second;
 
+  // If this translation unit sees the implementation, we need to use
+  // it to generate the llvm::Function below. It is important to do so
+  // because Objective-C allows for return types to be somewhat mismatched
+  // and the Body of the function will expect to be generated with
+  // the type of its implementation.
+  if (!OMD->getBody()) {
+if (auto *OI = dyn_cast(CD)) {
+  if (auto *Impl = OI->getImplementation())
+if (auto *M = Impl->getMethod(OMD->getSelector(), 
OMD->isInstanceMethod()))
+  OMD = M;
+} else if (auto *CI = dyn_cast(CD)) {
+  if (auto *Impl = OI->getImplementation())
+if (auto *M = Impl->getMethod(OMD->getSelector(), 
OMD->isInstanceMethod()))
+  OMD = M;
+}
+  }
+
   SmallString<256> Name;
   GetNameForMethod(OMD, CD, Name, /*ignoreCategoryNamespace*/true);
 
@@ -4042,7 +4060,7 @@
   llvm::Function *Method =
   llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
  Name.str(), &CGM.getModule());
-  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), 
Method));
+  DirectMethodDefinitions.insert(std::make_pair(COMD, Method));
 
   return Method;
 }


Index: clang/test/CodeGenObjC/direct-method-ret-mismatch.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/direct-method-ret-mismatch.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - | FileCheck %s
+
+__attribute__((objc_root_class))
+@interface Root
+- (Root *)method __attribute__((objc_direct));
+@end
+
+@implementation Root
+// CHECK-LABEL: define internal i8* @"\01-[Root something]"(
+- (id)something {
+  // CHECK: %{{[^ ]*}} = call {{.*}} @"\01-[Root method]"
+  return [self method];
+}
+
+// CHECK-LABEL: define hidden i8* @"\01-[Root method]"(
+- (id)method {
+  return self;
+}
+@end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4029,10 +4029,28 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
+  auto *COMD = OMD->getCanonicalDecl();
+  auto I = DirectMethodDefinitions.find(COMD);
   if (I != DirectMethodDefinitions.end())
 return I->second;
 
+  // If this translation unit sees the implementation, we need to use
+  // it to generate the llvm::Function below. It is important to do so
+  // because Objective-C allows for return types to be somewhat mismatched
+  // and the Body of the function will expect to be generated with
+  // the type of its implementation.
+  if (!OMD->getBody()) {
+if (auto *OI = dyn_cast(CD)) {
+  if (auto *Impl = OI->getImplementation())
+if (auto *M = Impl->getMethod(OMD->getSelector(),

[PATCH] D72747: [objc_direct] Allow for direct messages be sent to `self` when it is a Class

[Pierre Habouzit via Phabricator via cfe-commits]

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

with the test for real


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

https://reviews.llvm.org/D72747

Files:
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/SemaObjC/method-direct-arc.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
@@ -89,7 +89,10 @@
 }
 - (void)otherRootDirect {
 }
++ (void)someRootDirectMethod { // expected-note {{direct method 
'someRootDirectMethod' declared here}}
+}
 + (void)otherClassRootDirect {
+  [self someRootDirectMethod]; // expected-error {{messaging a Class with a 
method that is possibly direct}}
 }
 - (void)rootExtensionDirect {
 }
Index: clang/test/SemaObjC/method-direct-arc.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct-arc.m
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -fobjc-arc -fsyntax-only -verify -Wselector-type-mismatch %s
+
+extern Class object_getClass(id);
+
+__attribute__((objc_root_class))
+@interface Root
+- (Class)class;
++ (void)directMethod __attribute__((objc_direct)); // expected-note {{direct 
method 'directMethod' declared here}}
++ (void)anotherDirectMethod __attribute__((objc_direct));
+@end
+
+@implementation Root
+- (Class)class
+{
+  return object_getClass(self);
+}
++ (void)directMethod {
+}
++ (void)anotherDirectMethod {
+  [self directMethod]; // this should not warn
+}
++ (void)regularMethod {
+  [self directMethod];// this should not warn
+  [self anotherDirectMethod]; // this should not warn
+}
+- (void)regularInstanceMethod {
+  [[self class] directMethod]; // expected-error {{messaging a Class with a 
method that is possibly direct}}
+}
+@end
+
+@interface Sub : Root
+@end
+
+@implementation Sub
++ (void)foo {
+  [self directMethod]; // this should not warn
+}
+@end
+
+__attribute__((objc_root_class))
+@interface Other
+@end
+
+@implementation Other
++ (void)bar {
+  [self directMethod]; // expected-error {{no known class method for selector 
'directMethod'}}
+}
+@end
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -3012,7 +3012,11 @@
   << Method->getDeclName();
 }
 
-if (ReceiverType->isObjCClassType() && !isImplicit) {
+// Under ARC, self can't be assigned, and doing a direct call to `self`
+// when it's a Class is hence safe.  For other cases, we can't trust `self`
+// is what we think it is, so we reject it.
+if (ReceiverType->isObjCClassType() && !isImplicit &&
+!(Receiver->isObjCSelfExpr() && getLangOpts().ObjCAutoRefCount)) {
   Diag(Receiver->getExprLoc(),
diag::err_messaging_class_with_direct_method);
   Diag(Method->getLocation(), diag::note_direct_method_declared_at)


Index: clang/test/SemaObjC/method-direct.m
===
--- clang/test/SemaObjC/method-direct.m
+++ clang/test/SemaObjC/method-direct.m
@@ -89,7 +89,10 @@
 }
 - (void)otherRootDirect {
 }
++ (void)someRootDirectMethod { // expected-note {{direct method 'someRootDirectMethod' declared here}}
+}
 + (void)otherClassRootDirect {
+  [self someRootDirectMethod]; // expected-error {{messaging a Class with a method that is possibly direct}}
 }
 - (void)rootExtensionDirect {
 }
Index: clang/test/SemaObjC/method-direct-arc.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct-arc.m
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -fobjc-arc -fsyntax-only -verify -Wselector-type-mismatch %s
+
+extern Class object_getClass(id);
+
+__attribute__((objc_root_class))
+@interface Root
+- (Class)class;
++ (void)directMethod __attribute__((objc_direct)); // expected-note {{direct method 'directMethod' declared here}}
++ (void)anotherDirectMethod __attribute__((objc_direct));
+@end
+
+@implementation Root
+- (Class)class
+{
+  return object_getClass(self);
+}
++ (void)directMethod {
+}
++ (void)anotherDirectMethod {
+  [self directMethod]; // this should not warn
+}
++ (void)regularMethod {
+  [self directMethod];// this should not warn
+  [self anotherDirectMethod]; // this should not warn
+}
+- (void)regularInstanceMethod {
+  [[self class] directMethod]; // expected-error {{messaging a Class with a method that is possibly direct}}
+}
+@end
+
+@interface Sub : Root
+@end
+
+@implementation Sub
++ (void)foo {
+  [self directMethod]; // this should not warn
+}
+@end
+
+__attribute__((objc_root_class))
+@interface Other
+@end
+
+@implementation Other
++ (void)bar {
+  [self directMethod]; // expected-error {{no known class method for selector 'directMethod'}}
+}
+@end
Index: clang/lib/Sema/SemaExprObjC.cpp
=

[PATCH] D72747: [objc_direct] Allow for direct messages be sent to `self` when it is a Class

[Pierre Habouzit via Phabricator via cfe-commits]

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

adding the non ARC test I forgot to add


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

https://reviews.llvm.org/D72747

Files:
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/SemaObjC/method-direct-arc.m


Index: clang/test/SemaObjC/method-direct-arc.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct-arc.m
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -fobjc-arc -fsyntax-only -verify -Wselector-type-mismatch %s
+
+extern Class object_getClass(id);
+
+__attribute__((objc_root_class))
+@interface Root
+- (Class)class;
++ (void)directMethod __attribute__((objc_direct)); // expected-note {{direct 
method 'directMethod' declared here}}
++ (void)anotherDirectMethod __attribute__((objc_direct));
+@end
+
+@implementation Root
+- (Class)class
+{
+  return object_getClass(self);
+}
++ (void)directMethod {
+}
++ (void)anotherDirectMethod {
+  [self directMethod]; // this should not warn
+}
++ (void)regularMethod {
+  [self directMethod];// this should not warn
+  [self anotherDirectMethod]; // this should not warn
+}
+- (void)regularInstanceMethod {
+  [[self class] directMethod]; // expected-error {{messaging a Class with a 
method that is possibly direct}}
+}
+@end
+
+@interface Sub : Root
+@end
+
+@implementation Sub
++ (void)foo {
+  [self directMethod]; // this should not warn
+}
+@end
+
+__attribute__((objc_root_class))
+@interface Other
+@end
+
+@implementation Other
++ (void)bar {
+  [self directMethod]; // expected-error {{no known class method for selector 
'directMethod'}}
+}
+@end
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -3012,7 +3012,11 @@
   << Method->getDeclName();
 }
 
-if (ReceiverType->isObjCClassType() && !isImplicit) {
+// Under ARC, self can't be assigned, and doing a direct call to `self`
+// when it's a Class is hence safe.  For other cases, we can't trust `self`
+// is what we think it is, so we reject it.
+if (ReceiverType->isObjCClassType() && !isImplicit &&
+!(Receiver->isObjCSelfExpr() && getLangOpts().ObjCAutoRefCount)) {
   Diag(Receiver->getExprLoc(),
diag::err_messaging_class_with_direct_method);
   Diag(Method->getLocation(), diag::note_direct_method_declared_at)


Index: clang/test/SemaObjC/method-direct-arc.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct-arc.m
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -fobjc-arc -fsyntax-only -verify -Wselector-type-mismatch %s
+
+extern Class object_getClass(id);
+
+__attribute__((objc_root_class))
+@interface Root
+- (Class)class;
++ (void)directMethod __attribute__((objc_direct)); // expected-note {{direct method 'directMethod' declared here}}
++ (void)anotherDirectMethod __attribute__((objc_direct));
+@end
+
+@implementation Root
+- (Class)class
+{
+  return object_getClass(self);
+}
++ (void)directMethod {
+}
++ (void)anotherDirectMethod {
+  [self directMethod]; // this should not warn
+}
++ (void)regularMethod {
+  [self directMethod];// this should not warn
+  [self anotherDirectMethod]; // this should not warn
+}
+- (void)regularInstanceMethod {
+  [[self class] directMethod]; // expected-error {{messaging a Class with a method that is possibly direct}}
+}
+@end
+
+@interface Sub : Root
+@end
+
+@implementation Sub
++ (void)foo {
+  [self directMethod]; // this should not warn
+}
+@end
+
+__attribute__((objc_root_class))
+@interface Other
+@end
+
+@implementation Other
++ (void)bar {
+  [self directMethod]; // expected-error {{no known class method for selector 'directMethod'}}
+}
+@end
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -3012,7 +3012,11 @@
   << Method->getDeclName();
 }
 
-if (ReceiverType->isObjCClassType() && !isImplicit) {
+// Under ARC, self can't be assigned, and doing a direct call to `self`
+// when it's a Class is hence safe.  For other cases, we can't trust `self`
+// is what we think it is, so we reject it.
+if (ReceiverType->isObjCClassType() && !isImplicit &&
+!(Receiver->isObjCSelfExpr() && getLangOpts().ObjCAutoRefCount)) {
   Diag(Receiver->getExprLoc(),
diag::err_messaging_class_with_direct_method);
   Diag(Method->getLocation(), diag::note_direct_method_declared_at)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72747: [objc_direct] Allow for direct messages be sent to `self` when it is a Class

[Pierre Habouzit via Phabricator via cfe-commits]

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

fixed @ahatanak feedback


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

https://reviews.llvm.org/D72747

Files:
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/SemaObjC/method-direct-arc.m


Index: clang/test/SemaObjC/method-direct-arc.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct-arc.m
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -fobjc-arc -fsyntax-only -verify -Wselector-type-mismatch %s
+
+extern Class object_getClass(id);
+
+__attribute__((objc_root_class))
+@interface Root
+- (Class)class;
++ (void)directMethod __attribute__((objc_direct)); // expected-note {{direct 
method 'directMethod' declared here}}
++ (void)anotherDirectMethod __attribute__((objc_direct));
+@end
+
+@implementation Root
+- (Class)class
+{
+  return object_getClass(self);
+}
++ (void)directMethod {
+}
++ (void)anotherDirectMethod {
+  [self directMethod]; // this should not warn
+}
++ (void)regularMethod {
+  [self directMethod];// this should not warn
+  [self anotherDirectMethod]; // this should not warn
+}
+- (void)regularInstanceMethod {
+  [[self class] directMethod]; // expected-error {{messaging a Class with a 
method that is possibly direct}}
+}
+@end
+
+@interface Sub : Root
+@end
+
+@implementation Sub
++ (void)foo {
+  [self directMethod]; // this should not warn
+}
+@end
+
+__attribute__((objc_root_class))
+@interface Other
+@end
+
+@implementation Other
++ (void)bar {
+  [self directMethod]; // expected-error {{no known class method for selector 
'directMethod'}}
+}
+@end
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -3012,7 +3012,11 @@
   << Method->getDeclName();
 }
 
-if (ReceiverType->isObjCClassType() && !isImplicit) {
+// Under ARC, self can't be assigned, and doing a direct call to `self`
+// when it's a Class is hence safe.  For other cases, we can't trust `self`
+// is what we think it is, so we reject it.
+if (ReceiverType->isObjCClassType() && !isImplicit &&
+!(Receiver->isObjCSelfExpr() && getLangOpts().ObjCAutoRefCount)) {
   Diag(Receiver->getExprLoc(),
diag::err_messaging_class_with_direct_method);
   Diag(Method->getLocation(), diag::note_direct_method_declared_at)


Index: clang/test/SemaObjC/method-direct-arc.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct-arc.m
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -fobjc-arc -fsyntax-only -verify -Wselector-type-mismatch %s
+
+extern Class object_getClass(id);
+
+__attribute__((objc_root_class))
+@interface Root
+- (Class)class;
++ (void)directMethod __attribute__((objc_direct)); // expected-note {{direct method 'directMethod' declared here}}
++ (void)anotherDirectMethod __attribute__((objc_direct));
+@end
+
+@implementation Root
+- (Class)class
+{
+  return object_getClass(self);
+}
++ (void)directMethod {
+}
++ (void)anotherDirectMethod {
+  [self directMethod]; // this should not warn
+}
++ (void)regularMethod {
+  [self directMethod];// this should not warn
+  [self anotherDirectMethod]; // this should not warn
+}
+- (void)regularInstanceMethod {
+  [[self class] directMethod]; // expected-error {{messaging a Class with a method that is possibly direct}}
+}
+@end
+
+@interface Sub : Root
+@end
+
+@implementation Sub
++ (void)foo {
+  [self directMethod]; // this should not warn
+}
+@end
+
+__attribute__((objc_root_class))
+@interface Other
+@end
+
+@implementation Other
++ (void)bar {
+  [self directMethod]; // expected-error {{no known class method for selector 'directMethod'}}
+}
+@end
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -3012,7 +3012,11 @@
   << Method->getDeclName();
 }
 
-if (ReceiverType->isObjCClassType() && !isImplicit) {
+// Under ARC, self can't be assigned, and doing a direct call to `self`
+// when it's a Class is hence safe.  For other cases, we can't trust `self`
+// is what we think it is, so we reject it.
+if (ReceiverType->isObjCClassType() && !isImplicit &&
+!(Receiver->isObjCSelfExpr() && getLangOpts().ObjCAutoRefCount)) {
   Diag(Receiver->getExprLoc(),
diag::err_messaging_class_with_direct_method);
   Diag(Method->getLocation(), diag::note_direct_method_declared_at)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72747: [objc_direct] Allow for direct messages be sent to `self` when it is a Class

[Pierre Habouzit via Phabricator via cfe-commits]

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

Added some more tests to make sure it works along inheritance chains as expected


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

https://reviews.llvm.org/D72747

Files:
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/SemaObjC/method-direct-arc.m


Index: clang/test/SemaObjC/method-direct-arc.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct-arc.m
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -fobjc-arc -fsyntax-only -verify -Wselector-type-mismatch %s
+
+extern Class object_getClass(id);
+
+__attribute__((objc_root_class))
+@interface Root
+- (Class)class;
++ (void)directMethod __attribute__((objc_direct)); // expected-note {{direct 
method 'directMethod' declared here}}
++ (void)anotherDirectMethod __attribute__((objc_direct));
+@end
+
+@implementation Root
+- (Class)class
+{
+  return object_getClass(self);
+}
++ (void)directMethod {
+}
++ (void)anotherDirectMethod {
+  [self directMethod]; // this should not warn
+}
++ (void)regularMethod {
+  [self directMethod];// this should not warn
+  [self anotherDirectMethod]; // this should not warn
+}
+- (void)regularInstanceMethod {
+  [[self class] directMethod]; // expected-error {{messaging a Class with a 
method that is possibly direct}}
+}
+@end
+
+@interface Sub : Root
+@end
+
+@implementation Sub
++ (void)foo {
+  [self directMethod]; // this should not warn
+}
+@end
+
+__attribute__((objc_root_class))
+@interface Other
+@end
+
+@implementation Other
++ (void)bar {
+  [self directMethod]; // expected-error {{no known class method for selector 
'directMethod'}}
+}
+@end
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -3012,7 +3012,11 @@
   << Method->getDeclName();
 }
 
-if (ReceiverType->isObjCClassType() && !isImplicit) {
+// Under ARC, self can't be assigned, and doing a direct call to `self`
+// when it's a Class is hence safe.  For other cases, we can't trust `self`
+// is what we think it is, so we reject it.
+if (ReceiverType->isObjCClassType() && !isImplicit &&
+(!Receiver->isObjCSelfExpr() || !getLangOpts().ObjCAutoRefCount)) {
   Diag(Receiver->getExprLoc(),
diag::err_messaging_class_with_direct_method);
   Diag(Method->getLocation(), diag::note_direct_method_declared_at)


Index: clang/test/SemaObjC/method-direct-arc.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct-arc.m
@@ -0,0 +1,48 @@
+// RUN: %clang_cc1 -fobjc-arc -fsyntax-only -verify -Wselector-type-mismatch %s
+
+extern Class object_getClass(id);
+
+__attribute__((objc_root_class))
+@interface Root
+- (Class)class;
++ (void)directMethod __attribute__((objc_direct)); // expected-note {{direct method 'directMethod' declared here}}
++ (void)anotherDirectMethod __attribute__((objc_direct));
+@end
+
+@implementation Root
+- (Class)class
+{
+  return object_getClass(self);
+}
++ (void)directMethod {
+}
++ (void)anotherDirectMethod {
+  [self directMethod]; // this should not warn
+}
++ (void)regularMethod {
+  [self directMethod];// this should not warn
+  [self anotherDirectMethod]; // this should not warn
+}
+- (void)regularInstanceMethod {
+  [[self class] directMethod]; // expected-error {{messaging a Class with a method that is possibly direct}}
+}
+@end
+
+@interface Sub : Root
+@end
+
+@implementation Sub
++ (void)foo {
+  [self directMethod]; // this should not warn
+}
+@end
+
+__attribute__((objc_root_class))
+@interface Other
+@end
+
+@implementation Other
++ (void)bar {
+  [self directMethod]; // expected-error {{no known class method for selector 'directMethod'}}
+}
+@end
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -3012,7 +3012,11 @@
   << Method->getDeclName();
 }
 
-if (ReceiverType->isObjCClassType() && !isImplicit) {
+// Under ARC, self can't be assigned, and doing a direct call to `self`
+// when it's a Class is hence safe.  For other cases, we can't trust `self`
+// is what we think it is, so we reject it.
+if (ReceiverType->isObjCClassType() && !isImplicit &&
+(!Receiver->isObjCSelfExpr() || !getLangOpts().ObjCAutoRefCount)) {
   Diag(Receiver->getExprLoc(),
diag::err_messaging_class_with_direct_method);
   Diag(Method->getLocation(), diag::note_direct_method_declared_at)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72747: [objc_direct] Allow for direct messages be sent to `self` when it is a Class

[Pierre Habouzit via Phabricator via cfe-commits]

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

Sending a message to `self` when it is const and within a class method is safe 
because we know that `self` is the Class itself.

We can only relax this error in ARC.

Radar-Id: rdar://problem/58581965


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72747

Files:
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/SemaObjC/method-direct-arc.m


Index: clang/test/SemaObjC/method-direct-arc.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct-arc.m
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fobjc-arc -fsyntax-only -verify -Wselector-type-mismatch %s
+
+extern Class object_getClass(id);
+
+__attribute__((objc_root_class))
+@interface Root
+- (Class)class;
++ (void)directMethod1 __attribute__((objc_direct)); // expected-note {{direct 
method 'directMethod1' declared here}}
++ (void)directMethod2 __attribute__((objc_direct));
+@end
+
+@implementation Root
+- (Class)class
+{
+  return object_getClass(self);
+}
++ (void)directMethod1 {
+}
++ (void)directMethod2 {
+  [self directMethod1]; // this should not warn
+}
++ (void)regularMethod {
+  [self directMethod1]; // this should not warn
+  [self directMethod2]; // this should not warn
+}
+- (void)regularInstanceMethod {
+  [[self class] directMethod1]; // expected-error {{messaging a Class with a 
method that is possibly direct}}
+}
+@end
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -3012,7 +3012,11 @@
   << Method->getDeclName();
 }
 
-if (ReceiverType->isObjCClassType() && !isImplicit) {
+   // Under ARC, self can't be assigned, and doing a direct call to `self`
+   // when it's a Class is hence safe.  For other cases, we can't trust 
`self`
+   // is what we think it is, so we reject it.
+if (ReceiverType->isObjCClassType() && !isImplicit &&
+(!Receiver->isObjCSelfExpr() || !getLangOpts().ObjCAutoRefCount)) {
   Diag(Receiver->getExprLoc(),
diag::err_messaging_class_with_direct_method);
   Diag(Method->getLocation(), diag::note_direct_method_declared_at)


Index: clang/test/SemaObjC/method-direct-arc.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct-arc.m
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -fobjc-arc -fsyntax-only -verify -Wselector-type-mismatch %s
+
+extern Class object_getClass(id);
+
+__attribute__((objc_root_class))
+@interface Root
+- (Class)class;
++ (void)directMethod1 __attribute__((objc_direct)); // expected-note {{direct method 'directMethod1' declared here}}
++ (void)directMethod2 __attribute__((objc_direct));
+@end
+
+@implementation Root
+- (Class)class
+{
+  return object_getClass(self);
+}
++ (void)directMethod1 {
+}
++ (void)directMethod2 {
+  [self directMethod1]; // this should not warn
+}
++ (void)regularMethod {
+  [self directMethod1]; // this should not warn
+  [self directMethod2]; // this should not warn
+}
+- (void)regularInstanceMethod {
+  [[self class] directMethod1]; // expected-error {{messaging a Class with a method that is possibly direct}}
+}
+@end
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -3012,7 +3012,11 @@
   << Method->getDeclName();
 }
 
-if (ReceiverType->isObjCClassType() && !isImplicit) {
+	// Under ARC, self can't be assigned, and doing a direct call to `self`
+	// when it's a Class is hence safe.  For other cases, we can't trust `self`
+	// is what we think it is, so we reject it.
+if (ReceiverType->isObjCClassType() && !isImplicit &&
+(!Receiver->isObjCSelfExpr() || !getLangOpts().ObjCAutoRefCount)) {
   Diag(Receiver->getExprLoc(),
diag::err_messaging_class_with_direct_method);
   Diag(Method->getLocation(), diag::note_direct_method_declared_at)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71682: Relax the rules around objc_alloc and objc_alloc_init optimizations.

[Pierre Habouzit via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd18fbfc09720: Relax the rules around objc_alloc and 
objc_alloc_init optimizations. (authored by MadCoder).

Changed prior to commit:
  https://reviews.llvm.org/D71682?vs=234644&id=238161#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71682

Files:
  clang/lib/CodeGen/CGObjC.cpp
  clang/test/CodeGenObjC/objc-alloc-init.m

Index: clang/test/CodeGenObjC/objc-alloc-init.m
===
--- clang/test/CodeGenObjC/objc-alloc-init.m
+++ clang/test/CodeGenObjC/objc-alloc-init.m
@@ -22,21 +22,30 @@
 }
 
 @interface Y : X
++(Class)class;
 +(void)meth;
 -(void)instanceMeth;
 @end
 
 @implementation Y
++(Class)class {
+  return self;
+}
 +(void)meth {
   [[self alloc] init];
   // OPTIMIZED: call i8* @objc_alloc_init(
   // NOT_OPTIMIZED: call i8* @objc_alloc(
 }
++ (void)meth2 {
+  [[[self class] alloc] init];
+  // OPTIMIZED: call i8* @objc_alloc_init(
+  // NOT_OPTIMIZED: call i8* @objc_alloc(
+}
 -(void)instanceMeth {
   // EITHER-NOT: call i8* @objc_alloc
   // EITHER: call {{.*}} @objc_msgSend
   // EITHER: call {{.*}} @objc_msgSend
-  [[self alloc] init];
+  [[(id)self alloc] init];
 }
 @end
 
Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -461,38 +461,39 @@
   Sel.getNameForSlot(0) != "init")
 return None;
 
-  // Okay, this is '[receiver init]', check if 'receiver' is '[cls alloc]' or
-  // we are in an ObjC class method and 'receiver' is '[self alloc]'.
+  // Okay, this is '[receiver init]', check if 'receiver' is '[cls alloc]'
+  // with 'cls' a Class.
   auto *SubOME =
   dyn_cast(OME->getInstanceReceiver()->IgnoreParenCasts());
   if (!SubOME)
 return None;
   Selector SubSel = SubOME->getSelector();
 
-  // Check if we are in an ObjC class method and the receiver expression is
-  // 'self'.
-  const Expr *SelfInClassMethod = nullptr;
-  if (const auto *CurMD = dyn_cast_or_null(CGF.CurFuncDecl))
-if (CurMD->isClassMethod())
-  if ((SelfInClassMethod = SubOME->getInstanceReceiver()))
-if (!SelfInClassMethod->isObjCSelfExpr())
-  SelfInClassMethod = nullptr;
-
-  if ((SubOME->getReceiverKind() != ObjCMessageExpr::Class &&
-   !SelfInClassMethod) || !SubOME->getType()->isObjCObjectPointerType() ||
+  if (!SubOME->getType()->isObjCObjectPointerType() ||
   !SubSel.isUnarySelector() || SubSel.getNameForSlot(0) != "alloc")
 return None;
 
-  llvm::Value *Receiver;
-  if (SelfInClassMethod) {
-Receiver = CGF.EmitScalarExpr(SelfInClassMethod);
-  } else {
+  llvm::Value *Receiver = nullptr;
+  switch (SubOME->getReceiverKind()) {
+  case ObjCMessageExpr::Instance:
+if (!SubOME->getInstanceReceiver()->getType()->isObjCClassType())
+  return None;
+Receiver = CGF.EmitScalarExpr(SubOME->getInstanceReceiver());
+break;
+
+  case ObjCMessageExpr::Class: {
 QualType ReceiverType = SubOME->getClassReceiver();
 const ObjCObjectType *ObjTy = ReceiverType->castAs();
 const ObjCInterfaceDecl *ID = ObjTy->getInterface();
 assert(ID && "null interface should be impossible here");
 Receiver = CGF.CGM.getObjCRuntime().GetClass(CGF, ID);
+break;
+  }
+  case ObjCMessageExpr::SuperInstance:
+  case ObjCMessageExpr::SuperClass:
+return None;
   }
+
   return CGF.EmitObjCAllocInit(Receiver, CGF.ConvertType(OME->getType()));
 }
 
@@ -540,10 +541,7 @@
   switch (E->getReceiverKind()) {
   case ObjCMessageExpr::Instance:
 ReceiverType = E->getInstanceReceiver()->getType();
-if (auto *OMD = dyn_cast_or_null(CurFuncDecl))
-  if (OMD->isClassMethod())
-if (E->getInstanceReceiver()->isObjCSelfExpr())
-  isClassMessage = true;
+isClassMessage = ReceiverType->isObjCClassType();
 if (retainSelf) {
   TryEmitResult ter = tryEmitARCRetainScalarExpr(*this,
E->getInstanceReceiver());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71682: Relax the rules around objc_alloc and objc_alloc_init optimizations.

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder marked an inline comment as done.
MadCoder added inline comments.



Comment at: clang/test/CodeGenObjC/objc-alloc-init.m:30
 
 @implementation Y
++(Class)class {

ahatanak wrote:
> Can you add a test case for `[[self class] alloc]` to test the code in 
> `tryGenerateSpecializedMessageSend`?
isn't it what meth2 is doing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71682



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71694: [objc_direct] Tigthen checks for direct methods

[Pierre Habouzit via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG42f9d0c0bee3: [objc_direct] Tigthen checks for direct 
methods (authored by MadCoder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71694

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/DeclObjC.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/CodeGenObjC/direct-method.m
  clang/test/SemaObjC/method-direct-one-definition.m

Index: clang/test/SemaObjC/method-direct-one-definition.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct-one-definition.m
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-protocol-method-implementation %s
+
+__attribute__((objc_root_class))
+@interface A
+@end
+
+@interface A (Cat)
+- (void)A_Cat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@implementation A
+- (void)A_Cat { // expected-error {{direct method was declared in a category but is implemented in the primary interface}}
+}
+@end
+
+__attribute__((objc_root_class))
+@interface B
+- (void)B_primary __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface B ()
+- (void)B_extension __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface B (Cat)
+- (void)B_Cat __attribute__((objc_direct));
+@end
+
+@interface B (OtherCat)
+- (void)B_OtherCat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@implementation B (Cat)
+- (void)B_primary { // expected-error {{direct method was declared in the primary interface but is implemented in a category}}
+}
+- (void)B_extension { // expected-error {{direct method was declared in an extension but is implemented in a different category}}
+}
+- (void)B_Cat {
+}
+- (void)B_OtherCat { // expected-error {{direct method was declared in a category but is implemented in a different category}}
+}
+@end
+
+__attribute__((objc_root_class))
+@interface C
+- (void)C1 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+- (void)C2;  // expected-note {{previous declaration is here}}
+@end
+
+@interface C (Cat)
+- (void)C1;  // expected-error {{method declaration conflicts with previous direct declaration of method 'C1'}}
+- (void)C2 __attribute__((objc_direct)); // expected-error {{direct method declaration conflicts with previous declaration of method 'C2'}}
+@end
Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -172,10 +172,19 @@
 @interface Foo ()
 @property(nonatomic, readwrite) int getDirect_setDynamic;
 @property(nonatomic, readwrite, direct) int getDynamic_setDirect;
+- (int)directMethodInExtension __attribute__((objc_direct));
+@end
+
+@interface Foo (Cat)
+- (int)directMethodInCategory __attribute__((objc_direct));
 @end
 
 __attribute__((objc_direct_members))
 @implementation Foo
+// CHECK-LABEL: define hidden i32 @"\01-[Foo directMethodInExtension]"(
+- (int)directMethodInExtension {
+  return 42;
+}
 // CHECK-LABEL: define hidden i32 @"\01-[Foo getDirect_setDynamic]"(
 // CHECK-LABEL: define internal void @"\01-[Foo setGetDirect_setDynamic:]"(
 // CHECK-LABEL: define internal i32 @"\01-[Foo getDynamic_setDirect]"(
@@ -183,6 +192,17 @@
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
 
+@implementation Foo (Cat)
+// CHECK-LABEL: define hidden i32 @"\01-[Foo directMethodInCategory]"(
+- (int)directMethodInCategory {
+  return 42;
+}
+// CHECK-LABEL: define hidden i32 @"\01-[Foo directMethodInCategoryNoDecl]"(
+- (int)directMethodInCategoryNoDecl __attribute__((objc_direct)) {
+  return 42;
+}
+@end
+
 int useRoot(Root *r) {
   // CHECK-LABEL: define i32 @useRoot
   // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root getInt]"
@@ -195,8 +215,14 @@
   // CHECK-LABEL: define i32 @useFoo
   // CHECK: call void bitcast {{.*}} @"\01-[Foo setGetDynamic_setDirect:]"
   // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo getDirect_setDynamic]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo directMethodInExtension]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo directMethodInCategory]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo directMethodInCategoryNoDecl]"
   [f setGetDynamic_setDirect:1];
-  return [f getDirect_setDynamic];
+  return [f getDirect_setDynamic] +
+ [f directMethodInExtension] +
+ [f directMethodInCategory] +
+ [f directMethodInCategoryNoDecl];
 }
 
 __attribute__((objc_root_class))
Index: clang/lib/Sema/SemaDeclObjC.cpp
==

[PATCH] D71694: [objc_direct] Tigthen checks for direct methods

[Pierre Habouzit via Phabricator via cfe-commits]

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

Removed the category from the direct method name mangling as discussed with 
@aprantl


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

https://reviews.llvm.org/D71694

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/DeclObjC.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/CodeGenObjC/direct-method.m
  clang/test/SemaObjC/method-direct-one-definition.m

Index: clang/test/SemaObjC/method-direct-one-definition.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct-one-definition.m
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-protocol-method-implementation %s
+
+__attribute__((objc_root_class))
+@interface A
+@end
+
+@interface A (Cat)
+- (void)A_Cat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@implementation A
+- (void)A_Cat { // expected-error {{direct method was declared in a category but is implemented in the primary interface}}
+}
+@end
+
+__attribute__((objc_root_class))
+@interface B
+- (void)B_primary __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface B ()
+- (void)B_extension __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface B (Cat)
+- (void)B_Cat __attribute__((objc_direct));
+@end
+
+@interface B (OtherCat)
+- (void)B_OtherCat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@implementation B (Cat)
+- (void)B_primary { // expected-error {{direct method was declared in the primary interface but is implemented in a category}}
+}
+- (void)B_extension { // expected-error {{direct method was declared in an extension but is implemented in a different category}}
+}
+- (void)B_Cat {
+}
+- (void)B_OtherCat { // expected-error {{direct method was declared in a category but is implemented in a different category}}
+}
+@end
+
+__attribute__((objc_root_class))
+@interface C
+- (void)C1 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+- (void)C2;  // expected-note {{previous declaration is here}}
+@end
+
+@interface C (Cat)
+- (void)C1;  // expected-error {{method declaration conflicts with previous direct declaration of method 'C1'}}
+- (void)C2 __attribute__((objc_direct)); // expected-error {{direct method declaration conflicts with previous declaration of method 'C2'}}
+@end
Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -172,10 +172,19 @@
 @interface Foo ()
 @property(nonatomic, readwrite) int getDirect_setDynamic;
 @property(nonatomic, readwrite, direct) int getDynamic_setDirect;
+- (int)directMethodInExtension __attribute__((objc_direct));
+@end
+
+@interface Foo (Cat)
+- (int)directMethodInCategory __attribute__((objc_direct));
 @end
 
 __attribute__((objc_direct_members))
 @implementation Foo
+// CHECK-LABEL: define hidden i32 @"\01-[Foo directMethodInExtension]"(
+- (int)directMethodInExtension {
+  return 42;
+}
 // CHECK-LABEL: define hidden i32 @"\01-[Foo getDirect_setDynamic]"(
 // CHECK-LABEL: define internal void @"\01-[Foo setGetDirect_setDynamic:]"(
 // CHECK-LABEL: define internal i32 @"\01-[Foo getDynamic_setDirect]"(
@@ -183,6 +192,17 @@
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
 
+@implementation Foo (Cat)
+// CHECK-LABEL: define hidden i32 @"\01-[Foo directMethodInCategory]"(
+- (int)directMethodInCategory {
+  return 42;
+}
+// CHECK-LABEL: define hidden i32 @"\01-[Foo directMethodInCategoryNoDecl]"(
+- (int)directMethodInCategoryNoDecl __attribute__((objc_direct)) {
+  return 42;
+}
+@end
+
 int useRoot(Root *r) {
   // CHECK-LABEL: define i32 @useRoot
   // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root getInt]"
@@ -195,8 +215,14 @@
   // CHECK-LABEL: define i32 @useFoo
   // CHECK: call void bitcast {{.*}} @"\01-[Foo setGetDynamic_setDirect:]"
   // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo getDirect_setDynamic]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo directMethodInExtension]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo directMethodInCategory]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo directMethodInCategoryNoDecl]"
   [f setGetDynamic_setDirect:1];
-  return [f getDirect_setDynamic];
+  return [f getDirect_setDynamic] +
+ [f directMethodInExtension] +
+ [f directMethodInCategory] +
+ [f directMethodInCategoryNoDecl];
 }
 
 __attribute__((objc_root_class))
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.

[PATCH] D71694: [objc_direct] Tigthen checks for direct methods

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder marked an inline comment as done.
MadCoder added inline comments.



Comment at: clang/test/CodeGenObjC/direct-method.m:196
+@implementation Foo (Cat)
+// CHECK-LABEL: define hidden i32 @"\01-[Foo(Cat) directMethodInCategory]"(
+- (int)directMethodInCategory {

aprantl wrote:
> MadCoder wrote:
> > this may be questionable as a chosen mangling. it's exactly what dynamic 
> > dispatch does, but because a direct method cannot have clashes on the same 
> > class, this extra namespacing is not useful and is likely making the life 
> > of debuggers harder for expression evaluation.
> > 
> > @aprantl should I generate `@"\01-[Foo directMethodInCategory]"` instead? 
> > if yes I'd rather do it in this review
> @teemperor implemented the LLDB support. I would assume that changing it to 
> Foo without the category will be necessary for LLDB to find it. @teemperor 
> Can you add an LLDB test for this scenario?
that was my expectation as well, let me update the code to do so then :)


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

https://reviews.llvm.org/D71694



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71694: [objc_direct] Tigthen checks for direct methods

[Pierre Habouzit via Phabricator via cfe-commits]

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

I had forgotten to clang format and left some tabs here... some day I'll fix my 
vim config


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

https://reviews.llvm.org/D71694

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/DeclObjC.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/CodeGenObjC/direct-method.m
  clang/test/SemaObjC/method-direct-one-definition.m

Index: clang/test/SemaObjC/method-direct-one-definition.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct-one-definition.m
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-protocol-method-implementation %s
+
+__attribute__((objc_root_class))
+@interface A
+@end
+
+@interface A (Cat)
+- (void)A_Cat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@implementation A
+- (void)A_Cat { // expected-error {{direct method was declared in a category but is implemented in the primary interface}}
+}
+@end
+
+__attribute__((objc_root_class))
+@interface B
+- (void)B_primary __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface B ()
+- (void)B_extension __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface B (Cat)
+- (void)B_Cat __attribute__((objc_direct));
+@end
+
+@interface B (OtherCat)
+- (void)B_OtherCat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@implementation B (Cat)
+- (void)B_primary { // expected-error {{direct method was declared in the primary interface but is implemented in a category}}
+}
+- (void)B_extension { // expected-error {{direct method was declared in an extension but is implemented in a different category}}
+}
+- (void)B_Cat {
+}
+- (void)B_OtherCat { // expected-error {{direct method was declared in a category but is implemented in a different category}}
+}
+@end
+
+__attribute__((objc_root_class))
+@interface C
+- (void)C1 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+- (void)C2;  // expected-note {{previous declaration is here}}
+@end
+
+@interface C (Cat)
+- (void)C1;  // expected-error {{method declaration conflicts with previous direct declaration of method 'C1'}}
+- (void)C2 __attribute__((objc_direct)); // expected-error {{direct method declaration conflicts with previous declaration of method 'C2'}}
+@end
Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -172,10 +172,19 @@
 @interface Foo ()
 @property(nonatomic, readwrite) int getDirect_setDynamic;
 @property(nonatomic, readwrite, direct) int getDynamic_setDirect;
+- (int)directMethodInExtension __attribute__((objc_direct));
+@end
+
+@interface Foo (Cat)
+- (int)directMethodInCategory __attribute__((objc_direct));
 @end
 
 __attribute__((objc_direct_members))
 @implementation Foo
+// CHECK-LABEL: define hidden i32 @"\01-[Foo directMethodInExtension]"(
+- (int)directMethodInExtension {
+  return 42;
+}
 // CHECK-LABEL: define hidden i32 @"\01-[Foo getDirect_setDynamic]"(
 // CHECK-LABEL: define internal void @"\01-[Foo setGetDirect_setDynamic:]"(
 // CHECK-LABEL: define internal i32 @"\01-[Foo getDynamic_setDirect]"(
@@ -183,6 +192,17 @@
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
 
+@implementation Foo (Cat)
+// CHECK-LABEL: define hidden i32 @"\01-[Foo(Cat) directMethodInCategory]"(
+- (int)directMethodInCategory {
+  return 42;
+}
+// CHECK-LABEL: define hidden i32 @"\01-[Foo(Cat) directMethodInCategoryNoDecl]"(
+- (int)directMethodInCategoryNoDecl __attribute__((objc_direct)) {
+  return 42;
+}
+@end
+
 int useRoot(Root *r) {
   // CHECK-LABEL: define i32 @useRoot
   // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root getInt]"
@@ -195,8 +215,14 @@
   // CHECK-LABEL: define i32 @useFoo
   // CHECK: call void bitcast {{.*}} @"\01-[Foo setGetDynamic_setDirect:]"
   // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo getDirect_setDynamic]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo directMethodInExtension]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo(Cat) directMethodInCategory]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo(Cat) directMethodInCategoryNoDecl]"
   [f setGetDynamic_setDirect:1];
-  return [f getDirect_setDynamic];
+  return [f getDirect_setDynamic] +
+ [f directMethodInExtension] +
+ [f directMethodInCategory] +
+ [f directMethodInCategoryNoDecl];
 }
 
 __attribute__((objc_root_class))
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ 

[PATCH] D71694: [objc_direct] Tigthen checks for direct methods

[Pierre Habouzit via Phabricator via cfe-commits]

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

Addressed @aprantl review feedback.


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

https://reviews.llvm.org/D71694

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/DeclObjC.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/CodeGenObjC/direct-method.m
  clang/test/SemaObjC/method-direct-one-definition.m

Index: clang/test/SemaObjC/method-direct-one-definition.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct-one-definition.m
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-protocol-method-implementation %s
+
+__attribute__((objc_root_class))
+@interface A
+@end
+
+@interface A (Cat)
+- (void)A_Cat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@implementation A
+- (void)A_Cat { // expected-error {{direct method was declared in a category but is implemented in the primary interface}}
+}
+@end
+
+__attribute__((objc_root_class))
+@interface B
+- (void)B_primary __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface B ()
+- (void)B_extension __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface B (Cat)
+- (void)B_Cat __attribute__((objc_direct));
+@end
+
+@interface B (OtherCat)
+- (void)B_OtherCat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@implementation B (Cat)
+- (void)B_primary { // expected-error {{direct method was declared in the primary interface but is implemented in a category}}
+}
+- (void)B_extension { // expected-error {{direct method was declared in an extension but is implemented in a different category}}
+}
+- (void)B_Cat {
+}
+- (void)B_OtherCat { // expected-error {{direct method was declared in a category but is implemented in a different category}}
+}
+@end
+
+__attribute__((objc_root_class))
+@interface C
+- (void)C1 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+- (void)C2;  // expected-note {{previous declaration is here}}
+@end
+
+@interface C (Cat)
+- (void)C1;  // expected-error {{method declaration conflicts with previous direct declaration of method 'C1'}}
+- (void)C2 __attribute__((objc_direct)); // expected-error {{direct method declaration conflicts with previous declaration of method 'C2'}}
+@end
Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -172,10 +172,19 @@
 @interface Foo ()
 @property(nonatomic, readwrite) int getDirect_setDynamic;
 @property(nonatomic, readwrite, direct) int getDynamic_setDirect;
+- (int)directMethodInExtension __attribute__((objc_direct));
+@end
+
+@interface Foo (Cat)
+- (int)directMethodInCategory __attribute__((objc_direct));
 @end
 
 __attribute__((objc_direct_members))
 @implementation Foo
+// CHECK-LABEL: define hidden i32 @"\01-[Foo directMethodInExtension]"(
+- (int)directMethodInExtension {
+  return 42;
+}
 // CHECK-LABEL: define hidden i32 @"\01-[Foo getDirect_setDynamic]"(
 // CHECK-LABEL: define internal void @"\01-[Foo setGetDirect_setDynamic:]"(
 // CHECK-LABEL: define internal i32 @"\01-[Foo getDynamic_setDirect]"(
@@ -183,6 +192,17 @@
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
 
+@implementation Foo (Cat)
+// CHECK-LABEL: define hidden i32 @"\01-[Foo(Cat) directMethodInCategory]"(
+- (int)directMethodInCategory {
+  return 42;
+}
+// CHECK-LABEL: define hidden i32 @"\01-[Foo(Cat) directMethodInCategoryNoDecl]"(
+- (int)directMethodInCategoryNoDecl __attribute__((objc_direct)) {
+  return 42;
+}
+@end
+
 int useRoot(Root *r) {
   // CHECK-LABEL: define i32 @useRoot
   // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root getInt]"
@@ -195,8 +215,14 @@
   // CHECK-LABEL: define i32 @useFoo
   // CHECK: call void bitcast {{.*}} @"\01-[Foo setGetDynamic_setDirect:]"
   // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo getDirect_setDynamic]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo directMethodInExtension]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo(Cat) directMethodInCategory]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo(Cat) directMethodInCategoryNoDecl]"
   [f setGetDynamic_setDirect:1];
-  return [f getDirect_setDynamic];
+  return [f getDirect_setDynamic] +
+ [f directMethodInExtension] +
+ [f directMethodInCategory] +
+ [f directMethodInCategoryNoDecl];
 }
 
 __attribute__((objc_root_class))
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4508,12 +4508,6 @@
 

[PATCH] D71694: [objc_direct] Tigthen checks for direct methods

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder marked an inline comment as done.
MadCoder added inline comments.



Comment at: clang/lib/AST/DeclObjC.cpp:963
 if (ObjCInterfaceDecl *IFD = ImplD->getClassInterface()) {
-  if (ObjCMethodDecl *MD = IFD->getMethod(getSelector(),
-  isInstanceMethod()))
+  // When the container is the principal @implementation,
+  // the canonical Decl is either in an @interface, or in an extension.

aprantl wrote:
> What does "principal" mean here?
primary sorry, will fix


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

https://reviews.llvm.org/D71694



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71694: [objc_direct] Tigthen checks for direct methods

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a subscriber: aprantl.
MadCoder added inline comments.



Comment at: clang/test/CodeGenObjC/direct-method.m:196
+@implementation Foo (Cat)
+// CHECK-LABEL: define hidden i32 @"\01-[Foo(Cat) directMethodInCategory]"(
+- (int)directMethodInCategory {

this may be questionable as a chosen mangling. it's exactly what dynamic 
dispatch does, but because a direct method cannot have clashes on the same 
class, this extra namespacing is not useful and is likely making the life of 
debuggers harder for expression evaluation.

@aprantl should I generate `@"\01-[Foo directMethodInCategory]"` instead? if 
yes I'd rather do it in this review


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

https://reviews.llvm.org/D71694



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71694: [objc_direct] Tigthen checks for direct methods

[Pierre Habouzit via Phabricator via cfe-commits]

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

Small update because I uploaded before the full completion of the test-suite 
and somehow it's sentient and punished me... (some diagnostics were too verbose 
and stuttering things with `Sema::CheckObjCMethodOverrides`).


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

https://reviews.llvm.org/D71694

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/DeclObjC.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/CodeGenObjC/direct-method.m
  clang/test/SemaObjC/method-direct-one-definition.m

Index: clang/test/SemaObjC/method-direct-one-definition.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct-one-definition.m
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-protocol-method-implementation %s
+
+__attribute__((objc_root_class))
+@interface A
+@end
+
+@interface A (Cat)
+- (void)A_Cat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@implementation A
+- (void)A_Cat { // expected-error {{direct method was declared in a category but is implemented in the primary interface}}
+}
+@end
+
+__attribute__((objc_root_class))
+@interface B
+- (void)B_primary __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface B ()
+- (void)B_extension __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface B (Cat)
+- (void)B_Cat __attribute__((objc_direct));
+@end
+
+@interface B (OtherCat)
+- (void)B_OtherCat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@implementation B (Cat)
+- (void)B_primary { // expected-error {{direct method was declared in the primary interface but is implemented in a category}}
+}
+- (void)B_extension { // expected-error {{direct method was declared in an extension but is implemented in a different category}}
+}
+- (void)B_Cat {
+}
+- (void)B_OtherCat { // expected-error {{direct method was declared in a category but is implemented in a different category}}
+}
+@end
+
+__attribute__((objc_root_class))
+@interface C
+- (void)C1 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+- (void)C2;  // expected-note {{previous declaration is here}}
+@end
+
+@interface C (Cat)
+- (void)C1;  // expected-error {{method declaration conflicts with previous direct declaration of method 'C1'}}
+- (void)C2 __attribute__((objc_direct)); // expected-error {{direct method declaration conflicts with previous declaration of method 'C2'}}
+@end
Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -172,10 +172,19 @@
 @interface Foo ()
 @property(nonatomic, readwrite) int getDirect_setDynamic;
 @property(nonatomic, readwrite, direct) int getDynamic_setDirect;
+- (int)directMethodInExtension __attribute__((objc_direct));
+@end
+
+@interface Foo (Cat)
+- (int)directMethodInCategory __attribute__((objc_direct));
 @end
 
 __attribute__((objc_direct_members))
 @implementation Foo
+// CHECK-LABEL: define hidden i32 @"\01-[Foo directMethodInExtension]"(
+- (int)directMethodInExtension {
+  return 42;
+}
 // CHECK-LABEL: define hidden i32 @"\01-[Foo getDirect_setDynamic]"(
 // CHECK-LABEL: define internal void @"\01-[Foo setGetDirect_setDynamic:]"(
 // CHECK-LABEL: define internal i32 @"\01-[Foo getDynamic_setDirect]"(
@@ -183,6 +192,17 @@
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
 
+@implementation Foo (Cat)
+// CHECK-LABEL: define hidden i32 @"\01-[Foo(Cat) directMethodInCategory]"(
+- (int)directMethodInCategory {
+  return 42;
+}
+// CHECK-LABEL: define hidden i32 @"\01-[Foo(Cat) directMethodInCategoryNoDecl]"(
+- (int)directMethodInCategoryNoDecl __attribute__((objc_direct)) {
+  return 42;
+}
+@end
+
 int useRoot(Root *r) {
   // CHECK-LABEL: define i32 @useRoot
   // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root getInt]"
@@ -195,8 +215,14 @@
   // CHECK-LABEL: define i32 @useFoo
   // CHECK: call void bitcast {{.*}} @"\01-[Foo setGetDynamic_setDirect:]"
   // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo getDirect_setDynamic]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo directMethodInExtension]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo(Cat) directMethodInCategory]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo(Cat) directMethodInCategoryNoDecl]"
   [f setGetDynamic_setDirect:1];
-  return [f getDirect_setDynamic];
+  return [f getDirect_setDynamic] +
+ [f directMethodInExtension] +
+ [f directMethodInCategory] +
+ [f directMethodInCategoryNoDecl];
 }
 
 __attribute__((objc_root_class))
Index: clang/lib/S

[PATCH] D71694: [objc_direct] Tigthen checks for direct methods

[Pierre Habouzit via Phabricator via cfe-commits]

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

Because the name of a direct method must be agreed upon by the caller and the 
implementation, certain bad practices that one can get away with when using 
dynamism are fatal with direct methods.

To avoid really weird and unscruttable linker error, tighten the front-end 
error reporting.

Rule 1:

  Direct methods can only have at most one declaration in an @interface 
container. Any redeclaration is strictly forbidden.
  Today some amount of redeclaration is tolerated between the main interface 
and categories for dynamic methods, but we can't have that.

Rule 2:

  Direct method implementations can only be declared in a matching @interface 
container: when implemented in the primary @implementation then the declaration 
must be in the primary @interface or an extension, and when implemented in a 
category, the declaration must be in the @interface for the same category.

Also fix another issue with ObjCMethod::getCanonicalDecl(): when an 
implementation lives in the primary @interface, then its canonical declaration 
can be in any extension, even when it's not an accessor.

Add Sema tests to cover the new errors, and CG tests to beef up testing around 
function names for categories and extensions.

Radar-Id: rdar://problem/58054563


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71694

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/DeclObjC.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/CodeGenObjC/direct-method.m
  clang/test/SemaObjC/method-direct-one-definition.m

Index: clang/test/SemaObjC/method-direct-one-definition.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct-one-definition.m
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-protocol-method-implementation %s
+
+__attribute__((objc_root_class))
+@interface A
+@end
+
+@interface A (Cat)
+- (void)A_Cat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@implementation A
+- (void)A_Cat {
+} // expected-error {{direct method was declared in a category but is implemented in the primary interface}}
+@end
+
+__attribute__((objc_root_class))
+@interface B
+- (void)B_primary __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface B ()
+- (void)B_extension __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface B (Cat)
+- (void)B_Cat __attribute__((objc_direct));
+@end
+
+@interface B (OtherCat)
+- (void)B_OtherCat __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@implementation B (Cat)
+- (void)B_primary { // expected-error {{direct method was declared in the primary interface but is implemented in a category}}
+}
+- (void)B_extension { // expected-error {{direct method was declared in an extension but is implemented in a different category}}
+}
+- (void)B_Cat {
+}
+- (void)B_OtherCat { // expected-error {{direct method was declared in a category but is implemented in a different category}}
+}
+@end
+
+__attribute__((objc_root_class))
+@interface C
+- (void)C1 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+- (void)C2;  // expected-note {{previous declaration is here}}
+@end
+
+@interface C (Cat)
+- (void)C1;  // expected-error {{method declaration conflicts with previous direct declaration of method 'C1'}}
+- (void)C2 __attribute__((objc_direct)); // expected-error {{direct method declaration conflicts with previous declaration of method 'C2'}}
+@end
Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -172,10 +172,19 @@
 @interface Foo ()
 @property(nonatomic, readwrite) int getDirect_setDynamic;
 @property(nonatomic, readwrite, direct) int getDynamic_setDirect;
+- (int)directMethodInExtension __attribute__((objc_direct));
+@end
+
+@interface Foo (Cat)
+- (int)directMethodInCategory __attribute__((objc_direct));
 @end
 
 __attribute__((objc_direct_members))
 @implementation Foo
+// CHECK-LABEL: define hidden i32 @"\01-[Foo directMethodInExtension]"(
+- (int)directMethodInExtension {
+  return 42;
+}
 // CHECK-LABEL: define hidden i32 @"\01-[Foo getDirect_setDynamic]"(
 // CHECK-LABEL: define internal void @"\01-[Foo setGetDirect_setDynamic:]"(
 // CHECK-LABEL: define internal i32 @"\01-[Foo getDynamic_setDirect]"(
@@ -183,6 +192,17 @@
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
 
+@implementation Foo (Cat)
+// CHECK-LABEL: define hidden i32 @"\01-[Foo(Cat) directMethodInCategory]"(
+- (int)directMe

[PATCH] D71682: Relax the rules around objc_alloc and objc_alloc_init optimizations.

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder created this revision.
MadCoder added reviewers: rjmccall, arphaman, erik.pilkington, ahatanak.
MadCoder added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith.
MadCoder edited the summary of this revision.

Today the optimization is limited to:

- `[ClassName alloc]`
- `[self alloc] when within a class method`

However it means that when code is written this way:

  @interface MyObject
  - (id)copyWithZone:(NSZone *)zone
  {
  return [[self.class alloc] _initWith...];
  }
  @end

... then the optimization doesn't kick in and +[NSObject alloc] ends up in IMP 
caches where it could have been avoided.

It turns out that +alloc -> +[NSObject alloc] is the most cached SEL/IMP pair 
in the entire platform which is rather silly).

There's two theoretical risks allowing this optimization:

1. if the receiver is nil (which it can't be today), but it turns out that 
objc_alloc()/objc_alloc_init() cope with a nil receiver,
2. if the `Class` type for the receiver is a lie. However, for such a code to 
work today (and not fail witn an unrecognized selector anyway) you'd have to 
have implemented the -alloc *instance method*.

  Fortunately, objc_alloc() doesn't assume that the receiver is a Class, it 
basically starts with a test that is similar to `if (receiver->isa->bits & 
hasDefaultAWZ) { /* fastpath */ }`. This bit is only set on metaclasses by the 
runtime, so if an instance is passed to this function by accident, its isa will 
fail this test, and objc_alloc() will gracefully fallback to objc_msgSend().

  The one thing objc_alloc() doesn't support is tagged pointer instances. None 
of the tagged pointer classes implement an instance method called 'alloc' 
(actually there's a single class in the entire Apple codebase that has such a 
method).

Radar-Id: rdar://problem/58058316


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71682

Files:
  clang/lib/CodeGen/CGObjC.cpp
  clang/test/CodeGenObjC/objc-alloc-init.m

Index: clang/test/CodeGenObjC/objc-alloc-init.m
===
--- clang/test/CodeGenObjC/objc-alloc-init.m
+++ clang/test/CodeGenObjC/objc-alloc-init.m
@@ -22,21 +22,30 @@
 }
 
 @interface Y : X
++(Class)class;
 +(void)meth;
 -(void)instanceMeth;
 @end
 
 @implementation Y
++(Class)class {
+  return self;
+}
 +(void)meth {
   [[self alloc] init];
   // OPTIMIZED: call i8* @objc_alloc_init(
   // NOT_OPTIMIZED: call i8* @objc_alloc(
 }
++ (void)meth2 {
+  [[[self class] alloc] init];
+  // OPTIMIZED: call i8* @objc_alloc_init(
+  // NOT_OPTIMIZED: call i8* @objc_alloc(
+}
 -(void)instanceMeth {
   // EITHER-NOT: call i8* @objc_alloc
   // EITHER: call {{.*}} @objc_msgSend
   // EITHER: call {{.*}} @objc_msgSend
-  [[self alloc] init];
+  [[(id)self alloc] init];
 }
 @end
 
Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -461,38 +461,39 @@
   Sel.getNameForSlot(0) != "init")
 return None;
 
-  // Okay, this is '[receiver init]', check if 'receiver' is '[cls alloc]' or
-  // we are in an ObjC class method and 'receiver' is '[self alloc]'.
+  // Okay, this is '[receiver init]', check if 'receiver' is '[cls alloc]'
+  // with 'cls' a Class.
   auto *SubOME =
   dyn_cast(OME->getInstanceReceiver()->IgnoreParenCasts());
   if (!SubOME)
 return None;
   Selector SubSel = SubOME->getSelector();
 
-  // Check if we are in an ObjC class method and the receiver expression is
-  // 'self'.
-  const Expr *SelfInClassMethod = nullptr;
-  if (const auto *CurMD = dyn_cast_or_null(CGF.CurFuncDecl))
-if (CurMD->isClassMethod())
-  if ((SelfInClassMethod = SubOME->getInstanceReceiver()))
-if (!SelfInClassMethod->isObjCSelfExpr())
-  SelfInClassMethod = nullptr;
-
-  if ((SubOME->getReceiverKind() != ObjCMessageExpr::Class &&
-   !SelfInClassMethod) || !SubOME->getType()->isObjCObjectPointerType() ||
+  if (!SubOME->getType()->isObjCObjectPointerType() ||
   !SubSel.isUnarySelector() || SubSel.getNameForSlot(0) != "alloc")
 return None;
 
-  llvm::Value *Receiver;
-  if (SelfInClassMethod) {
-Receiver = CGF.EmitScalarExpr(SelfInClassMethod);
-  } else {
+  llvm::Value *Receiver = nullptr;
+  switch (SubOME->getReceiverKind()) {
+  case ObjCMessageExpr::Instance:
+if (!SubOME->getInstanceReceiver()->getType()->isObjCClassType())
+  return None;
+Receiver = CGF.EmitScalarExpr(SubOME->getInstanceReceiver());
+break;
+
+  case ObjCMessageExpr::Class: {
 QualType ReceiverType = SubOME->getClassReceiver();
 const ObjCObjectType *ObjTy = ReceiverType->getAs();
 const ObjCInterfaceDecl *ID = ObjTy->getInterface();
 assert(ID && "null interface should be impossible here");
 Receiver = CGF.CGM.getObjCRuntime().GetClass(CGF, ID);
+break;
+  }
+  case ObjCMessageExpr::SuperInstance:
+  case ObjCMessageExpr::SuperClass

[PATCH] D71588: [objc_direct] fix uniquing when re-declaring a readwrite-direct property

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder created this revision.
MadCoder added reviewers: arphaman, rjmccall, dexonsmith.
MadCoder added a project: clang.
Herald added a subscriber: cfe-commits.
MadCoder added a reviewer: liuliu.

ObjCMethodDecl::getCanonicalDecl() for re-declared readwrite properties,
only looks in the ObjCInterface for the declaration of the setter
method, which it won't find.

When the method is a property accessor, we must look in extensions for a
possible redeclaration.

Extend the CG test to cover this issue.

Radar-Id: rdar://problem/57991337


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71588

Files:
  clang/lib/AST/DeclObjC.cpp
  clang/test/CodeGenObjC/direct-method.m


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -191,6 +191,14 @@
   return [r getInt] + [r intProperty] + [r intProperty2];
 }
 
+int useFoo(Foo *f) {
+  // CHECK-LABEL: define i32 @useFoo
+  // CHECK: call void bitcast {{.*}} @"\01-[Foo setGetDynamic_setDirect:]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo 
getDirect_setDynamic]"
+  [f setGetDynamic_setDirect:1];
+  return [f getDirect_setDynamic];
+}
+
 __attribute__((objc_root_class))
 @interface RootDeclOnly
 @property(direct, readonly) int intProperty;
Index: clang/lib/AST/DeclObjC.cpp
===
--- clang/lib/AST/DeclObjC.cpp
+++ clang/lib/AST/DeclObjC.cpp
@@ -958,10 +958,18 @@
   auto *CtxD = cast(getDeclContext());
 
   if (auto *ImplD = dyn_cast(CtxD)) {
-if (ObjCInterfaceDecl *IFD = ImplD->getClassInterface())
+if (ObjCInterfaceDecl *IFD = ImplD->getClassInterface()) {
   if (ObjCMethodDecl *MD = IFD->getMethod(getSelector(),
   isInstanceMethod()))
 return MD;
+  // readwrite properties may have been re-declared in an extension.
+  // look harder.
+  if (isPropertyAccessor())
+for (auto *Ext : IFD->known_extensions())
+  if (ObjCMethodDecl *MD = Ext->getMethod(getSelector(),
+  isInstanceMethod()))
+return MD;
+}
   } else if (auto *CImplD = dyn_cast(CtxD)) {
 if (ObjCCategoryDecl *CatD = CImplD->getCategoryDecl())
   if (ObjCMethodDecl *MD = CatD->getMethod(getSelector(),


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -191,6 +191,14 @@
   return [r getInt] + [r intProperty] + [r intProperty2];
 }
 
+int useFoo(Foo *f) {
+  // CHECK-LABEL: define i32 @useFoo
+  // CHECK: call void bitcast {{.*}} @"\01-[Foo setGetDynamic_setDirect:]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Foo getDirect_setDynamic]"
+  [f setGetDynamic_setDirect:1];
+  return [f getDirect_setDynamic];
+}
+
 __attribute__((objc_root_class))
 @interface RootDeclOnly
 @property(direct, readonly) int intProperty;
Index: clang/lib/AST/DeclObjC.cpp
===
--- clang/lib/AST/DeclObjC.cpp
+++ clang/lib/AST/DeclObjC.cpp
@@ -958,10 +958,18 @@
   auto *CtxD = cast(getDeclContext());
 
   if (auto *ImplD = dyn_cast(CtxD)) {
-if (ObjCInterfaceDecl *IFD = ImplD->getClassInterface())
+if (ObjCInterfaceDecl *IFD = ImplD->getClassInterface()) {
   if (ObjCMethodDecl *MD = IFD->getMethod(getSelector(),
   isInstanceMethod()))
 return MD;
+  // readwrite properties may have been re-declared in an extension.
+  // look harder.
+  if (isPropertyAccessor())
+for (auto *Ext : IFD->known_extensions())
+  if (ObjCMethodDecl *MD = Ext->getMethod(getSelector(),
+  isInstanceMethod()))
+return MD;
+}
   } else if (auto *CImplD = dyn_cast(CtxD)) {
 if (ObjCCategoryDecl *CatD = CImplD->getCategoryDecl())
   if (ObjCMethodDecl *MD = CatD->getMethod(getSelector(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

In D71091#1776009 , @liuliu wrote:

> No problem! I have a custom patch that isn't as nice but get the job done. 
> Let me know when you put new diff out and I can test it on our codebase again.


https://reviews.llvm.org/D71226/new/ is the incremental patch as this one is 
already merged


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71091



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71226: Also synthesize _cmd and self for properties

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

In D71226#1776038 , @rjmccall wrote:

> It'd be nice if we didn't have to synthesize parameters for all declarations 
> when we're just handling an `@interface`, but we can improve that in 
> follow-ups.


yeah ideally I'd want to do it lazily when we emit the direct call, but at this 
point the Decl is const and that was a huge change to make.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71226



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71226: Also synthesize _cmd and self for properties

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

@liuliu that fixes your test case (which I reproduced in the CG test)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71226



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71226: Also synthesize _cmd and self for properties

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder created this revision.
MadCoder added reviewers: liuliu, rjmccall, arphaman, jfb.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.
MadCoder added a comment.

@liuliu that fixes your test case (which I reproduced in the CG test)


Radar-Id: rdar://problem/57764169


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71226

Files:
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/CodeGenObjC/direct-method.m


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -190,3 +190,14 @@
   // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root intProperty2]"
   return [r getInt] + [r intProperty] + [r intProperty2];
 }
+
+__attribute__((objc_root_class))
+@interface RootDeclOnly
+@property(direct, readonly) int intProperty;
+@end
+
+int useRootDeclOnly(RootDeclOnly *r) {
+  // CHECK-LABEL: define i32 @useRootDeclOnly
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[RootDeclOnly 
intProperty]"
+  return [r intProperty];
+}
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -2500,6 +2500,8 @@
 // A user declared getter will be synthesize when @synthesize of
 // the property with the same name is seen in the @implementation
 GetterMethod->setPropertyAccessor(true);
+
+  GetterMethod->createImplicitParams(Context, 
GetterMethod->getClassInterface());
   property->setGetterMethodDecl(GetterMethod);
 
   // Skip setter if property is read-only.
@@ -2574,6 +2576,8 @@
   // A user declared setter will be synthesize when @synthesize of
   // the property with the same name is seen in the @implementation
   SetterMethod->setPropertyAccessor(true);
+
+SetterMethod->createImplicitParams(Context, 
SetterMethod->getClassInterface());
 property->setSetterMethodDecl(SetterMethod);
   }
   // Add any synthesized methods to the global pool. This allows us to


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -190,3 +190,14 @@
   // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root intProperty2]"
   return [r getInt] + [r intProperty] + [r intProperty2];
 }
+
+__attribute__((objc_root_class))
+@interface RootDeclOnly
+@property(direct, readonly) int intProperty;
+@end
+
+int useRootDeclOnly(RootDeclOnly *r) {
+  // CHECK-LABEL: define i32 @useRootDeclOnly
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[RootDeclOnly intProperty]"
+  return [r intProperty];
+}
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -2500,6 +2500,8 @@
 // A user declared getter will be synthesize when @synthesize of
 // the property with the same name is seen in the @implementation
 GetterMethod->setPropertyAccessor(true);
+
+  GetterMethod->createImplicitParams(Context, GetterMethod->getClassInterface());
   property->setGetterMethodDecl(GetterMethod);
 
   // Skip setter if property is read-only.
@@ -2574,6 +2576,8 @@
   // A user declared setter will be synthesize when @synthesize of
   // the property with the same name is seen in the @implementation
   SetterMethod->setPropertyAccessor(true);
+
+SetterMethod->createImplicitParams(Context, SetterMethod->getClassInterface());
 property->setSetterMethodDecl(SetterMethod);
   }
   // Add any synthesized methods to the global pool. This allows us to
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

In D71091#1775722 , @liuliu wrote:

> We don't use PCH :) I arc patched the latest diff, still have that crash. 
> More strangely, I tried this latest diff on previously simplistic test case 
> and it crashes now:
>
>   #import 
>  
>   @interface SCTestMainEntity : NSObject
>  
>   @property (nonatomic, readonly, direct) int date;
>  
>   - (instancetype)initWithDate:(int)date;
>  
>   @end
>  
>   int main(void)
>   {
>   SCTestMainEntity *entity = [[SCTestMainEntity alloc] initWithDate:10];
>   printf("%d\n", entity.date);
>   return 0;
>   }
>


Ugh, you're right, and that's because my test is missing one case. doh.

you can work-around by re-redefining `-(int)date __attribute__((objc_direct));` 
next to the property. it works in the test beacuse the call site picks up the 
implementation to form the call. sigh.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71091



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added inline comments.



Comment at: clang/lib/Serialization/ASTWriterDecl.cpp:674
   }
   Record.push_back(D->isInstanceMethod());
   Record.push_back(D->isVariadic());

arphaman wrote:
> @MadCoder Given the fact that you're now setting SelfDecl and CmdDecl for 
> declarations, they need to be serialized properly. I would suggest the 
> following change to the writer:
> 
> ```
> bool HasBody = D->getBody() != nullptr;
> Record.push_back(HasBody);
> if (HasBody)
>   Record.AddStmt(D->getBody());
> Record.AddDeclRef(D->getSelfDecl());
> Record.AddDeclRef(D-> getCmdDecl());
> ```
> 
> The AST reader should be updated accordingly.
hah was the source of the last etst failure I was chasing, thanks!


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

https://reviews.llvm.org/D71091



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 232644.
MadCoder marked an inline comment as done.
MadCoder added a comment.

@liuliu I wouldn't be surprised this addresses your issues if snapchat is using 
PCH


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

https://reviews.llvm.org/D71091

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/test/CodeGenObjC/direct-method.m

Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -11,9 +11,17 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (int)getInt __attribute__((objc_direct));
+@property(direct, readonly) int intProperty;
+@property(direct, readonly) int intProperty2;
 @end
 
 @implementation Root
+// CHECK-LABEL: define hidden i32 @"\01-[Root intProperty2]"
+- (int)intProperty2 {
+  return 42;
+}
+
 // CHECK-LABEL: define hidden i32 @"\01-[Root getInt]"(
 - (int)getInt __attribute__((objc_direct)) {
   // loading parameters
@@ -152,6 +160,7 @@
 }
 
 @end
+// CHECK-LABEL: define hidden i32 @"\01-[Root intProperty]"
 
 @interface Foo : Root {
   id __strong _cause_cxx_destruct;
@@ -173,3 +182,11 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHECK-LABEL: define i32 @useRoot
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root intProperty]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root intProperty2]"
+  return [r getInt] + [r intProperty] + [r intProperty2];
+}
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -722,13 +722,6 @@
 } else if (const auto *OCD = dyn_cast(DC)) {
   OS << OCD->getClassInterface()->getName() << '('
  << OCD->getName() << ')';
-} else if (isa(DC)) {
-  // We can extract the type of the class from the self pointer.
-  if (ImplicitParamDecl *SelfDecl = OMD->getSelfDecl()) {
-QualType ClassTy =
-cast(SelfDecl->getType())->getPointeeType();
-ClassTy.print(OS, PrintingPolicy(LangOptions()));
-  }
 }
 OS << ' ' << OMD->getSelector().getAsString() << ']';
 
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -664,14 +664,13 @@
   VisitNamedDecl(D);
   // FIXME: convert to LazyStmtPtr?
   // Unlike C/C++, method bodies will never be in header files.
-  bool HasBodyStuff = D->getBody() != nullptr ||
-  D->getSelfDecl() != nullptr || D->getCmdDecl() != nullptr;
+  bool HasBodyStuff = D->getBody() != nullptr;
   Record.push_back(HasBodyStuff);
   if (HasBodyStuff) {
 Record.AddStmt(D->getBody());
-Record.AddDeclRef(D->getSelfDecl());
-Record.AddDeclRef(D->getCmdDecl());
   }
+  Record.AddDeclRef(D->getSelfDecl());
+  Record.AddDeclRef(D->getCmdDecl());
   Record.push_back(D->isInstanceMethod());
   Record.push_back(D->isVariadic());
   Record.push_back(D->isPropertyAccessor());
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1017,9 +1017,9 @@
 // definitions rarely show up in headers.
 Reader.PendingBodies[MD] = GetCurrentCursorOffset();
 HasPendingBody = true;
-MD->setSelfDecl(ReadDeclAs());
-MD->setCmdDecl(ReadDeclAs());
   }
+  MD->setSelfDecl(ReadDeclAs());
+  MD->setCmdDecl(ReadDeclAs());
   MD->setInstanceMethod(Record.readInt());
   MD->setVariadic(Record.readInt());
   MD->setPropertyAccessor(Record.readInt());
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4869,6 +4869,9 @@
 }
   }
 
+  // Insert the invisible arguments, self and _cmd!
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+
   ActOnDocumentableDecl(ObjCMethod);
 
   return ObjCMethod;
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4027,7 +4027,7 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
 

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 232641.
MadCoder marked an inline comment as done.
MadCoder added a comment.

woops a bogus hunk went in


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

https://reviews.llvm.org/D71091

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/test/CodeGenObjC/direct-method.m

Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -11,9 +11,17 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (int)getInt __attribute__((objc_direct));
+@property(direct, readonly) int intProperty;
+@property(direct, readonly) int intProperty2;
 @end
 
 @implementation Root
+// CHECK-LABEL: define hidden i32 @"\01-[Root intProperty2]"
+- (int)intProperty2 {
+  return 42;
+}
+
 // CHECK-LABEL: define hidden i32 @"\01-[Root getInt]"(
 - (int)getInt __attribute__((objc_direct)) {
   // loading parameters
@@ -152,6 +160,7 @@
 }
 
 @end
+// CHECK-LABEL: define hidden i32 @"\01-[Root intProperty]"
 
 @interface Foo : Root {
   id __strong _cause_cxx_destruct;
@@ -173,3 +182,11 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHECK-LABEL: define i32 @useRoot
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root intProperty]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root intProperty2]"
+  return [r getInt] + [r intProperty] + [r intProperty2];
+}
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -722,13 +722,6 @@
 } else if (const auto *OCD = dyn_cast(DC)) {
   OS << OCD->getClassInterface()->getName() << '('
  << OCD->getName() << ')';
-} else if (isa(DC)) {
-  // We can extract the type of the class from the self pointer.
-  if (ImplicitParamDecl *SelfDecl = OMD->getSelfDecl()) {
-QualType ClassTy =
-cast(SelfDecl->getType())->getPointeeType();
-ClassTy.print(OS, PrintingPolicy(LangOptions()));
-  }
 }
 OS << ' ' << OMD->getSelector().getAsString() << ']';
 
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -664,8 +664,7 @@
   VisitNamedDecl(D);
   // FIXME: convert to LazyStmtPtr?
   // Unlike C/C++, method bodies will never be in header files.
-  bool HasBodyStuff = D->getBody() != nullptr ||
-  D->getSelfDecl() != nullptr || D->getCmdDecl() != nullptr;
+  bool HasBodyStuff = D->getBody() != nullptr;
   Record.push_back(HasBodyStuff);
   if (HasBodyStuff) {
 Record.AddStmt(D->getBody());
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4869,6 +4869,9 @@
 }
   }
 
+  // Insert the invisible arguments, self and _cmd!
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+
   ActOnDocumentableDecl(ObjCMethod);
 
   return ObjCMethod;
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4027,7 +4027,7 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD);
+  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
   if (I != DirectMethodDefinitions.end())
 return I->second;
 
@@ -4040,7 +4040,7 @@
   llvm::Function *Method =
   llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
  Name.str(), &CGM.getModule());
-  DirectMethodDefinitions.insert(std::make_pair(OMD, Method));
+  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), Method));
 
   return Method;
 }
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -292,13 +292,6 @@
 }
   } else if (const auto *OCD = dyn_cast(DC)) {
 OS << OCD->getClassInterface()->getName() << '(' << OCD->getName() << ')';
-  } else if (isa(DC)) {
-// We can extract the type of the class from the self 

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added inline comments.



Comment at: clang/lib/Sema/SemaDeclObjC.cpp:4839
 
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+

MadCoder wrote:
> liuliu wrote:
> > Yeah, I applied this change. As I said, for my trivial example it does 
> > work. But for my more thorough example with @property (direct) etc, it 
> > doesn't. I can dig into more later today or Monday. I just got everything 
> > work (with another more hackier fix) and would first to evaluate this as a 
> > viable path first before commit more time to debug this.
> > 
> > We probably can also just have a follow-up fix for that, depends on if 
> > reviewers thought this is blocking or not. I personally think not.
> Huh I could have seen why property methods would have an issue but they seem 
> to have gotten their `direct`ness properly anyway (I'll add them to the CG 
> test for good measure while I debug the tests I broke).
> 
> so I'll need a reduced test case from you I can't seem to find an easy way to 
> break it indeed.
I just added a test using a direct property with both a synthesized and a 
manual one, and it seems to work for me (?)


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

https://reviews.llvm.org/D71091



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 232638.

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

https://reviews.llvm.org/D71091

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/test/CodeGenObjC/direct-method.m

Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -11,9 +11,17 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (int)getInt __attribute__((objc_direct));
+@property(direct, readonly) int intProperty;
+@property(direct, readonly) int intProperty2;
 @end
 
 @implementation Root
+// CHECK-LABEL: define hidden i32 @"\01-[Root intProperty2]"
+- (int)intProperty2 {
+  return 42;
+}
+
 // CHECK-LABEL: define hidden i32 @"\01-[Root getInt]"(
 - (int)getInt __attribute__((objc_direct)) {
   // loading parameters
@@ -152,6 +160,7 @@
 }
 
 @end
+// CHECK-LABEL: define hidden i32 @"\01-[Root intProperty]"
 
 @interface Foo : Root {
   id __strong _cause_cxx_destruct;
@@ -173,3 +182,11 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHECK-LABEL: define i32 @useRoot
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root intProperty]"
+  // CHECK: %{{[^ ]*}} = call i32 bitcast {{.*}} @"\01-[Root intProperty2]"
+  return [r getInt] + [r intProperty] + [r intProperty2];
+}
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -722,13 +722,6 @@
 } else if (const auto *OCD = dyn_cast(DC)) {
   OS << OCD->getClassInterface()->getName() << '('
  << OCD->getName() << ')';
-} else if (isa(DC)) {
-  // We can extract the type of the class from the self pointer.
-  if (ImplicitParamDecl *SelfDecl = OMD->getSelfDecl()) {
-QualType ClassTy =
-cast(SelfDecl->getType())->getPointeeType();
-ClassTy.print(OS, PrintingPolicy(LangOptions()));
-  }
 }
 OS << ' ' << OMD->getSelector().getAsString() << ']';
 
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -664,8 +664,7 @@
   VisitNamedDecl(D);
   // FIXME: convert to LazyStmtPtr?
   // Unlike C/C++, method bodies will never be in header files.
-  bool HasBodyStuff = D->getBody() != nullptr ||
-  D->getSelfDecl() != nullptr || D->getCmdDecl() != nullptr;
+  bool HasBodyStuff = D->getBody() != nullptr;
   Record.push_back(HasBodyStuff);
   if (HasBodyStuff) {
 Record.AddStmt(D->getBody());
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -383,9 +383,6 @@
   // Create Decl objects for each parameter, entrring them in the scope for
   // binding to their use.
 
-  // Insert the invisible arguments, self and _cmd!
-  MDecl->createImplicitParams(Context, MDecl->getClassInterface());
-
   PushOnScopeChains(MDecl->getSelfDecl(), FnBodyScope);
   PushOnScopeChains(MDecl->getCmdDecl(), FnBodyScope);
 
@@ -4869,6 +4866,9 @@
 }
   }
 
+  // Insert the invisible arguments, self and _cmd!
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+
   ActOnDocumentableDecl(ObjCMethod);
 
   return ObjCMethod;
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4027,7 +4027,7 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD);
+  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
   if (I != DirectMethodDefinitions.end())
 return I->second;
 
@@ -4040,7 +4040,7 @@
   llvm::Function *Method =
   llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
  Name.str(), &CGM.getModule());
-  DirectMethodDefinitions.insert(std::make_pair(OMD, Method));
+  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), Method));
 
   return Method;
 }
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder marked an inline comment as done.
MadCoder added inline comments.



Comment at: clang/lib/Sema/SemaDeclObjC.cpp:4839
 
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+

liuliu wrote:
> Yeah, I applied this change. As I said, for my trivial example it does work. 
> But for my more thorough example with @property (direct) etc, it doesn't. I 
> can dig into more later today or Monday. I just got everything work (with 
> another more hackier fix) and would first to evaluate this as a viable path 
> first before commit more time to debug this.
> 
> We probably can also just have a follow-up fix for that, depends on if 
> reviewers thought this is blocking or not. I personally think not.
Huh I could have seen why property methods would have an issue but they seem to 
have gotten their `direct`ness properly anyway (I'll add them to the CG test 
for good measure while I debug the tests I broke).

so I'll need a reduced test case from you I can't seem to find an easy way to 
break it indeed.


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

https://reviews.llvm.org/D71091



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

In D71091#1773066 , @liuliu wrote:

> With this latest fix applied on top of 
> b220662a45c8067a2ae485ae34c1138d93506df9 
> , in our 
> company's internal code, I still encounter the crash.
>
>   Showing All Messages
>   /Users/liuliu/Snapchat/Dev/phantom/3.   
> Libraries/Storage/DocObject/Implementations/SCSQLiteDocObjectContext/Tests/Generated/SCTestMainEntityChangeRequest.mm:338:25:
>  LLVM IR generation of compound statement ('{}')
>   /Users/liuliu/Snapchat/Dev/phantom/0  clang-10 
> 0x00010b9edf0c llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 60
>   1  clang-10 0x00010b9ee4c9 
> PrintStackTraceSignalHandler(void*) + 25
>   /Users/liuliu/Snapchat/Dev/phantom/2  clang-10 
> 0x00010b9ec086 llvm::sys::RunSignalHandlers() + 118
>   3  clang-10 0x00010b9f1e6c SignalHandler(int) + 252
>   4  libsystem_platform.dylib 0x7fff65d8eb5d _sigtramp + 29
>   5  libsystem_platform.dylib 0x7ffee7e9b790 _sigtramp + 
> 18446744071596723280
>   /Users/liuliu/Snapchat/Dev/phantom/6  clang-10 
> 0x00010bfdff14 
> clang::CodeGen::CodeGenTypes::arrangeObjCMethodDeclaration(clang::ObjCMethodDecl
>  const*) + 52
>   /Users/liuliu/Snapchat/Dev/phantom/7  clang-10 
> 0x00010c1f6bcd (anonymous 
> namespace)::CGObjCCommonMac::GenerateDirectMethod(clang::ObjCMethodDecl 
> const*, clang::ObjCContainerDecl const*) + 285
>   /Users/liuliu/Snapchat/Dev/phantom/8  clang-10 
> 0x00010c1f63e9 (anonymous 
> namespace)::CGObjCCommonMac::EmitMessageSend(clang::CodeGen::CodeGenFunction&,
>  clang::CodeGen::ReturnValueSlot, clang::QualType, clang::Selector, 
> llvm::Value*, clang::QualType, bool, clang::CodeGen::CallArgList const&, 
> clang::ObjCMethodDecl const*, clang::ObjCInterfaceDecl const*, (anonymous 
> namespace)::ObjCCommonTypesHelper const&) + 1401
>   /Users/liuliu/Snapchat/Dev/phantom/9  clang-10 
> 0x00010c205b14 (anonymous 
> namespace)::CGObjCNonFragileABIMac::GenerateMessageSend(clang::CodeGen::CodeGenFunction&,
>  clang::CodeGen::ReturnValueSlot, clang::QualType, clang::Selector, 
> llvm::Value*, clang::CodeGen::CallArgList const&, clang::ObjCInterfaceDecl 
> const*, clang::ObjCMethodDecl const*) + 644
>
>
> It indeed won't crash any more in trivial examples. I need to have some other 
> time to reduce my local example to a reasonable size.


This seems like the problem I just fixed, did you use the latest patch? because 
the problem was that we didn't bother to emit the implicit arguments for just 
prototypes before as it was only used by CodeGen of the implementations. This 
patch is supposed to add it to all declarations now, so I fail to understand 
how it can be missing anywhere ?


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

https://reviews.llvm.org/D71091



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

just added all the context this time (-W)




Comment at: clang/lib/CodeGen/CGObjCMac.cpp:4030
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD);
+  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
   if (I != DirectMethodDefinitions.end())

jfb wrote:
> I'd rather have `auto *` or `Decl *` here.
this is a pair this doesn't work


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

https://reviews.llvm.org/D71091



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 232602.
MadCoder marked 2 inline comments as done.

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

https://reviews.llvm.org/D71091

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/CodeGenObjC/direct-method.m


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -11,6 +11,7 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (int)getInt __attribute__((objc_direct));
 @end
 
 @implementation Root
@@ -173,3 +174,9 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHEC-LABEL: define i32 @useRoot
+  // CHECK: %call = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  return [r getInt];
+}
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4836,6 +4836,8 @@
 cast(ClassDecl)->addDecl(ObjCMethod);
   }
 
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+
   if (PrevMethod) {
 // You can never have two method definitions with the same name.
 Diag(ObjCMethod->getLocation(), diag::err_duplicate_method_decl)
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4027,7 +4027,7 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD);
+  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
   if (I != DirectMethodDefinitions.end())
 return I->second;
 
@@ -4040,7 +4040,7 @@
   llvm::Function *Method =
   llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
  Name.str(), &CGM.getModule());
-  DirectMethodDefinitions.insert(std::make_pair(OMD, Method));
+  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), 
Method));
 
   return Method;
 }


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -11,6 +11,7 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (int)getInt __attribute__((objc_direct));
 @end
 
 @implementation Root
@@ -173,3 +174,9 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHEC-LABEL: define i32 @useRoot
+  // CHECK: %call = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  return [r getInt];
+}
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4836,6 +4836,8 @@
 cast(ClassDecl)->addDecl(ObjCMethod);
   }
 
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+
   if (PrevMethod) {
 // You can never have two method definitions with the same name.
 Diag(ObjCMethod->getLocation(), diag::err_duplicate_method_decl)
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4027,7 +4027,7 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD);
+  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
   if (I != DirectMethodDefinitions.end())
 return I->second;
 
@@ -4040,7 +4040,7 @@
   llvm::Function *Method =
   llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
  Name.str(), &CGM.getModule());
-  DirectMethodDefinitions.insert(std::make_pair(OMD, Method));
+  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), Method));
 
   return Method;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 232491.
MadCoder edited the summary of this revision.

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

https://reviews.llvm.org/D71091

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/CodeGenObjC/direct-method.m


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -11,6 +11,7 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (int)getInt __attribute__((objc_direct));
 @end
 
 @implementation Root
@@ -173,3 +174,9 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHEC-LABEL: define i32 @useRoot
+  // CHECK: %call = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  return [r getInt];
+}
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4836,6 +4836,8 @@
 cast(ClassDecl)->addDecl(ObjCMethod);
   }
 
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+
   if (PrevMethod) {
 // You can never have two method definitions with the same name.
 Diag(ObjCMethod->getLocation(), diag::err_duplicate_method_decl)
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4027,7 +4027,7 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD);
+  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
   if (I != DirectMethodDefinitions.end())
 return I->second;
 
@@ -4040,7 +4040,7 @@
   llvm::Function *Method =
   llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
  Name.str(), &CGM.getModule());
-  DirectMethodDefinitions.insert(std::make_pair(OMD, Method));
+  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), 
Method));
 
   return Method;
 }


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -11,6 +11,7 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (int)getInt __attribute__((objc_direct));
 @end
 
 @implementation Root
@@ -173,3 +174,9 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHEC-LABEL: define i32 @useRoot
+  // CHECK: %call = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  return [r getInt];
+}
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4836,6 +4836,8 @@
 cast(ClassDecl)->addDecl(ObjCMethod);
   }
 
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+
   if (PrevMethod) {
 // You can never have two method definitions with the same name.
 Diag(ObjCMethod->getLocation(), diag::err_duplicate_method_decl)
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4027,7 +4027,7 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD);
+  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
   if (I != DirectMethodDefinitions.end())
 return I->second;
 
@@ -4040,7 +4040,7 @@
   llvm::Function *Method =
   llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
  Name.str(), &CGM.getModule());
-  DirectMethodDefinitions.insert(std::make_pair(OMD, Method));
+  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), Method));
 
   return Method;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

oh hah, thanks :)


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

https://reviews.llvm.org/D71091



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

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

Fix the fact that the hashmap of direct method was indexed by Declarations 
instead of names (and depending on code ordering, the declaration used at 
codegen time may be the one from the @interface or from the @implementation 
leading to name collisions and llvm "helpfully" adding `.1`'s everywhere


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

https://reviews.llvm.org/D71091

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/CodeGenObjC/direct-method.m


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -11,6 +11,7 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (int)getInt __attribute__((objc_direct));
 @end
 
 @implementation Root
@@ -173,3 +174,9 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHEC-LABEL: define i32 @useRoot
+  // CHECK: %call = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  return [r getInt];
+}
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4836,6 +4836,8 @@
 cast(ClassDecl)->addDecl(ObjCMethod);
   }
 
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+
   if (PrevMethod) {
 // You can never have two method definitions with the same name.
 Diag(ObjCMethod->getLocation(), diag::err_duplicate_method_decl)
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -876,7 +876,7 @@
 
   /// DirectMethodDefinitions - map of direct methods which have been defined 
in
   /// this translation unit.
-  llvm::DenseMap 
DirectMethodDefinitions;
+  llvm::DenseMap DirectMethodDefinitions;
 
   /// PropertyNames - uniqued method variable names.
   llvm::DenseMap PropertyNames;
@@ -4027,20 +4027,20 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD);
-  if (I != DirectMethodDefinitions.end())
-return I->second;
-
   SmallString<256> Name;
   GetNameForMethod(OMD, CD, Name);
 
+  auto I = DirectMethodDefinitions.find(Name);
+  if (I != DirectMethodDefinitions.end())
+return I->second;
+
   CodeGenTypes &Types = CGM.getTypes();
   llvm::FunctionType *MethodTy =
 Types.GetFunctionType(Types.arrangeObjCMethodDeclaration(OMD));
   llvm::Function *Method =
   llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
  Name.str(), &CGM.getModule());
-  DirectMethodDefinitions.insert(std::make_pair(OMD, Method));
+  DirectMethodDefinitions.insert(std::make_pair(Method->getName(), Method));
 
   return Method;
 }


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -11,6 +11,7 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (int)getInt __attribute__((objc_direct));
 @end
 
 @implementation Root
@@ -173,3 +174,9 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHEC-LABEL: define i32 @useRoot
+  // CHECK: %call = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  return [r getInt];
+}
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4836,6 +4836,8 @@
 cast(ClassDecl)->addDecl(ObjCMethod);
   }
 
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+
   if (PrevMethod) {
 // You can never have two method definitions with the same name.
 Diag(ObjCMethod->getLocation(), diag::err_duplicate_method_decl)
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -876,7 +876,7 @@
 
   /// DirectMethodDefinitions - map of direct methods which have been defined in
   /// this translation unit.
-  llvm::DenseMap DirectMethodDefinitions;
+  llvm::DenseMap DirectMethodDefinitions;
 
   /// PropertyNames - uniqued method variable names.
   llvm::DenseMap PropertyNames;
@@ -4027,20 +4027,20 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const Obj

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

hmm wait I have an old problem I had fixed creep up again :'(

need to fix it again sigh.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71091



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

turns out that I had no codegen check for the call site and that one of the 
last iteration broke it trivially :'(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71091



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder created this revision.
MadCoder added reviewers: rjmccall, arphaman, dexonsmith.
MadCoder added a project: clang.
Herald added a subscriber: cfe-commits.
MadCoder added a comment.

turns out that I had no codegen check for the call site and that one of the 
last iteration broke it trivially :'(


Radar-Id: rdar://problem/57661767


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71091

Files:
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/CodeGenObjC/direct-method.m


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -173,3 +173,9 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHEC-LABEL: define i32 @useRoot
+  // CHECK: %call = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  return [r getInt];
+}
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4836,6 +4836,10 @@
 cast(ClassDecl)->addDecl(ObjCMethod);
   }
 
+  if (ObjCMethod->isDirectMethod()) {
+ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+  }
+
   if (PrevMethod) {
 // You can never have two method definitions with the same name.
 Diag(ObjCMethod->getLocation(), diag::err_duplicate_method_decl)


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -173,3 +173,9 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHEC-LABEL: define i32 @useRoot
+  // CHECK: %call = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  return [r getInt];
+}
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4836,6 +4836,10 @@
 cast(ClassDecl)->addDecl(ObjCMethod);
   }
 
+  if (ObjCMethod->isDirectMethod()) {
+ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+  }
+
   if (PrevMethod) {
 // You can never have two method definitions with the same name.
 Diag(ObjCMethod->getLocation(), diag::err_duplicate_method_decl)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

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

Diff against previous is:

  diff
  diff --git a/clang/lib/CodeGen/CGObjCMac.cpp b/clang/lib/CodeGen/CGObjCMac.cpp
  index 814f67787dd..775e3406da7 100644
  --- a/clang/lib/CodeGen/CGObjCMac.cpp
  +++ b/clang/lib/CodeGen/CGObjCMac.cpp
  @@ -2159,21 +2159,23 @@ 
CGObjCCommonMac::EmitMessageSend(CodeGen::CodeGenFunction &CGF,
const ObjCInterfaceDecl *ClassReceiver,
const ObjCCommonTypesHelper &ObjCTypes) {
 CodeGenTypes &Types = CGM.getTypes();
  -  CallArgList ActualArgs;
  -  if (!IsSuper)
  -Arg0 = CGF.Builder.CreateBitCast(Arg0, ObjCTypes.ObjectPtrTy);
  -  ActualArgs.add(RValue::get(Arg0), Arg0Ty);
  +  auto selTy = CGF.getContext().getObjCSelType();
  +  llvm::Value *SelValue;
  +
 if (Method && Method->isDirectMethod()) {
   // Direct methods will synthesize the proper `_cmd` internally,
   // so just don't bother with setting the `_cmd` argument.
   assert(!IsSuper);
  -auto selTy = CGF.getContext().getObjCSelType();
  -auto undefSel = llvm::UndefValue::get(Types.ConvertType(selTy));
  -ActualArgs.add(RValue::get(undefSel), selTy);
  +SelValue = llvm::UndefValue::get(Types.ConvertType(selTy));
 } else {
  -ActualArgs.add(RValue::get(GetSelector(CGF, Sel)),
  -   CGF.getContext().getObjCSelType());
  +SelValue = GetSelector(CGF, Sel);
 }
  +
  +  CallArgList ActualArgs;
  +  if (!IsSuper)
  +Arg0 = CGF.Builder.CreateBitCast(Arg0, ObjCTypes.ObjectPtrTy);
  +  ActualArgs.add(RValue::get(Arg0), Arg0Ty);
  +  ActualArgs.add(RValue::get(SelValue), selTy);
 ActualArgs.addFrom(CallArgs);
  
 // If we're calling a method, use the formal signature.
  @@ -7622,7 +7624,7 @@ Address 
CGObjCNonFragileABIMac::EmitSelectorAddr(CodeGenFunction &CGF,
 llvm::ConstantExpr::getBitCast(GetMethodVarName(Sel),
ObjCTypes.SelectorPtrTy);
   std::string SectionName =
  -GetSectionName("__objc_selrefs", "literal_pointers");
  +GetSectionName("__objc_selrefs", "literal_pointers,no_dead_strip");
   Entry = new llvm::GlobalVariable(
   CGM.getModule(), ObjCTypes.SelectorPtrTy, false,
   getLinkageTypeForObjCMetadata(CGM, SectionName), Casted,

Basically, I emit the selref before emiting Arg0 as it used to, to avoid having 
to change all the tests.
I also put the "no_dead_strip" on selector sections back, as it's a remnant 
from an old iteration and this change is not needed for this per se (if we mean 
to do it it should be its own change).


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

https://reviews.llvm.org/D69991

Files:
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/ObjCRuntime.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclObjC.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.h
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/CodeGenObjC/direct-method.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/method-direct-properties.m
  clang/test/SemaObjC/method-direct.m

Index: clang/test/SemaObjC/method-direct.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct.m
@@ -0,0 +1,148 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+@protocol Proto
+- (void)protoMethod;  // expected-note {{previous declaration is here}}
++ (void)classProtoMethod; // expected-note {{previous declaration is here}}
+@end
+
+@protocol ProtoDirectFail
+- (void)protoMethod __attribute__((objc_direct));  // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
++ (void)classProtoMethod __attribute__((objc_direct)); // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
+@end
+
+__attribute__((objc_root_class))
+@interface Root
+- (void)rootRegular;  // expected-note {{previous declaration is here}}
++ (void)classRootRegular; // expected-note {{previous declaration is here}}
+- (void)rootDirect __attribute__((objc_direct));  // expected-note {{previous declaration is here}};
++ (void)classRootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
+- (void)otherRootDirect __a

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

when running the full test-suite before sending the patch, and it broke tests 
because some loads are now ordered differently :(
yay.

so I need to either make sure the CG is done in the same order again, or to fix 
the test expectations.


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

https://reviews.llvm.org/D69991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added inline comments.



Comment at: clang/lib/Sema/SemaExprObjC.cpp:1129
+  }
+}
+

rjmccall wrote:
> Is this a good idea to do for arbitrary diagnostics?  I feel like saying 
> "direct" in the note when that has nothing to do with the conflict could be a 
> little misleading.
fair enough. some of the mismatches will be because of directness so it can 
help in that instance but I can see how it's confusing.

fixed, and the fact that tests didn't need any updating probably proves you 
right ;)


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

https://reviews.llvm.org/D69991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 229410.
MadCoder marked 2 inline comments as done.
MadCoder added a comment.

reverted the hunk about "direct methods" note.


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

https://reviews.llvm.org/D69991

Files:
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/ObjCRuntime.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclObjC.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.h
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/CodeGenObjC/direct-method.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/method-direct-properties.m
  clang/test/SemaObjC/method-direct.m

Index: clang/test/SemaObjC/method-direct.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct.m
@@ -0,0 +1,148 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+@protocol Proto
+- (void)protoMethod;  // expected-note {{previous declaration is here}}
++ (void)classProtoMethod; // expected-note {{previous declaration is here}}
+@end
+
+@protocol ProtoDirectFail
+- (void)protoMethod __attribute__((objc_direct));  // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
++ (void)classProtoMethod __attribute__((objc_direct)); // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
+@end
+
+__attribute__((objc_root_class))
+@interface Root
+- (void)rootRegular;  // expected-note {{previous declaration is here}}
++ (void)classRootRegular; // expected-note {{previous declaration is here}}
+- (void)rootDirect __attribute__((objc_direct));  // expected-note {{previous declaration is here}};
++ (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)notDirectInIface; // expected-note {{previous declaration is here}}
++ (void)classNotDirectInIface;// expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct_members))
+@interface Root ()
+- (void)rootExtensionDirect;  // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect; // expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct_members))
+@interface Root(Direct)
+- (void)rootCategoryDirect;  // expected-note {{previous declaration is here}}
++ (void)classRootCategoryDirect; // expected-note {{previous declaration is here}}
+@end
+
+@interface Root ()
+- (void)rootExtensionRegular;   // expected-note {{previous declaration is here}}
++ (void)classRootExtensionRegular;  // expected-note {{previous declaration is here}}
+- (void)rootExtensionDirect2 __attribute__((objc_direct));  // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface Root (Direct2)
+- (void)rootCategoryRegular;   // expected-note {{previous declaration is here}}
++ (void)classRootCategoryRegular;  // expected-note {{previous declaration is here}}
+- (void)rootCategoryDirect2 __attribute__((objc_direct));  // expected-note {{previous declaration is here}}
++ (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}}
++ (void)classRootRegular __attribute__((objc_direct));  // expected-error {{methods that override superclass methods cannot be direct}}
+- (vo

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:3919
+in particular, this means that it cannot override a superclass method or 
satisfy
+a protocol requirement.
+

rjmccall wrote:
> Please add a new paragraph here:
> 
>   Because a direct method cannot be overridden, it is an error to perform
>   a ``super`` message send of one.
> 
> And you should test that.  (I noticed this because you had an 
> `assert(!IsSuper);` in IRGen, which was both a good idea and also begging for 
> a justification. :))
hah turns out I actually need to implement the Sema check for this :D



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:4087
+
+ReceiverCanBeNull = isWeakLinkedClass(OID);
+  }

rjmccall wrote:
> The assumption here is that a direct class method can never be invoked on a 
> nullable value, like a `Class`.  I think that's true, but it's worth making 
> that explicit in a comment.
Hmm actually I think that it's _not_ true. as in I'm not disallowing it today 
but I should be. I need to figure out how to do that, messaging a `Class` 
should be as disallowed as messaging `id`.

but right now it's not.


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

https://reviews.llvm.org/D69991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 229366.
MadCoder marked 7 inline comments as done.
MadCoder added a comment.

Updated for the new round of comments, with added tests and Sema checks errors 
for:

- messaging super
- messaging a nullable Class expression


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

https://reviews.llvm.org/D69991

Files:
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/ObjCRuntime.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclObjC.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.h
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/CodeGenObjC/direct-method.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/method-direct-properties.m
  clang/test/SemaObjC/method-direct.m

Index: clang/test/SemaObjC/method-direct.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct.m
@@ -0,0 +1,148 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+@protocol Proto
+- (void)protoMethod;  // expected-note {{previous declaration is here}}
++ (void)classProtoMethod; // expected-note {{previous declaration is here}}
+@end
+
+@protocol ProtoDirectFail
+- (void)protoMethod __attribute__((objc_direct));  // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
++ (void)classProtoMethod __attribute__((objc_direct)); // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
+@end
+
+__attribute__((objc_root_class))
+@interface Root
+- (void)rootRegular;  // expected-note {{previous declaration is here}}
++ (void)classRootRegular; // expected-note {{previous declaration is here}}
+- (void)rootDirect __attribute__((objc_direct));  // expected-note {{previous declaration is here}};
++ (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)notDirectInIface; // expected-note {{previous declaration is here}}
++ (void)classNotDirectInIface;// expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct_members))
+@interface Root ()
+- (void)rootExtensionDirect;  // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect; // expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct_members))
+@interface Root(Direct)
+- (void)rootCategoryDirect;  // expected-note {{previous declaration is here}}
++ (void)classRootCategoryDirect; // expected-note {{previous declaration is here}}
+@end
+
+@interface Root ()
+- (void)rootExtensionRegular;   // expected-note {{previous declaration is here}}
++ (void)classRootExtensionRegular;  // expected-note {{previous declaration is here}}
+- (void)rootExtensionDirect2 __attribute__((objc_direct));  // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface Root (Direct2)
+- (void)rootCategoryRegular;   // expected-note {{previous declaration is here}}
++ (void)classRootCategoryRegular;  // expected-note {{previous declaration is here}}
+- (void)rootCategoryDirect2 __attribute__((objc_direct));  // expected-note {{previous declaration is here}}
++ (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}}
++ (void)classRootRegular __attribute__((objc_direct))

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 229238.
MadCoder marked 2 inline comments as done.
MadCoder added a comment.

Implemented all the tests @rjmccall wanted (and then some)


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

https://reviews.llvm.org/D69991

Files:
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/ObjCRuntime.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclObjC.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.h
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/CodeGenObjC/direct-method.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/method-direct-properties.m
  clang/test/SemaObjC/method-direct.m

Index: clang/test/SemaObjC/method-direct.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct.m
@@ -0,0 +1,131 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+@protocol Proto
+- (void)protoMethod;  // expected-note {{previous declaration is here}}
++ (void)classProtoMethod; // expected-note {{previous declaration is here}}
+@end
+
+@protocol ProtoDirectFail
+- (void)protoMethod __attribute__((objc_direct));  // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
++ (void)classProtoMethod __attribute__((objc_direct)); // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
+@end
+
+__attribute__((objc_root_class))
+@interface Root
+- (void)rootRegular;  // expected-note {{previous declaration is here}}
++ (void)classRootRegular; // expected-note {{previous declaration is here}}
+- (void)rootDirect __attribute__((objc_direct));  // expected-note {{previous declaration is here}};
++ (void)classRootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
+- (void)notDirectInIface; // expected-note {{previous declaration is here}}
++ (void)classNotDirectInIface;// expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct_members))
+@interface Root ()
+- (void)rootExtensionDirect;  // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect; // expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct_members))
+@interface Root(Direct)
+- (void)rootCategoryDirect;  // expected-note {{previous declaration is here}}
++ (void)classRootCategoryDirect; // expected-note {{previous declaration is here}}
+@end
+
+@interface Root ()
+- (void)rootExtensionRegular;   // expected-note {{previous declaration is here}}
++ (void)classRootExtensionRegular;  // expected-note {{previous declaration is here}}
+- (void)rootExtensionDirect2 __attribute__((objc_direct));  // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface Root (Direct2)
+- (void)rootCategoryRegular;   // expected-note {{previous declaration is here}}
++ (void)classRootCategoryRegular;  // expected-note {{previous declaration is here}}
+- (void)rootCategoryDirect2 __attribute__((objc_direct));  // expected-note {{previous declaration is here}}
++ (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}}
++ (void)classRootRegular __attribute__((objc_direct));  // expected-error {{methods that override superclass methods cannot be direct}}
+- (void)protoMethod __attribute__((objc_direct));   // expected-error {{methods that implement protocol requirements cannot be direct}}
++ (void)classProtoMethod __attribute__((objc_direct));  // expected-error {{methods that i

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder marked 3 inline comments as done.
MadCoder added inline comments.



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:4075
+
+  if (OMD->isClassMethod()) {
+const ObjCInterfaceDecl *OID = dyn_cast(CD);

rjmccall wrote:
> MadCoder wrote:
> > I suspect this function requires synthesizing some debug metadata but I 
> > have absolutely no idea how and what ;)
> I'm sure Adrian has thoughts about that.
@aprantl said that the debug info looked right to him


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

https://reviews.llvm.org/D69991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

In D69991#1744979 , @aprantl wrote:

> Is it intentional that the direct method names use the exact same symbol 
> namespace (`\01-[class message]`) as "real" Objective-C methods? Could that 
> be a problem? Should we use a slightly different naming scheme?


I don't have an answer here, real compiler folks should answer (ping @rjmccall 
?).

Two statements:

- I would desire for the backtraces in a crash report to still look similar so 
moving to `_-[class message]` (with the leading `_`) would be fine
- the name can't quite collide because you can't define this symbol as both 
direct and dynamic, so it's effectively always free

I can however see why it may or may not confuse the debugger


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

https://reviews.llvm.org/D69991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 229183.
MadCoder marked 2 inline comments as done.
MadCoder added a comment.

Beefed up the tests, addressed the `selfValue` related issue.

Also rolled back to the `ObjCDeclInterfaceDecl` cast in GenerateDirectMethod as 
it turns out that all callers pass this type rather than an `ObjCImplDecl` 
however it's never nil and always of that type so make this an assert instead.

Did a pass of clang-format to clean up style


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

https://reviews.llvm.org/D69991

Files:
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/ObjCRuntime.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclObjC.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.h
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/CodeGenObjC/direct-method.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/method-direct.m

Index: clang/test/SemaObjC/method-direct.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct.m
@@ -0,0 +1,139 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+@protocol Proto
+- (void)protoMethod;  // expected-note {{previous declaration is here}}
++ (void)classProtoMethod; // expected-note {{previous declaration is here}}
+@end
+
+@protocol ProtoDirectFail
+@property(nonatomic, direct) int protoProperty;// expected-error {{'objc_direct' attribute cannot be applied to properties declared in an Objective-C protocol}}
+- (void)protoMethod __attribute__((objc_direct));  // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
++ (void)classProtoMethod __attribute__((objc_direct)); // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
+@end
+
+__attribute__((objc_root_class))
+@interface Root
+@property(nonatomic, direct) int directOrNot; // expected-note {{previous declaration is here}}
+- (int)directOrNot;
+- (void)rootRegular;  // expected-note {{previous declaration is here}}
++ (void)classRootRegular; // expected-note {{previous declaration is here}}
+- (void)rootDirect __attribute__((objc_direct));  // expected-note {{previous declaration is here}};
++ (void)classRootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
+- (void)notDirectInIface; // expected-note {{previous declaration is here}}
++ (void)classNotDirectInIface;// expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct_members))
+@interface
+Root()
+- (void)rootExtensionDirect;  // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect; // expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct_members))
+@interface Root(Direct)
+- (void)rootCategoryDirect;  // expected-note {{previous declaration is here}}
++ (void)classRootCategoryDirect; // expected-note {{previous declaration is here}}
+@end
+
+@interface Root ()
+- (void)rootExtensionRegular;   // expected-note {{previous declaration is here}}
++ (void)classRootExtensionRegular;  // expected-note {{previous declaration is here}}
+- (void)rootExtensionDirect2 __attribute__((objc_direct));  // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface Root (Direct2)
+- (void)rootCategoryRegular;   // expected-note {{previous declaration is here}}
++ (void)classRootCategoryRegular;  // expected-note {{previous declaration is here}}
+- (void)rootCategoryDirect2 __attribute__((objc_direct));  // expected-note {{previous declaration is here}}
++ (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)root

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added inline comments.



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:4076
+  if (OMD->isClassMethod()) {
+const ObjCInterfaceDecl *OID = dyn_cast(CD);
+Selector SelfSel = GetNullarySelector("self", CGM.getContext());

rjmccall wrote:
> When would the method container ever be an interface?  This is running on a 
> method implementation; it should always be within an `ObjCImplDecl`.  (There 
> was a case where this wasn't true for accessors, but Adrian just fixed that; 
> regardless, you can definitely pass down an `ObjCImplDecl`.). From the 
> `ObjCImplDecl` you can extract the class interface, which can never be null.
so it turns out the `CD` is always a classInterface because that's how things 
are called, so I will instead make it a cast<>


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

https://reviews.llvm.org/D69991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

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

Updated `clang/test/Misc/pragma-attribute-supported-attributes-list.test` that 
I had forgotten.


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

https://reviews.llvm.org/D69991

Files:
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/ObjCRuntime.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclObjC.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.h
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/CodeGenObjC/direct-method.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/method-direct.m

Index: clang/test/SemaObjC/method-direct.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct.m
@@ -0,0 +1,127 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+@protocol Proto
+- (void)protoMethod; // expected-note {{previous declaration is here}}
++ (void)classProtoMethod; // expected-note {{previous declaration is here}}
+@end
+
+@protocol ProtoDirectFail
+@property (nonatomic, direct) int protoProperty; // expected-error {{'objc_direct' attribute cannot be applied to properties declared in an Objective-C protocol}}
+- (void)protoMethod __attribute__((objc_direct)); // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
++ (void)classProtoMethod __attribute__((objc_direct)); // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
+@end
+
+__attribute__((objc_root_class))
+@interface Root
+@property (nonatomic, direct) int directOrNot; // expected-note {{previous declaration is here}}
+- (int)directOrNot;
+- (void)rootRegular; // expected-note {{previous declaration is here}}
++ (void)classRootRegular; // expected-note {{previous declaration is here}}
+- (void)rootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
++ (void)classRootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
+- (void)notDirectInIface; // expected-note {{previous declaration is here}}
++ (void)classNotDirectInIface; // expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct_members))
+@interface Root ()
+- (void)rootExtensionDirect; // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect; // expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct_members))
+@interface Root (Direct)
+- (void)rootCategoryDirect; // expected-note {{previous declaration is here}}
++ (void)classRootCategoryDirect; // expected-note {{previous declaration is here}}
+@end
+
+@interface Root ()
+- (void)rootExtensionRegular; // expected-note {{previous declaration is here}}
++ (void)classRootExtensionRegular; // expected-note {{previous declaration is here}}
+- (void)rootExtensionDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface Root (Direct2)
+- (void)rootCategoryRegular; // expected-note {{previous declaration is here}}
++ (void)classRootCategoryRegular; // expected-note {{previous declaration is here}}
+- (void)rootCategoryDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
++ (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}}
++ (void)classRootRegular __attribute__((objc_direct)); // expected-error {{methods that override superclass methods cannot be direct}}
+- (void)protoMethod __attribute__((objc_direct)); // expected-error {{methods that implement protocol requirements cannot be direct}}
++ (void)classProtoMethod __attribute__((objc_direct)); // expected-error {{methods that implement protocol requirements cannot be direct}}
+- (void)rootExtensionRegular __attribu

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder marked an inline comment as done.
MadCoder added inline comments.



Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:3885
+{
+  // GNU runtime doesn't support direct calls at this time
+}

MadCoder wrote:
> rjmccall wrote:
> > This doesn't seem to be diagnosed in Sema.
> how should I do it, is there an example I can follow?
actually figured it out.


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

https://reviews.llvm.org/D69991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 229023.
MadCoder marked 8 inline comments as done.
MadCoder edited the summary of this revision.
MadCoder added a comment.

Updated the patch to restrict `objc_direct` to methods and use 
`objc_direct_members` for containers, and several diagnostics improvements 
(especially in the vicinity of properties and the GNU runtime + tests).

Addressed all of @rjmccall comments except the one about `selfValue` that I 
don't understand yet and the lldb related stuff.


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

https://reviews.llvm.org/D69991

Files:
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/ObjCRuntime.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclObjC.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.h
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/CodeGenObjC/direct-method.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/method-direct.m

Index: clang/test/SemaObjC/method-direct.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct.m
@@ -0,0 +1,127 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+@protocol Proto
+- (void)protoMethod; // expected-note {{previous declaration is here}}
++ (void)classProtoMethod; // expected-note {{previous declaration is here}}
+@end
+
+@protocol ProtoDirectFail
+@property (nonatomic, direct) int protoProperty; // expected-error {{'objc_direct' attribute cannot be applied to properties declared in an Objective-C protocol}}
+- (void)protoMethod __attribute__((objc_direct)); // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
++ (void)classProtoMethod __attribute__((objc_direct)); // expected-error {{'objc_direct' attribute cannot be applied to methods declared in an Objective-C protocol}}
+@end
+
+__attribute__((objc_root_class))
+@interface Root
+@property (nonatomic, direct) int directOrNot; // expected-note {{previous declaration is here}}
+- (int)directOrNot;
+- (void)rootRegular; // expected-note {{previous declaration is here}}
++ (void)classRootRegular; // expected-note {{previous declaration is here}}
+- (void)rootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
++ (void)classRootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
+- (void)notDirectInIface; // expected-note {{previous declaration is here}}
++ (void)classNotDirectInIface; // expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct_members))
+@interface Root ()
+- (void)rootExtensionDirect; // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect; // expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct_members))
+@interface Root (Direct)
+- (void)rootCategoryDirect; // expected-note {{previous declaration is here}}
++ (void)classRootCategoryDirect; // expected-note {{previous declaration is here}}
+@end
+
+@interface Root ()
+- (void)rootExtensionRegular; // expected-note {{previous declaration is here}}
++ (void)classRootExtensionRegular; // expected-note {{previous declaration is here}}
+- (void)rootExtensionDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface Root (Direct2)
+- (void)rootCategoryRegular; // expected-note {{previous declaration is here}}
++ (void)classRootCategoryRegular; // expected-note {{previous declaration is here}}
+- (void)rootCategoryDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
++ (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}}
++ (void)classRootRegular __attribute__((objc_direct)); // expected-error {{methods that override superclass methods ca

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder marked an inline comment as not done.
MadCoder added inline comments.



Comment at: clang/include/clang/Basic/AttrDocs.td:3909
+called directly. It can also be used on Objective-C Categories and Extensions,
+in which case it applies to all methods declared in such an ``@interface``.
+

rjmccall wrote:
> If this is still true (rather than being done with `objc_direct_members`),
> I feel it should be documented next to the statement at the end about
> `objc_direct_members` in order to make the contrasting use cases clear.
> But we should discuss whether this should be using `objc_direct_members`
> for that use case.
`objc_direct_members` is for the @implementation only right now. 
`objc_direct_members` but it could make sense to be used on `@interface` 
instead I agree. It would make sense.

EDIT: actually I quite like this, it is much cleaner. I'll work on updating the 
patch.



Comment at: clang/include/clang/Basic/AttrDocs.td:3912
+Properties can also be declared with the ``direct`` property attribute
+which causes the property getter and setter to be direct methods as well.
+

rjmccall wrote:
> "If an Objective-C property is declared with the ``direct`` property 
> attribute, then its getter and setter are declared to be direct unless they 
> are explicitly declared."
> 
> I assume that's correct w.r.t the treatment of explicit declarations of 
> getters and setters?
I would expect so, I shall have a test for it



Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:3885
+{
+  // GNU runtime doesn't support direct calls at this time
+}

rjmccall wrote:
> This doesn't seem to be diagnosed in Sema.
how should I do it, is there an example I can follow?



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:1588
 // interface, we cannot perform this check.
+   //
+   // Note that for direct methods, because objc_msgSend is skipped,

rjmccall wrote:
> MadCoder wrote:
> > doh I'll fix the tabs
> The indentation still seems wrong.
yeah I fixed it in my checkout but didn't update the diff here yet



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:4095
+Builder.CreateStore(result.getScalarVal(),
+CGF.GetAddrOfLocalVar(OMD->getSelfDecl()));
+

rjmccall wrote:
> We should also be changing `selfValue` so that the null check below will do 
> the right thing with e.g. subclasses of weak-linked classes, where IIUC the 
> input value might be non-null but the initialized class pointer might be null.
I'm not sure what you mean, wouldn't storing to 
`CGF.GetAddrOfLocalVar(OMD->getSelfDecl())` actually affect selfValue itself? 
I'm not 100% sure I understand what you mean here.


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

https://reviews.llvm.org/D69991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added inline comments.



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:2169
+// Direct methods will synthesize the proper `_cmd` internally,
+   // so just don't bother with setting the `_cmd` argument.
+assert(!IsSuper);

jeez will fix the indent here too


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

https://reviews.llvm.org/D69991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 229015.
MadCoder marked 16 inline comments as done and an inline comment as not done.
MadCoder added a comment.

Handled a bunch of comments (marked done).

will update again to implement the deeper requested change around 
`objc_direct_members` and some more unaddressed comments.


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

https://reviews.llvm.org/D69991

Files:
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclObjC.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.h
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/CodeGenObjC/direct-method.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/method-direct.m

Index: clang/test/SemaObjC/method-direct.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct.m
@@ -0,0 +1,120 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+@protocol Proto
+- (void)protoMethod; // expected-note {{previous declaration is here}}
++ (void)classProtoMethod; // expected-note {{previous declaration is here}}
+@end
+
+@protocol ProtoDirectFail
+- (void)protoMethod __attribute__((objc_direct)); // expected-error {{objc_direct attribute cannot be applied to methods declared in an Objective-C protocol}}
++ (void)classProtoMethod __attribute__((objc_direct)); // expected-error {{objc_direct attribute cannot be applied to methods declared in an Objective-C protocol}}
+@end
+
+__attribute__((objc_root_class))
+@interface Root
+- (void)rootRegular; // expected-note {{previous declaration is here}}
++ (void)classRootRegular; // expected-note {{previous declaration is here}}
+- (void)rootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
++ (void)classRootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
+- (void)notDirectInIface; // expected-note {{previous declaration is here}}
++ (void)classNotDirectInIface; // expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct))
+@interface Root ()
+- (void)rootExtensionDirect; // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect; // expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct))
+@interface Root (Direct)
+- (void)rootCategoryDirect; // expected-note {{previous declaration is here}}
++ (void)classRootCategoryDirect; // expected-note {{previous declaration is here}}
+@end
+
+@interface Root ()
+- (void)rootExtensionRegular; // expected-note {{previous declaration is here}}
++ (void)classRootExtensionRegular; // expected-note {{previous declaration is here}}
+- (void)rootExtensionDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
++ (void)classRootExtensionDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+@interface Root (Direct2)
+- (void)rootCategoryRegular; // expected-note {{previous declaration is here}}
++ (void)classRootCategoryRegular; // expected-note {{previous declaration is here}}
+- (void)rootCategoryDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
++ (void)classRootCategoryDirect2 __attribute__((objc_direct)); // expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_root_class, objc_direct)) // expected-error {{'objc_direct' attribute only applies to Objective-C methods and Objective-C containers}}
+@interface SubDirectFail : Root
+- (instancetype)init;
+@end
+
+@interface Sub : Root 
+/* invalid overrides with directs */
+- (void)rootRegular __attribute__((objc_direct)); // expected-error {{method overrides or implementing protocol conformance cannot be direct}}
++ (void)classRootRegular __attribute__((objc_direct)); // expected-error {{method overrides or implementing protocol conformance cannot be direct}}
+- (void)protoMethod __attribute__((objc_direct)); // expected-error {{method overrides or implementing protocol conformance cannot be direct}}
++ (void)classProtoMethod __attribute__((objc_direct)); // expected-error {{method overrides or implementing protocol conformance cannot be direct}}
+- (void)rootExtensionRegular __attribute__((objc_direct)); // expected-error {{method overrides or implementing protocol conformance cannot be direct}}
++ (void)classRootExtensionRegular __attribute__((objc_direct)); // expec

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

In D69991#1739456 , @aprantl wrote:

> Why doesn't this need an update in lib/Serialization, is there generic code 
> that handles all attributes?


I followed the patch made for `NonLazyClass` and it didn't touch 
`lib/Serialization` and None of the attribute names I know hit in there either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder marked 2 inline comments as done.
MadCoder added inline comments.



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:1588
 // interface, we cannot perform this check.
+   //
+   // Note that for direct methods, because objc_msgSend is skipped,

doh I'll fix the tabs



Comment at: clang/lib/CodeGen/CGObjCMac.cpp:4075
+
+  if (OMD->isClassMethod()) {
+const ObjCInterfaceDecl *OID = dyn_cast(CD);

I suspect this function requires synthesizing some debug metadata but I have 
absolutely no idea how and what ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69991



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69991: Implement __attribute__((objc_direct)), __attribute__((objc_direct_members))

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder created this revision.
MadCoder added reviewers: rjmccall, jfb, doug.gregor, dexonsmith.
Herald added subscribers: cfe-commits, kristof.beyls.
Herald added a project: clang.

__attribute__((objc_direct)) is an attribute on methods declaration and
implementation, categories, or extensions (for the latter two it applies
the attribute to all methods declared in the category/implementation),

A `direct` property specifier is added (@property(direct) type name)

These attributes / specifiers cause the method to have no associated
Objective-C metadata (for the property or the method itself), and the
calling convention to be a direct C function call.

The symbol for the method has enforced hidden visibility and such direct
calls are hence unreachable cross image. An explicit C function must be
made if so desired to wrap them.

The implicit `self` and `_cmd` arguments are preserved, however to
maintain compatibility with the usual `objc_msgSend` semantics,
3 fundamental precautions are taken:

1. for instance methods, `self` is nil-checked. On arm64 backends this 
typically adds a single instruction (cbz x0, ) to the codegen, for 
the vast majority of the cases when the return type is a scalar.

2. for class methods, because the class may not be realized/initialized yet, a 
call to `[self self]` is emitted. When the proper deployment target is used, 
this is optimized to `objc_opt_self(self)`.

  However, long term we might want to emit something better that the optimizer 
can reason about. When inlining kicks in, these calls aren't optimized away as 
the optimizer has no idea that a single call is really necessary.

3. the calling convention for the `_cmd` argument is changed: the caller leaves 
the second argument to the call undefined, and the selector is loaded inside 
the body when it's referenced only.

As far as error reporting goes, the compiler refuses:

- making any overloads direct,
- making an overload of a direct method,
- implementations marked as direct when the declaration in the interface isn't 
(the other way around is allowed, as the direct attribute is inherited from the 
declaration),
- marking methods required for protocol conformance as direct,
- messaging an unqualified `id` with a direct method,
- forming any @selector() expression with only direct selectors.

As warnings:

- any inconsistency of direct-related calling convention when @selector() or 
messaging is used,
- forming any @selector() expression with a possibly direct selector.

Lastly an `objc_direct_members` attribute is added that can decorate
`@implementation` blocks and causes methods only declared there (and in
no `@interface`) to be automatically direct.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69991

Files:
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclObjC.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.h
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/CodeGenObjC/direct-method.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/method-direct.m

Index: clang/test/SemaObjC/method-direct.m
===
--- /dev/null
+++ clang/test/SemaObjC/method-direct.m
@@ -0,0 +1,120 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -Wselector-type-mismatch %s
+
+@protocol Proto
+- (void)protoMethod; // expected-note {{previous declaration is here}}
++ (void)classProtoMethod; // expected-note {{previous declaration is here}}
+@end
+
+@protocol ProtoDirectFail
+- (void)protoMethod __attribute__((objc_direct)); // expected-error {{objc_direct attribute cannot be applied to methods declared in an Objective-C protocol}}
++ (void)classProtoMethod __attribute__((objc_direct)); // expected-error {{objc_direct attribute cannot be applied to methods declared in an Objective-C protocol}}
+@end
+
+__attribute__((objc_root_class))
+@interface Root
+- (void)rootRegular; // expected-note {{previous declaration is here}}
++ (void)classRootRegular; // expected-note {{previous declaration is here}}
+- (void)rootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
++ (void)classRootDirect __attribute__((objc_direct)); // expected-note {{previous declaration is here}};
+- (void)notDirectInIface; // expected-note {{previous declaration is here}}
++ (void)classNotDirectInIface; // expected-note {{previous declaration is here}}
+@end
+
+__attribute__((objc_direct))
+@interfac