[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza added inline comments.



Comment at: clang/include/clang/AST/DeclObjC.h:2181
 
+  /// This is true iff the protocol is tagged with the `objc_static_protocol`
+  /// attribute.

kastiglione wrote:
> This comment refers to the original spelling.
Yes indeed, I'll push a comment fix commit. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Dave Lee via Phabricator via cfe-commits]

kastiglione added inline comments.



Comment at: clang/include/clang/AST/DeclObjC.h:2181
 
+  /// This is true iff the protocol is tagged with the `objc_static_protocol`
+  /// attribute.

This comment refers to the original spelling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG14f6bfcb52e7: [clang] Implement objc_non_runtime_protocol to 
remove protocol metadata (authored by lanza).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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/lib/AST/DeclObjC.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/CodeGenObjC/non-runtime-protocol.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -118,6 +118,7 @@
 // CHECK-NEXT: ObjCExternallyRetained (SubjectMatchRule_variable_not_is_parameter, SubjectMatchRule_function, SubjectMatchRule_block, SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCMethodFamily (SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCNonLazyClass (SubjectMatchRule_objc_interface, SubjectMatchRule_objc_implementation)
+// CHECK-NEXT: ObjCNonRuntimeProtocol (SubjectMatchRule_objc_protocol)
 // CHECK-NEXT: ObjCPreciseLifetime (SubjectMatchRule_variable)
 // CHECK-NEXT: ObjCRequiresPropertyDefs (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCRequiresSuper (SubjectMatchRule_objc_method)
Index: clang/test/CodeGenObjC/non-runtime-protocol.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/non-runtime-protocol.m
@@ -0,0 +1,142 @@
+// RUN: not %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DPROTOEXPR -o - 2>&1 \
+// RUN: | FileCheck -check-prefix=PROTOEXPR %s
+
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DREDUNDANCY -o - \
+// RUN: | FileCheck -check-prefix=REDUNDANCY1 %s
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DREDUNDANCY -o - \
+// RUN: | FileCheck -check-prefix=REDUNDANCY2 %s
+
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=NONFRAGILE %s
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=INHERITANCE %s
+
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-apple-darwin -fobjc-runtime=macosx-fragile-10.5 %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=FRAGILE %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-apple-darwin -fobjc-runtime=macosx-fragile-10.5 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=FRAGILEINHERITANCE %s
+
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=GNU %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=GNUINHERITANCE %s
+//
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep-2 %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=GNU2 %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep-2 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=GNU2INHERITANCE %s
+
+__attribute__((objc_root_class))
+@interface Root
+@end
+@implementation Root
+@end
+
+#ifdef REDUNDANCY
+// REDUNDANCY1-NOT: _OBJC_CLASS_PROTOCOLS_$_Implementer{{.*}}_OBJC_PROTOCOL_$_B
+// REDUNDANCY2: _OBJC_CLASS_PROTOCOLS_$_Implementer{{.*}}_OBJC_PROTOCOL_$_C{{.*}}_OBJC_PROTOCOL_$_A
+@protocol C
+@end
+@protocol B 
+@end
+@protocol A 
+@end
+__attribute__((objc_non_runtime_protocol)) @protocol Alpha
+@end
+__attribute__((objc_non_runtime_protocol)) @protocol Beta
+@end
+@interface Implementer : Root 
+@end
+@implementation Implementer
+@end
+#endif
+
+#ifdef BASE
+// Confirm that we're not emitting protocol information for the
+// NONFRAGILE-NOT: OBJC_CLASS_NAME{{.*}}NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_$_PROTOCOL_INSTANCE_METHODS_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_$_PROTOCOL_CLASS_METHODS_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_PROTOCOL_$_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_LABEL_PROTOCOL_$_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_CLASS_PROTOCOLS_$_NonRuntimeImplementer
+// FRAGILE-NOT: OBJC_CLASS_NAME_.{{.*}}"Runtime\00"
+// FRAGILE-NOT: OBJC_PROTOCOL_NonRuntime
+// FRAGILE_NOT: OBJC_PROTOCOLS_NonRuntimeImplementer
+// 

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza updated this revision to Diff 295911.
lanza added a comment.

Clean clang-tidy warnings before landing


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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/lib/AST/DeclObjC.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/CodeGenObjC/non-runtime-protocol.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -118,6 +118,7 @@
 // CHECK-NEXT: ObjCExternallyRetained (SubjectMatchRule_variable_not_is_parameter, SubjectMatchRule_function, SubjectMatchRule_block, SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCMethodFamily (SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCNonLazyClass (SubjectMatchRule_objc_interface, SubjectMatchRule_objc_implementation)
+// CHECK-NEXT: ObjCNonRuntimeProtocol (SubjectMatchRule_objc_protocol)
 // CHECK-NEXT: ObjCPreciseLifetime (SubjectMatchRule_variable)
 // CHECK-NEXT: ObjCRequiresPropertyDefs (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCRequiresSuper (SubjectMatchRule_objc_method)
Index: clang/test/CodeGenObjC/non-runtime-protocol.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/non-runtime-protocol.m
@@ -0,0 +1,142 @@
+// RUN: not %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DPROTOEXPR -o - 2>&1 \
+// RUN: | FileCheck -check-prefix=PROTOEXPR %s
+
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DREDUNDANCY -o - \
+// RUN: | FileCheck -check-prefix=REDUNDANCY1 %s
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DREDUNDANCY -o - \
+// RUN: | FileCheck -check-prefix=REDUNDANCY2 %s
+
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=NONFRAGILE %s
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=INHERITANCE %s
+
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-apple-darwin -fobjc-runtime=macosx-fragile-10.5 %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=FRAGILE %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-apple-darwin -fobjc-runtime=macosx-fragile-10.5 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=FRAGILEINHERITANCE %s
+
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=GNU %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=GNUINHERITANCE %s
+//
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep-2 %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=GNU2 %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep-2 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=GNU2INHERITANCE %s
+
+__attribute__((objc_root_class))
+@interface Root
+@end
+@implementation Root
+@end
+
+#ifdef REDUNDANCY
+// REDUNDANCY1-NOT: _OBJC_CLASS_PROTOCOLS_$_Implementer{{.*}}_OBJC_PROTOCOL_$_B
+// REDUNDANCY2: _OBJC_CLASS_PROTOCOLS_$_Implementer{{.*}}_OBJC_PROTOCOL_$_C{{.*}}_OBJC_PROTOCOL_$_A
+@protocol C
+@end
+@protocol B 
+@end
+@protocol A 
+@end
+__attribute__((objc_non_runtime_protocol)) @protocol Alpha
+@end
+__attribute__((objc_non_runtime_protocol)) @protocol Beta
+@end
+@interface Implementer : Root 
+@end
+@implementation Implementer
+@end
+#endif
+
+#ifdef BASE
+// Confirm that we're not emitting protocol information for the
+// NONFRAGILE-NOT: OBJC_CLASS_NAME{{.*}}NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_$_PROTOCOL_INSTANCE_METHODS_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_$_PROTOCOL_CLASS_METHODS_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_PROTOCOL_$_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_LABEL_PROTOCOL_$_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_CLASS_PROTOCOLS_$_NonRuntimeImplementer
+// FRAGILE-NOT: OBJC_CLASS_NAME_.{{.*}}"Runtime\00"
+// FRAGILE-NOT: OBJC_PROTOCOL_NonRuntime
+// FRAGILE_NOT: OBJC_PROTOCOLS_NonRuntimeImplementer
+// GNU-NOT: private unnamed_addr constant {{.*}} c"NonRuntimeProtocol\00"
+// GNU-NOT: @.objc_protocol {{.*}}
+// GNU2-NOT: private unnamed_addr constant {{.*}} c"NonRuntimeProtocol\00"
+// GNU2-NOT: @.objc_protocol {{.*}}

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGObjC.cpp:487
+
+  // Walk both lists to get the full set of implied protocols
+  llvm::DenseSet AllImpliedProtocols;

lanza wrote:
> rjmccall wrote:
> > You should add something like ", including all the runtime protocols but 
> > not the non-runtime protocols".
> Do you anticipate a usage of `getImpliedProtocols` other than this algorithm? 
> I include the non-runtime protos in the all implied list simply because it 
> simplifies the collection. e.g. you insert iff it's a runtime protocol and if 
> not you have to check `contains` and then potentially add a non-runtime to 
> the work queue as many times as it's seen. 
> 
> Ultimately their inclusion in the all-implied list is meaningless because we 
> never attempt to insert a non-runtime protocol into the `RuntimePDs` list 
> anyways. So it's either extra elements in the `AllImplied` list or extra work 
> in the `getImpliedProtocols` method without any behavioral differences.
Ah, yes, that's a fair point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza updated this revision to Diff 295876.
lanza added a comment.

Comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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/lib/AST/DeclObjC.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/CodeGenObjC/non-runtime-protocol.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -118,6 +118,7 @@
 // CHECK-NEXT: ObjCExternallyRetained (SubjectMatchRule_variable_not_is_parameter, SubjectMatchRule_function, SubjectMatchRule_block, SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCMethodFamily (SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCNonLazyClass (SubjectMatchRule_objc_interface, SubjectMatchRule_objc_implementation)
+// CHECK-NEXT: ObjCNonRuntimeProtocol (SubjectMatchRule_objc_protocol)
 // CHECK-NEXT: ObjCPreciseLifetime (SubjectMatchRule_variable)
 // CHECK-NEXT: ObjCRequiresPropertyDefs (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCRequiresSuper (SubjectMatchRule_objc_method)
Index: clang/test/CodeGenObjC/non-runtime-protocol.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/non-runtime-protocol.m
@@ -0,0 +1,142 @@
+// RUN: not %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DPROTOEXPR -o - 2>&1 \
+// RUN: | FileCheck -check-prefix=PROTOEXPR %s
+
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DREDUNDANCY -o - \
+// RUN: | FileCheck -check-prefix=REDUNDANCY1 %s
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DREDUNDANCY -o - \
+// RUN: | FileCheck -check-prefix=REDUNDANCY2 %s
+
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=NONFRAGILE %s
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=INHERITANCE %s
+
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-apple-darwin -fobjc-runtime=macosx-fragile-10.5 %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=FRAGILE %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-apple-darwin -fobjc-runtime=macosx-fragile-10.5 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=FRAGILEINHERITANCE %s
+
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=GNU %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=GNUINHERITANCE %s
+//
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep-2 %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=GNU2 %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep-2 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=GNU2INHERITANCE %s
+
+__attribute__((objc_root_class))
+@interface Root
+@end
+@implementation Root
+@end
+
+#ifdef REDUNDANCY
+// REDUNDANCY1-NOT: _OBJC_CLASS_PROTOCOLS_$_Implementer{{.*}}_OBJC_PROTOCOL_$_B
+// REDUNDANCY2: _OBJC_CLASS_PROTOCOLS_$_Implementer{{.*}}_OBJC_PROTOCOL_$_C{{.*}}_OBJC_PROTOCOL_$_A
+@protocol C
+@end
+@protocol B 
+@end
+@protocol A 
+@end
+__attribute__((objc_non_runtime_protocol)) @protocol Alpha
+@end
+__attribute__((objc_non_runtime_protocol)) @protocol Beta
+@end
+@interface Implementer : Root 
+@end
+@implementation Implementer
+@end
+#endif
+
+#ifdef BASE
+// Confirm that we're not emitting protocol information for the
+// NONFRAGILE-NOT: OBJC_CLASS_NAME{{.*}}NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_$_PROTOCOL_INSTANCE_METHODS_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_$_PROTOCOL_CLASS_METHODS_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_PROTOCOL_$_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_LABEL_PROTOCOL_$_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_CLASS_PROTOCOLS_$_NonRuntimeImplementer
+// FRAGILE-NOT: OBJC_CLASS_NAME_.{{.*}}"Runtime\00"
+// FRAGILE-NOT: OBJC_PROTOCOL_NonRuntime
+// FRAGILE_NOT: OBJC_PROTOCOLS_NonRuntimeImplementer
+// GNU-NOT: private unnamed_addr constant {{.*}} c"NonRuntimeProtocol\00"
+// GNU-NOT: @.objc_protocol {{.*}}
+// GNU2-NOT: private unnamed_addr constant {{.*}} c"NonRuntimeProtocol\00"
+// GNU2-NOT: @.objc_protocol {{.*}}
+__attribute__((objc_non_runtime_protocol))

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza added inline comments.



Comment at: clang/lib/CodeGen/CGObjC.cpp:487
+
+  // Walk both lists to get the full set of implied protocols
+  llvm::DenseSet AllImpliedProtocols;

rjmccall wrote:
> You should add something like ", including all the runtime protocols but not 
> the non-runtime protocols".
Do you anticipate a usage of `getImpliedProtocols` other than this algorithm? I 
include the non-runtime protos in the all implied list simply because it 
simplifies the collection. e.g. you insert iff it's a runtime protocol and if 
not you have to check `contains` and then potentially add a non-runtime to the 
work queue as many times as it's seen. 

Ultimately their inclusion in the all-implied list is meaningless because we 
never attempt to insert a non-runtime protocol into the `RuntimePDs` list 
anyways. So it's either extra elements in the `AllImplied` list or extra work 
in the `getImpliedProtocols` method without any behavioral differences.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Thanks, that's a lot better.  Just some minor suggestions.




Comment at: clang/lib/CodeGen/CGObjC.cpp:477
+  // If there are no non-runtime protocols then we can just stop now.
+  if (!NonRuntimePDs.size())
+return RuntimePds;

`empty()`, please



Comment at: clang/lib/CodeGen/CGObjC.cpp:487
+
+  // Walk both lists to get the full set of implied protocols
+  llvm::DenseSet AllImpliedProtocols;

You should add something like ", including all the runtime protocols but not 
the non-runtime protocols".



Comment at: clang/lib/CodeGen/CGObjC.cpp:499
+
+  for (const auto *PD : ResolvedProtos) {
+if (!AllImpliedProtocols.contains(PD)) {

Comment on this pass.



Comment at: clang/lib/CodeGen/CGObjC.cpp:501
+if (!AllImpliedProtocols.contains(PD)) {
+  const auto *Can = PD->getCanonicalDecl();
+  RuntimePds.push_back(Can);

PD is already canonical here (and if it weren't, looking in 
`AllImpliedProtocols` wouldn't work).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza requested review of this revision.
lanza marked an inline comment as done.
lanza added inline comments.



Comment at: clang/lib/CodeGen/CGObjC.cpp:490
+  llvm::UniqueVector FoundProtocols;
+  std::set DeclaredProtocols;
+

rjmccall wrote:
> You should use llvm::DenseSet for this.  But in general there are more sets 
> here than I think you really need, and you're building them eagerly when you 
> don't have to.  I would suggest:
> 
> 1. Walk the list of declared protocols, adding the runtime protocols to the 
> main result list and the first runtime protocols implied by the non-runtime 
> protocols to a different list.
> 
> 2. If there are any protocols in the first-implied list (which is unlikely), 
> build the implied-protocols set for all the protocols in the main list 
> (inclusive of the protocols there) and the first-implied list (exclusive).  
> It would be totally reasonable to add a method to make this easier: `void 
> ObjCProtocolDecl::getImpliedProtocols(llvm::DenseSet 
> &) const;`
> 
> 3. Add any protocols that are in the first-implied list but not the implied 
> set back to the main list.
> 
> Also, protocols can be redeclared, so you should make sure to map to the 
> canonical decl before adding them to a set (or UniqueVector).
Got ya, that should be much better for the 99.99% case where there are no 
non-runtime protocols. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza updated this revision to Diff 295729.
lanza added a comment.

Update with John's suggestions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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/lib/AST/DeclObjC.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/CodeGenObjC/non-runtime-protocol.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -118,6 +118,7 @@
 // CHECK-NEXT: ObjCExternallyRetained (SubjectMatchRule_variable_not_is_parameter, SubjectMatchRule_function, SubjectMatchRule_block, SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCMethodFamily (SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCNonLazyClass (SubjectMatchRule_objc_interface, SubjectMatchRule_objc_implementation)
+// CHECK-NEXT: ObjCNonRuntimeProtocol (SubjectMatchRule_objc_protocol)
 // CHECK-NEXT: ObjCPreciseLifetime (SubjectMatchRule_variable)
 // CHECK-NEXT: ObjCRequiresPropertyDefs (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCRequiresSuper (SubjectMatchRule_objc_method)
Index: clang/test/CodeGenObjC/non-runtime-protocol.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/non-runtime-protocol.m
@@ -0,0 +1,142 @@
+// RUN: not %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DPROTOEXPR -o - 2>&1 \
+// RUN: | FileCheck -check-prefix=PROTOEXPR %s
+
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DREDUNDANCY -o - \
+// RUN: | FileCheck -check-prefix=REDUNDANCY1 %s
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DREDUNDANCY -o - \
+// RUN: | FileCheck -check-prefix=REDUNDANCY2 %s
+
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=NONFRAGILE %s
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=INHERITANCE %s
+
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-apple-darwin -fobjc-runtime=macosx-fragile-10.5 %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=FRAGILE %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-apple-darwin -fobjc-runtime=macosx-fragile-10.5 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=FRAGILEINHERITANCE %s
+
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=GNU %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=GNUINHERITANCE %s
+//
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep-2 %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=GNU2 %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep-2 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=GNU2INHERITANCE %s
+
+__attribute__((objc_root_class))
+@interface Root
+@end
+@implementation Root
+@end
+
+#ifdef REDUNDANCY
+// REDUNDANCY1-NOT: _OBJC_CLASS_PROTOCOLS_$_Implementer{{.*}}_OBJC_PROTOCOL_$_B
+// REDUNDANCY2: _OBJC_CLASS_PROTOCOLS_$_Implementer{{.*}}_OBJC_PROTOCOL_$_C{{.*}}_OBJC_PROTOCOL_$_A
+@protocol C
+@end
+@protocol B 
+@end
+@protocol A 
+@end
+__attribute__((objc_non_runtime_protocol)) @protocol Alpha
+@end
+__attribute__((objc_non_runtime_protocol)) @protocol Beta
+@end
+@interface Implementer : Root 
+@end
+@implementation Implementer
+@end
+#endif
+
+#ifdef BASE
+// Confirm that we're not emitting protocol information for the
+// NONFRAGILE-NOT: OBJC_CLASS_NAME{{.*}}NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_$_PROTOCOL_INSTANCE_METHODS_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_$_PROTOCOL_CLASS_METHODS_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_PROTOCOL_$_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_LABEL_PROTOCOL_$_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_CLASS_PROTOCOLS_$_NonRuntimeImplementer
+// FRAGILE-NOT: OBJC_CLASS_NAME_.{{.*}}"Runtime\00"
+// FRAGILE-NOT: OBJC_PROTOCOL_NonRuntime
+// FRAGILE_NOT: OBJC_PROTOCOLS_NonRuntimeImplementer
+// GNU-NOT: private unnamed_addr constant {{.*}} c"NonRuntimeProtocol\00"
+// GNU-NOT: @.objc_protocol {{.*}}
+// GNU2-NOT: private unnamed_addr constant {{.*}} c"NonRuntimeProtocol\00"
+// GNU2-NOT: @.objc_protocol {{.*}}

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CGObjC.cpp:490
+  llvm::UniqueVector FoundProtocols;
+  std::set DeclaredProtocols;
+

You should use llvm::DenseSet for this.  But in general there are more sets 
here than I think you really need, and you're building them eagerly when you 
don't have to.  I would suggest:

1. Walk the list of declared protocols, adding the runtime protocols to the 
main result list and the first runtime protocols implied by the non-runtime 
protocols to a different list.

2. If there are any protocols in the first-implied list (which is unlikely), 
build the implied-protocols set for all the protocols in the main list 
(inclusive of the protocols there) and the first-implied list (exclusive).  It 
would be totally reasonable to add a method to make this easier: `void 
ObjCProtocolDecl::getImpliedProtocols(llvm::DenseSet 
&) const;`

3. Add any protocols that are in the first-implied list but not the implied set 
back to the main list.

Also, protocols can be redeclared, so you should make sure to map to the 
canonical decl before adding them to a set (or UniqueVector).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza added inline comments.



Comment at: clang/lib/CodeGen/CGObjC.cpp:466
+  for (; begin != end; ++begin)
+AppendFirstRuntimeProtocols(*begin, PDs);
+

rjmccall wrote:
> Should this make an effort to avoid declaring redundant bases?  e.g.
> 
> ```
> @protocol Base @end
> @protocol NonRuntime @end
> @protocol Runtime @end
> @interface MyClass  @end
> @implementation MyClass @end
> ```
> 
> Ideally `MyClass` only declares conformance to `Runtime` rather than 
> redundantly declaring conformance to `Base`, which I think you'd naturally 
> get from this algorithm.
You are right. I fixed this but excluded the case where the declaration 
explicitly lists in it's `<>` declaration that it does inherit from a protocol. 
e.g. In the test case:

```
@protocol C @endp
@protocol B  @end
@protocol A  @end
__attribute__((objc_non_runtime_protocol)) @protocol Alpha @end
__attribute__((objc_non_runtime_protocol)) @protocol Beta @end
@interface Implementer : Root  @end
@implementation Implementer @end
```

The non-runtime removing algorithm generates `A,B,C` as the list but B and C 
are both redundant. It makes sense to remove `B` for that reason but since the 
declaration explicitly mentions `C` I vote that it makes sense to leave it 
included as that's what one would expect from a `class_copyProtocolList`. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza marked 5 inline comments as done.
lanza added a comment.

Fixed and added a test under the `REDUNDANCY` prefix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza updated this revision to Diff 294207.
lanza added a comment.

Fix duplicate inheritance issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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/lib/AST/DeclObjC.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/CodeGenObjC/non-runtime-protocol.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -118,6 +118,7 @@
 // CHECK-NEXT: ObjCExternallyRetained (SubjectMatchRule_variable_not_is_parameter, SubjectMatchRule_function, SubjectMatchRule_block, SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCMethodFamily (SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCNonLazyClass (SubjectMatchRule_objc_interface, SubjectMatchRule_objc_implementation)
+// CHECK-NEXT: ObjCNonRuntimeProtocol (SubjectMatchRule_objc_protocol)
 // CHECK-NEXT: ObjCPreciseLifetime (SubjectMatchRule_variable)
 // CHECK-NEXT: ObjCRequiresPropertyDefs (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCRequiresSuper (SubjectMatchRule_objc_method)
Index: clang/test/CodeGenObjC/non-runtime-protocol.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/non-runtime-protocol.m
@@ -0,0 +1,142 @@
+// RUN: not %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DPROTOEXPR -o - 2>&1 \
+// RUN: | FileCheck -check-prefix=PROTOEXPR %s
+
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DREDUNDANCY -o - \
+// RUN: | FileCheck -check-prefix=REDUNDANCY1 %s
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DREDUNDANCY -o - \
+// RUN: | FileCheck -check-prefix=REDUNDANCY2 %s
+
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=NONFRAGILE %s
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=INHERITANCE %s
+
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-apple-darwin -fobjc-runtime=macosx-fragile-10.5 %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=FRAGILE %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-apple-darwin -fobjc-runtime=macosx-fragile-10.5 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=FRAGILEINHERITANCE %s
+
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=GNU %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=GNUINHERITANCE %s
+//
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep-2 %s -DBASE -o - \
+// RUN: | FileCheck -check-prefix=GNU2 %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep-2 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=GNU2INHERITANCE %s
+
+__attribute__((objc_root_class))
+@interface Root
+@end
+@implementation Root
+@end
+
+#ifdef REDUNDANCY
+// REDUNDANCY1-NOT: _OBJC_CLASS_PROTOCOLS_$_Implementer{{.*}}_OBJC_PROTOCOL_$_B
+// REDUNDANCY2: _OBJC_CLASS_PROTOCOLS_$_Implementer{{.*}}_OBJC_PROTOCOL_$_A{{.*}}_OBJC_PROTOCOL_$_C
+@protocol C
+@end
+@protocol B 
+@end
+@protocol A 
+@end
+__attribute__((objc_non_runtime_protocol)) @protocol Alpha
+@end
+__attribute__((objc_non_runtime_protocol)) @protocol Beta
+@end
+@interface Implementer : Root 
+@end
+@implementation Implementer
+@end
+#endif
+
+#ifdef BASE
+// Confirm that we're not emitting protocol information for the
+// NONFRAGILE-NOT: OBJC_CLASS_NAME{{.*}}NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_$_PROTOCOL_INSTANCE_METHODS_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_$_PROTOCOL_CLASS_METHODS_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_PROTOCOL_$_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_LABEL_PROTOCOL_$_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_CLASS_PROTOCOLS_$_NonRuntimeImplementer
+// FRAGILE-NOT: OBJC_CLASS_NAME_.{{.*}}"Runtime\00"
+// FRAGILE-NOT: OBJC_PROTOCOL_NonRuntime
+// FRAGILE_NOT: OBJC_PROTOCOLS_NonRuntimeImplementer
+// GNU-NOT: private unnamed_addr constant {{.*}} c"NonRuntimeProtocol\00"
+// GNU-NOT: @.objc_protocol {{.*}}
+// GNU2-NOT: private unnamed_addr constant {{.*}} c"NonRuntimeProtocol\00"
+// GNU2-NOT: @.objc_protocol {{.*}}

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Okay, the result of internal review is that we're comfortable with this feature.

Reviewers brought up the point that it would be interesting to add some way to 
ensure unique emission of a protocol, so that protocols that can't be 
non-runtime could avoid bloating binary sizes outside the defining module.  
Maybe this could be deployment-gated so that builds targeting iOS 21 can take 
advantage of NSMoveOnlyType being exported but builds targeting iOS 20 still 
have to emit their own copy.  But that's a separable improvement and doesn't 
need to block this work.




Comment at: clang/include/clang/Basic/AttrDocs.td:4505
+intend to use protocols to implement compile time behaviors then the metadata 
is
+uneeded overhead.
+  }];

Suggestion:

  The ``objc_non_runtime_protocol`` attribute can be used to mark that an
  Objective-C protocol is only used during static type-checking and doesn't
  need to be represented dynamically.  This avoids several small code-size
  and run-time overheads associated with handling the protocol's metadata.
  A non-runtime protocol cannot be used as the operand of a ``@protocol``
  expression, and dynamic attempts to find it with ``objc_getProtocol`` will 
fail.

  If a non-runtime protocol inherits from any ordinary protocols, classes and
  derived protocols that declare conformance to the non-runtime protocol
  will dynamically list their conformance to those base protocols.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:1062
+  "non_runtime_protocol attribute on protocol %0 ignored (not implemented by 
this Objective-C runtime)">,
+  InGroup;
 def err_objc_direct_dynamic_property : Error<

This is dead now.



Comment at: clang/lib/CodeGen/CGObjC.cpp:466
+  for (; begin != end; ++begin)
+AppendFirstRuntimeProtocols(*begin, PDs);
+

Should this make an effort to avoid declaring redundant bases?  e.g.

```
@protocol Base @end
@protocol NonRuntime @end
@protocol Runtime @end
@interface MyClass  @end
@implementation MyClass @end
```

Ideally `MyClass` only declares conformance to `Runtime` rather than 
redundantly declaring conformance to `Base`, which I think you'd naturally get 
from this algorithm.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[David Chisnall via Phabricator via cfe-commits]

theraven accepted this revision.
theraven added a comment.
This revision is now accepted and ready to land.

Looks good to me, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza added a comment.

@theraven @rjmccall should be ready for review whenever you guys are ready!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza updated this revision to Diff 293944.
lanza added a comment.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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/lib/AST/DeclObjC.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGObjCGNU.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/CodeGen/CGObjCRuntime.h
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/CodeGenObjC/non-runtime-protocol.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -118,6 +118,7 @@
 // CHECK-NEXT: ObjCExternallyRetained (SubjectMatchRule_variable_not_is_parameter, SubjectMatchRule_function, SubjectMatchRule_block, SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCMethodFamily (SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCNonLazyClass (SubjectMatchRule_objc_interface, SubjectMatchRule_objc_implementation)
+// CHECK-NEXT: ObjCNonRuntimeProtocol (SubjectMatchRule_objc_protocol)
 // CHECK-NEXT: ObjCPreciseLifetime (SubjectMatchRule_variable)
 // CHECK-NEXT: ObjCRequiresPropertyDefs (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCRequiresSuper (SubjectMatchRule_objc_method)
Index: clang/test/CodeGenObjC/non-runtime-protocol.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/non-runtime-protocol.m
@@ -0,0 +1,118 @@
+// RUN: not %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DPROTOEXPR -o - 2>&1 \
+// RUN: | FileCheck -check-prefix=PROTOEXPR %s
+
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - \
+// RUN: | FileCheck -check-prefix=NONFRAGILE %s
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=INHERITANCE %s
+
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-apple-darwin -fobjc-runtime=macosx-fragile-10.5 %s -o - \
+// RUN: | FileCheck -check-prefix=FRAGILE %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-apple-darwin -fobjc-runtime=macosx-fragile-10.5 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=FRAGILEINHERITANCE %s
+
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep %s -o - \
+// RUN: | FileCheck -check-prefix=GNU %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=GNUINHERITANCE %s
+//
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep-2 %s -o - \
+// RUN: | FileCheck -check-prefix=GNU2 %s
+// RUN: %clang_cc1 -emit-llvm -triple x86_64-linux-gnu -fobjc-runtime=gnustep-2 %s -DINHERITANCE -o - \
+// RUN: | FileCheck -check-prefix=GNU2INHERITANCE %s
+
+__attribute__((objc_root_class))
+@interface Root
+@end
+@implementation Root
+@end
+
+#ifndef INHERITANCE
+// Confirm that we're not emitting protocol information for the
+// NONFRAGILE-NOT: OBJC_CLASS_NAME{{.*}}NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_$_PROTOCOL_INSTANCE_METHODS_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_$_PROTOCOL_CLASS_METHODS_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_PROTOCOL_$_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_LABEL_PROTOCOL_$_NonRuntimeProtocol
+// NONFRAGILE-NOT: _OBJC_CLASS_PROTOCOLS_$_NonRuntimeImplementer
+// FRAGILE-NOT: OBJC_CLASS_NAME_.{{.*}}"Runtime\00"
+// FRAGILE-NOT: OBJC_PROTOCOL_NonRuntime
+// FRAGILE_NOT: OBJC_PROTOCOLS_NonRuntimeImplementer
+// GNU-NOT: private unnamed_addr constant {{.*}} c"NonRuntimeProtocol\00"
+// GNU-NOT: @.objc_protocol {{.*}}
+// GNU2-NOT: private unnamed_addr constant {{.*}} c"NonRuntimeProtocol\00"
+// GNU2-NOT: @.objc_protocol {{.*}}
+__attribute__((objc_non_runtime_protocol))
+@protocol NonRuntimeProtocol
+- (void)doThing;
++ (void)doClassThing;
+@end
+// NONFRAGILE: @"_OBJC_METACLASS_RO_$_NonRuntimeImplementer" {{.*}} %struct._objc_protocol_list* null
+// NONFRAGILE: @"_OBJC_CLASS_RO_$_NonRuntimeImplementer" {{.*}} %struct._objc_protocol_list* null
+@interface NonRuntimeImplementer : Root 
+- (void)doThing;
++ (void)doClassThing;
+@end
+
+@implementation NonRuntimeImplementer
+- (void)doThing {
+}
++ (void)doClassThing {
+}
+@end
+
+void useNonRuntime(NonRuntimeImplementer *si) {
+  [si doThing];
+  [NonRuntimeImplementer doClassThing];
+
+#ifdef PROTOEXPR
+  // PROTOEXPR: cannot use a protocol declared 'objc_non_runtime_protocol' in a @protocol expression
+  Protocol *p = @protocol(NonRuntimeProtocol);
+#endif
+}
+#endif
+
+#ifdef INHERITANCE
+// Confirm 

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza added a comment.

> Hmm, I thought we actually just generated a bogus definition for the protocol 
> when it was forward-declared; really, this is better behavior that I 
> expected. Regardless, I don't think it's worthwhile to diagnose this more 
> strongly than a warning because of the history of not doing so.

That's fair!

> Not really important for this PR anyway. Was your question answered well 
> enough to move forward?

Yea sure, I'm okay with leaving it as a warning and moving forward and fixing 
your suggestions. I just wanted to make sure to cover any possible concerns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D75574#2262622 , @lanza wrote:

>> I don't think it'll actually error out at link time: protocol objects get 
>> emitted eagerly on use, cross-module linking is just a code-size 
>> optimization. This actually has caused longstanding problems.
>
> But if it's just a forward declaration there's nothing to emit. The above 
> code compiles just fine as is with just a warning. Here's the result of 
> `clang protocol.m -lobjc`
>
>   proto.m:10:31: warning: cannot find protocol definition for 
> 'NonRuntimeProto'
>   @interface Implementer : Root
> ^
>   proto.m:8:11: note: protocol 'NonRuntimeProto' has no definition
>   @protocol NonRuntimeProto;
> ^
>   1 warning generated.
>   Undefined symbols for architecture x86_64:
> "__OBJC_PROTOCOL_$_NonRuntimeProto", referenced from:
> __OBJC_CLASS_PROTOCOLS_$_Implementer in proto-bd4a43.o
>   ld: symbol(s) not found for architecture x86_64
>   clang-12: error: linker command failed with exit code 1 (use -v to see 
> invocation)
>
> The protocol definition isn't actually required to compile an implementation. 
> And if that protocol is declared as `objc_non_runtime_protocol` it won't ever 
> see one.
>
> Simply requiring that it is annotated accordingly also isn't satisfactory for 
> the same inheritance problem you mentioned above. Annotating a forward decl 
> can tell clang not to emit it but won't let clang know if there's a base 
> protocol that still needs to be emitted. e.g. if we have
>
>   // SomeHeader.h
>   @protocol Base
>   @end
>   __attribute__((objc_non_runtime_protocol))
>   @protocol NonRuntime 
>   @end
>   
>   
>   // and in main.m
>   __attribute__((objc_non_runtime_protocol))
>   @protocol NonRuntime
>   @interface AClass : Root
>   @end
>   @implementation AClass
>   @end
>
> we'll get a compile warning but no link error. But it will be wrong as the 
> protocol `Base` should still be in the protocollist for `AClass`.
>
> I'm not sure how big of an issue it would be introducing a new error here for 
> code that used to compile, but that's really the only way I see this working.

Hmm, I thought we actually just generated a bogus definition for the protocol 
when it was forward-declared; really, this is better behavior that I expected.  
Regardless, I don't think it's worthwhile to diagnose this more strongly than a 
warning because of the history of not doing so.

Not really important for this PR anyway.  Was your question answered well 
enough to move forward?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza added a comment.

> I don't think it'll actually error out at link time: protocol objects get 
> emitted eagerly on use, cross-module linking is just a code-size 
> optimization. This actually has caused longstanding problems.

But if it's just a forward declaration there's nothing to emit. The above code 
compiles just fine as is with just a warning. Here's the result of `clang 
protocol.m -lobjc`

  proto.m:10:31: warning: cannot find protocol definition for 'NonRuntimeProto'
  @interface Implementer : Root
^
  proto.m:8:11: note: protocol 'NonRuntimeProto' has no definition
  @protocol NonRuntimeProto;
^
  1 warning generated.
  Undefined symbols for architecture x86_64:
"__OBJC_PROTOCOL_$_NonRuntimeProto", referenced from:
__OBJC_CLASS_PROTOCOLS_$_Implementer in proto-bd4a43.o
  ld: symbol(s) not found for architecture x86_64
  clang-12: error: linker command failed with exit code 1 (use -v to see 
invocation)

The protocol definition isn't actually required to compile an implementation. 
And if that protocol is declared as `objc_non_runtime_protocol` it won't ever 
see one.

Simply requiring that it is annotated accordingly also isn't satisfactory for 
the same inheritance problem you mentioned above

  __attribute__((objc_non_runtime_protocol))
  @protocol SomeProto;

can tell clang not to emit it but won't let clang know if there's a base 
protocol that still needs to be emitted. e.g. if we have

  @protocol Base
  @end
  __attribute__((objc_non_runtime_protocol))
  @protocol SomeProto 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I don't think it'll actually error out at link time: protocol objects get 
emitted eagerly on use, cross-module linking is just a code-size optimization.  
This actually has caused longstanding problems.

Anyway, I think it's fine to require forward declarations to have the attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza added a comment.

A concern that has come up while rewriting this for the listed concerns is 
forward declared protocols that are defined as `non_runtime`.

  @protocol NonRuntimeProto;
  
  @interface Implementer : Root 
  @end
  
  @implementation Implementer
  ...
  @end

This compiles just fine but with a warning in clang 12. It'll emit a reference 
to the `NonRuntimeProto` without ever having had the chance to see the 
`objc_non_runtime_protocol` defined elsewhere and will thus error out at link.

There's a few ways to move forward with this:

1. Ignore it. If the user decided that a protocol was 
`objc_non_runtime_protocol` they probably have the insight necessary to address 
this link error. This is no more difficult than many C++ linker errors to 
diagnose.
2. Make it an error if the protocol is not defined prior to the implementation 
but leave forward-decls as valid for interfaces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D75574#2202136 , @theraven wrote:

> This feature looks generally useful.  A few small suggestions:
>
> - This is really a way of transforming a formal protocol into an informal 
> protocol.  Objective-C has had a convention of informal protocols since the 
> '80s, but they're implemented as categories on the root class with no 
> `@implementation`.  I'd suggest that 
> `__attribute__((objc_informal_protocol))` or similar might be a better 
> spelling for this, explicitly bringing the informal notion into the language. 
>  A lot of the informal protocols in Cocoa could be better expressed using 
> this and `@optional` methods than as categories on `NSObject`.

It's still a formal protocol, it just doesn't have runtime representation.  I 
think the name is appropriate.  It's an interesting point that some of the 
informal protocols could be formalized without penalty using this, though.

> - Given that this doesn't depend on any features in the runtime (from the 
> runtime's perspective, the protocol doesn't exist), I don't think it makes 
> sense to have an `ObjCRuntime` method to query whether this is supported by 
> the runtime.  We should enable it everywhere if it's going in anywhere.

Agreed.

> - The changes required in CGObjcCGNU.cpp are fairly small and I agree that 
> @rjmccall's proposal  for a callback-driven visitor would simplify the 
> changes in both runtimes.
> - The semantics are slightly confusing with the deep approach though.  
> Normally, if you iterate over the protocols that a class conforms to, you 
> only see the ones that it directly conforms to.  With this model, you'd see 
> indirect ones.  We might want to set some metadata to allow programmers to 
> differentiate the two, or we might want to have a warning (off by default?) 
> if an informal protocol conforms to a formal one, or simply disallow it.

I don't see why this information would be useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[David Chisnall via Phabricator via cfe-commits]

theraven added a comment.

This feature looks generally useful.  A few small suggestions:

- This is really a way of transforming a formal protocol into an informal 
protocol.  Objective-C has had a convention of informal protocols since the 
'80s, but they're implemented as categories on the root class with no 
`@implementation`.  I'd suggest that `__attribute__((objc_informal_protocol))` 
or similar might be a better spelling for this, explicitly bringing the 
informal notion into the language.  A lot of the informal protocols in Cocoa 
could be better expressed using this and `@optional` methods than as categories 
on `NSObject`.
- Given that this doesn't depend on any features in the runtime (from the 
runtime's perspective, the protocol doesn't exist), I don't think it makes 
sense to have an `ObjCRuntime` method to query whether this is supported by the 
runtime.  We should enable it everywhere if it's going in anywhere.
- The changes required in CGObjcCGNU.cpp are fairly small and I agree that 
@rjmccall's proposal  for a callback-driven visitor would simplify the changes 
in both runtimes.
- The semantics are slightly confusing with the deep approach though.  
Normally, if you iterate over the protocols that a class conforms to, you only 
see the ones that it directly conforms to.  With this model, you'd see indirect 
ones.  We might want to set some metadata to allow programmers to differentiate 
the two, or we might want to have a warning (off by default?) if an informal 
protocol conforms to a formal one, or simply disallow it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[John McCall via Phabricator via cfe-commits]

rjmccall added a reviewer: theraven.
rjmccall added a comment.

One thing that's come up so far: you generally need to be looking through 
non-runtime protocols, not ignoring them.  This matters when non-runtime 
protocols inherit from ordinary protocols.  It may be useful to provide a 
generic function that walks an array of protocols and calls a callback with the 
unique ordinary protocols it implies.

You should also implement this for non-Apple runtimes, which should be 
straightforward with that generic function; it's just a matter of testing it.  
CC'ing David Chisnall.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza added a comment.

No problem! Thank you, John!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Sorry, this slipped out of my mind.  I've started the process internally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza added a comment.

ping @rjmccall. Any update on a timeline for this review process? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Sorry, I should've been more straight with you.  I'm still happy to raise this 
internally, but this is a hectic time of the year at Apple, and I'm not 
actually going to raise it until after WWDC.  It wouldn't be making it into the 
new Xcode release anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574



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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza added a comment.

@rjmccall Hey John, I sent the proposal to the addresses I was pointed to but 
haven't heard back in multiple weeks. Any update on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574



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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza added a comment.

> If someone writes up a short proposal for this, with motivation and impact, 
> we'd be happy to present it internally.

I have a rough draft that I'll be touching up and submitting Monday most 
likely! Thanks, John!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574



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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

If someone writes up a short proposal for this, with motivation and impact, 
we'd be happy to present it internally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574



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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza updated this revision to Diff 252706.
lanza added a comment.

Rename and address some issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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/lib/AST/DeclObjC.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/CodeGenObjC/non-runtime-protocol.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -114,6 +114,7 @@
 // CHECK-NEXT: ObjCExternallyRetained (SubjectMatchRule_variable_not_is_parameter, SubjectMatchRule_function, SubjectMatchRule_block, SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCMethodFamily (SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCNonLazyClass (SubjectMatchRule_objc_interface, SubjectMatchRule_objc_implementation)
+// CHECK-NEXT: ObjCNonRuntimeProtocol (SubjectMatchRule_objc_protocol)
 // CHECK-NEXT: ObjCPreciseLifetime (SubjectMatchRule_variable)
 // CHECK-NEXT: ObjCRequiresPropertyDefs (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCRequiresSuper (SubjectMatchRule_objc_method)
Index: clang/test/CodeGenObjC/non-runtime-protocol.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/non-runtime-protocol.m
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - \
+// RUN: | FileCheck %s
+// RUN: not %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DPROTOEXPR -o - 2>&1 \
+// RUN: | FileCheck -check-prefix=PROTOEXPR %s
+
+__attribute__((objc_root_class))
+@interface Root
+@end
+@implementation Root
+@end
+
+// Confirm that we're not emitting protocol information for the
+// CHECK-NOT: OBJC_CLASS_NAME{{.*}}NonRuntimeProtocol
+// CHECK-NOT: _OBJC_$_PROTOCOL_INSTANCE_METHODS_NonRuntimeProtocol
+// CHECK-NOT: _OBJC_$_PROTOCOL_CLASS_METHODS_NonRuntimeProtocol
+// CHECK-NOT: _OBJC_PROTOCOL_$_NonRuntimeProtocol
+// CHECK-NOT: _OBJC_LABEL_PROTOCOL_$_NonRuntimeProtocol
+// CHECK-NOT: _OBJC_CLASS_PROTOCOLS_$_NonRuntimeImplementer
+// CHECK-NOT: @llvm.compiler.used {{.*}}NonRuntimeProtocol
+__attribute__((objc_non_runtime_protocol))
+@protocol NonRuntimeProtocol
+- (void)doThing;
++ (void)doClassThing;
+@end
+// CHECK: @"_OBJC_METACLASS_RO_$_NonRuntimeImplementer" {{.*}} %struct._objc_protocol_list* null
+// CHECK: @"_OBJC_CLASS_RO_$_NonRuntimeImplementer" {{.*}} %struct._objc_protocol_list* null
+@interface NonRuntimeImplementer : Root 
+- (void)doThing;
++ (void)doClassThing;
+@end
+
+@implementation NonRuntimeImplementer
+- (void)doThing {}
++ (void)doClassThing {}
+@end
+
+void useNonRuntime(NonRuntimeImplementer *si) {
+  [si doThing];
+  [NonRuntimeImplementer doClassThing];
+
+#ifdef PROTOEXPR
+// PROTOEXPR: can't use a protocol declared 'objc_non_runtime_protocol' in a @protocol expression
+  Protocol* p = @protocol(NonRuntimeProtocol);
+#endif
+}
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -1280,6 +1280,9 @@
 Diag(ProtoLoc, diag::err_undeclared_protocol) << ProtocolId;
 return true;
   }
+  if (PDecl->isNonRuntimeProtocol())
+Diag(ProtoLoc, diag::err_objc_non_runtime_protocol_in_protocol_expr)
+<< PDecl;
   if (!PDecl->hasDefinition()) {
 Diag(ProtoLoc, diag::err_atprotocol_protocol) << PDecl;
 Diag(PDecl->getLocation(), diag::note_entity_declared_at) << PDecl;
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -2603,6 +2603,15 @@
 D->addAttr(newAttr);
 }
 
+static void handleObjCNonRuntimeProtocolAttr(Sema , Decl *D,
+ const ParsedAttr ) {
+  if (S.getLangOpts().ObjCRuntime.allowsNonRuntimeProtocols()) {
+handleSimpleAttribute(S, D, AL);
+  } else {
+S.Diag(AL.getLoc(), diag::warn_objc_non_runtime_protocol_ignored) << AL;
+  }
+}
+
 static void handleObjCDirectAttr(Sema , Decl *D, const ParsedAttr ) {
   // objc_direct cannot be set on methods declared in the context of a protocol
   if (isa(D->getDeclContext())) {
@@ -7146,6 +7155,9 @@
   case ParsedAttr::AT_ObjCDirect:
 handleObjCDirectAttr(S, D, AL);
 break;
+  case ParsedAttr::AT_ObjCNonRuntimeProtocol:
+handleObjCNonRuntimeProtocolAttr(S, D, AL);
+break;
   case 

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza updated this revision to Diff 252707.
lanza added a comment.

Reword commit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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/lib/AST/DeclObjC.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/CodeGenObjC/non-runtime-protocol.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -114,6 +114,7 @@
 // CHECK-NEXT: ObjCExternallyRetained (SubjectMatchRule_variable_not_is_parameter, SubjectMatchRule_function, SubjectMatchRule_block, SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCMethodFamily (SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCNonLazyClass (SubjectMatchRule_objc_interface, SubjectMatchRule_objc_implementation)
+// CHECK-NEXT: ObjCNonRuntimeProtocol (SubjectMatchRule_objc_protocol)
 // CHECK-NEXT: ObjCPreciseLifetime (SubjectMatchRule_variable)
 // CHECK-NEXT: ObjCRequiresPropertyDefs (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCRequiresSuper (SubjectMatchRule_objc_method)
Index: clang/test/CodeGenObjC/non-runtime-protocol.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/non-runtime-protocol.m
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - \
+// RUN: | FileCheck %s
+// RUN: not %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DPROTOEXPR -o - 2>&1 \
+// RUN: | FileCheck -check-prefix=PROTOEXPR %s
+
+__attribute__((objc_root_class))
+@interface Root
+@end
+@implementation Root
+@end
+
+// Confirm that we're not emitting protocol information for the
+// CHECK-NOT: OBJC_CLASS_NAME{{.*}}NonRuntimeProtocol
+// CHECK-NOT: _OBJC_$_PROTOCOL_INSTANCE_METHODS_NonRuntimeProtocol
+// CHECK-NOT: _OBJC_$_PROTOCOL_CLASS_METHODS_NonRuntimeProtocol
+// CHECK-NOT: _OBJC_PROTOCOL_$_NonRuntimeProtocol
+// CHECK-NOT: _OBJC_LABEL_PROTOCOL_$_NonRuntimeProtocol
+// CHECK-NOT: _OBJC_CLASS_PROTOCOLS_$_NonRuntimeImplementer
+// CHECK-NOT: @llvm.compiler.used {{.*}}NonRuntimeProtocol
+__attribute__((objc_non_runtime_protocol))
+@protocol NonRuntimeProtocol
+- (void)doThing;
++ (void)doClassThing;
+@end
+// CHECK: @"_OBJC_METACLASS_RO_$_NonRuntimeImplementer" {{.*}} %struct._objc_protocol_list* null
+// CHECK: @"_OBJC_CLASS_RO_$_NonRuntimeImplementer" {{.*}} %struct._objc_protocol_list* null
+@interface NonRuntimeImplementer : Root 
+- (void)doThing;
++ (void)doClassThing;
+@end
+
+@implementation NonRuntimeImplementer
+- (void)doThing {}
++ (void)doClassThing {}
+@end
+
+void useNonRuntime(NonRuntimeImplementer *si) {
+  [si doThing];
+  [NonRuntimeImplementer doClassThing];
+
+#ifdef PROTOEXPR
+// PROTOEXPR: can't use a protocol declared 'objc_non_runtime_protocol' in a @protocol expression
+  Protocol* p = @protocol(NonRuntimeProtocol);
+#endif
+}
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -1280,6 +1280,9 @@
 Diag(ProtoLoc, diag::err_undeclared_protocol) << ProtocolId;
 return true;
   }
+  if (PDecl->isNonRuntimeProtocol())
+Diag(ProtoLoc, diag::err_objc_non_runtime_protocol_in_protocol_expr)
+<< PDecl;
   if (!PDecl->hasDefinition()) {
 Diag(ProtoLoc, diag::err_atprotocol_protocol) << PDecl;
 Diag(PDecl->getLocation(), diag::note_entity_declared_at) << PDecl;
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -2603,6 +2603,15 @@
 D->addAttr(newAttr);
 }
 
+static void handleObjCNonRuntimeProtocolAttr(Sema , Decl *D,
+ const ParsedAttr ) {
+  if (S.getLangOpts().ObjCRuntime.allowsNonRuntimeProtocols()) {
+handleSimpleAttribute(S, D, AL);
+  } else {
+S.Diag(AL.getLoc(), diag::warn_objc_non_runtime_protocol_ignored) << AL;
+  }
+}
+
 static void handleObjCDirectAttr(Sema , Decl *D, const ParsedAttr ) {
   // objc_direct cannot be set on methods declared in the context of a protocol
   if (isa(D->getDeclContext())) {
@@ -7146,6 +7155,9 @@
   case ParsedAttr::AT_ObjCDirect:
 handleObjCDirectAttr(S, D, AL);
 break;
+  case ParsedAttr::AT_ObjCNonRuntimeProtocol:
+handleObjCNonRuntimeProtocolAttr(S, D, AL);
+break;
   case 

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Manman Ren via Phabricator via cfe-commits]

manmanren added a comment.

In D75574#1925808 , @rjmccall wrote:

> This might also be interesting to @manmanren.


Thank you, John!

Manman


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574



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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[John McCall via Phabricator via cfe-commits]

rjmccall added a subscriber: manmanren.
rjmccall added a comment.

This might also be interesting to @manmanren.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574



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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza marked an inline comment as done.
lanza added a comment.

> Adding some more knowledgeable reviewers for comments on your RFC. I pointed 
> out a few minor nits, but I'll hold off on a technical review until the 
> ObjC-specific details are worked out and there is buy-in on the feature.

Thanks!

> This should be implemented via a custom LangOpt in Attr.td

This was done to more-or-less mirror the behavior from `objc_direct`. Though if 
people think the implementations should differ I'm cool with that.

In D75574#1911368 , @rjmccall wrote:

> I'm fine with people developing a proposal for this openly, but it needs to 
> be said that language changes cannot just be made in open-source; they have 
> to go through the official language review process, which for Objective-C is 
> an internal committee within Apple.


Yea sure, whatever the correct process is I'm more than happy to cooperate. I 
was going to post something on the mailing list some time this week, but this 
diff got attention early! What would be the best way to start that conversation?

> The summary calls this `objc_direct_protocol`, but it's 
> `objc_static_protocol` in the patch.  I agree that "direct" isn't a great 
> name for this.  `static` is complicated, because while we use "static" vs. 
> "dynamic" this way when we're talking *about* languages, I'm not sure any of 
> the umpteen language uses of `static` ever use it in exactly this way, and 
> it's possibly quite confusing to add one.  Throwing  out other names here: 
> `objc_non_runtime_protocol`? `objc_compiler_only_protocol`?

Yea, the naming I'm not too happy about and changed it around a few times. Your 
proposed naming is certainly more clear.

> The technical content of the patch seems fine.






Comment at: clang/include/clang/AST/DeclObjC.h:2197
 
+  /// True if the protocol is tagged as objc_static_protocol
+  bool isStaticProtocol() const;

aaron.ballman wrote:
> Comments should be properly punctuated (missing a full stop at the end of the 
> sentence), here and elsewhere.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574



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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I'm fine with people developing a proposal for this openly, but it needs to be 
said that language changes cannot just be made in open-source; they have to go 
through the official language review process, which for Objective-C is an 
internal committee within Apple.

The summary calls this `objc_direct_protocol`, but it's `objc_static_protocol` 
in the patch.  I agree that "direct" isn't a great name for this.  `static` is 
complicated, because while we use "static" vs. "dynamic" this way when we're 
talking *about* languages, I'm not sure any of the umpteen language uses of 
`static` ever use it in exactly this way, and it's possibly quite confusing to 
add one.  Throwing  out other names here: `objc_non_runtime_protocol`? 
`objc_compiler_only_protocol`?

The technical content of the patch seems fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574



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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: aaron.ballman, rjmccall, erik.pilkington, 
MadCoder.
aaron.ballman added a comment.
Herald added a subscriber: dexonsmith.

Adding some more knowledgeable reviewers for comments on your RFC. I pointed 
out a few minor nits, but I'll hold off on a technical review until the 
ObjC-specific details are worked out and there is buy-in on the feature.




Comment at: clang/include/clang/AST/DeclObjC.h:2197
 
+  /// True if the protocol is tagged as objc_static_protocol
+  bool isStaticProtocol() const;

Comments should be properly punctuated (missing a full stop at the end of the 
sentence), here and elsewhere.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:2609
+ const ParsedAttr ) {
+  if (S.getLangOpts().ObjCRuntime.allowsStaticProtocols()) {
+handleSimpleAttribute(S, D, AL);

This should be implemented via a custom `LangOpt` in `Attr.td`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574



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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[Nathan Lanza via Phabricator via cfe-commits]

lanza created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Motivated by the new objc_direct attribute, this change aims to limit
metadata generation from Protocols that the programmer knows isn't
going to be used at runtime. This attribute simply causes the frontend
to not generate any protocol metadata entries (e.g. OBJC_CLASS_NAME,
_OBJC_$_PROTOCOL_INSTANCE_METHDOS, _OBJC_PROTOCOL, etc) for a protocol
marked with `__attribute__((objc_direct))`.

There are a few APIs used to retrieve a protocol at runtime.
`@protocol(SomeProtocol)` will now error out of the requested protocol
is marked with attribute. `objc_getProtocol` will returl `NULL` which
is consistent with the behavior of a non-existing protocol.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75574

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/lib/AST/DeclObjC.cpp
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/test/CodeGenObjC/static-protocol.m

Index: clang/test/CodeGenObjC/static-protocol.m
===
--- /dev/null
+++ clang/test/CodeGenObjC/static-protocol.m
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -o - \
+// RUN: | FileCheck %s
+// RUN: not %clang_cc1 -emit-llvm -fobjc-arc -triple x86_64-apple-darwin10 %s -DPROTOEXPR -o - 2>&1 \
+// RUN: | FileCheck -check-prefix=PROTOEXPR %s
+
+__attribute__((objc_root_class))
+@interface Root
+@end
+@implementation Root
+@end
+
+// Confirm that we're not emitting protocol information for the
+// CHECK-NOT: OBJC_CLASS_NAME{{.*}}StaticProtocol
+// CHECK-NOT: _OBJC_$_PROTOCOL_INSTANCE_METHODS_StaticProtocol
+// CHECK-NOT: _OBJC_$_PROTOCOL_CLASS_METHODS_StaticProtocol
+// CHECK-NOT: _OBJC_PROTOCOL_$_StaticProtocol
+// CHECK-NOT: _OBJC_LABEL_PROTOCOL_$_StaticProtocol
+// CHECK-NOT: _OBJC_CLASS_PROTOCOLS_$_StaticImplementer
+// CHECK-NOT: @llvm.compiler.used {{.*}}StaticProtocol
+__attribute__((objc_static_protocol))
+@protocol StaticProtocol
+- (void)doThing;
++ (void)doClassThing;
+@end
+// CHECK: @"_OBJC_METACLASS_RO_$_StaticImplementer" {{.*}} %struct._objc_protocol_list* null
+// CHECK: @"_OBJC_CLASS_RO_$_StaticImplementer" {{.*}} %struct._objc_protocol_list* null
+@interface StaticImplementer : Root 
+- (void)doThing;
++ (void)doClassThing;
+@end
+
+@implementation StaticImplementer
+- (void)doThing {}
++ (void)doClassThing {}
+@end
+
+void useStatic(StaticImplementer *si) {
+  [si doThing];
+  [StaticImplementer doClassThing];
+
+#ifdef PROTOEXPR
+// PROTOEXPR: can't use a protocol declared 'objc_static_protocol' in a @protocol expression
+  Protocol* p = @protocol(StaticProtocol);
+#endif
+}
Index: clang/lib/Sema/SemaExprObjC.cpp
===
--- clang/lib/Sema/SemaExprObjC.cpp
+++ clang/lib/Sema/SemaExprObjC.cpp
@@ -1280,6 +1280,8 @@
 Diag(ProtoLoc, diag::err_undeclared_protocol) << ProtocolId;
 return true;
   }
+  if (PDecl->isStaticProtocol())
+Diag(ProtoLoc, diag::err_objc_static_protocol_in_protocol_expr) << PDecl;
   if (!PDecl->hasDefinition()) {
 Diag(ProtoLoc, diag::err_atprotocol_protocol) << PDecl;
 Diag(PDecl->getLocation(), diag::note_entity_declared_at) << PDecl;
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -2604,6 +2604,15 @@
 D->addAttr(newAttr);
 }
 
+static void handleObjCStaticProtocolAttr(Sema , Decl *D,
+ const ParsedAttr ) {
+  if (S.getLangOpts().ObjCRuntime.allowsStaticProtocols()) {
+handleSimpleAttribute(S, D, AL);
+  } else {
+S.Diag(AL.getLoc(), diag::warn_objc_static_protocol_ignored) << AL;
+  }
+}
+
 static void handleObjCDirectAttr(Sema , Decl *D, const ParsedAttr ) {
   // objc_direct cannot be set on methods declared in the context of a protocol
   if (isa(D->getDeclContext())) {
@@ -7095,6 +7104,9 @@
   case ParsedAttr::AT_ObjCDirect:
 handleObjCDirectAttr(S, D, AL);
 break;
+  case ParsedAttr::AT_ObjCStaticProtocol:
+handleObjCStaticProtocolAttr(S, D, AL);
+break;
   case ParsedAttr::AT_ObjCDirectMembers:
 handleObjCDirectMembersAttr(S, D, AL);
 handleSimpleAttribute(S, D, AL);
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -3031,7 +3031,8 @@
   // it now. Otherwise do nothing, the protocol objects are lazily
   // emitted.
   if (Protocols.count(PD->getIdentifier()))
-GetOrEmitProtocol(PD);
+if (!PD->isStaticProtocol())
+