from:"Alastair Houghton via Phabricator via cfe\-commits"

[PATCH] D135273: [Clang][ObjC] Add optionality to property attribute strings.

2022-10-05 Thread Alastair Houghton via Phabricator via cfe-commits

al45tair created this revision.
al45tair added reviewers: ahatanak, rjmccall, theraven.
Herald added a project: All.
al45tair requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Add a new attribute, "?", to the property attribute string for properties of 
protocols that are declared `@optional`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135273

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/CodeGenObjC/objc-asm-attribute-test.m


Index: clang/test/CodeGenObjC/objc-asm-attribute-test.m
===
--- clang/test/CodeGenObjC/objc-asm-attribute-test.m
+++ clang/test/CodeGenObjC/objc-asm-attribute-test.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -no-opaque-pointers -emit-llvm -triple x86_64-apple-darwin 
%s -o - | FileCheck %s
+// RUN: %clang_cc1 -no-opaque-pointers -Wno-objc-root-class -emit-llvm -triple 
x86_64-apple-darwin %s -o - | FileCheck %s
 // rdar://16462586
 
 __attribute__((objc_runtime_name("MySecretNamespace.Protocol")))
@@ -11,10 +11,17 @@
 @protocol Protocol2
 - (void) MethodP2;
 + (void) ClsMethodP2;
+
+@optional
+@property(retain) id optionalProp;
+
 @end
 
+@class Message;
+
 __attribute__((objc_runtime_name("MySecretNamespace.Protocol3")))
 @protocol Protocol3
+
 @end
 
 __attribute__((objc_runtime_name("MySecretNamespace.Message")))
@@ -56,9 +63,13 @@
 }
 
 // CHECK: @"OBJC_IVAR_$_MySecretNamespace.Message.MyIVAR" ={{.*}} global i64 0
+
 // CHECK: @"OBJC_CLASS_$_MySecretNamespace.Message" ={{.*}} global 
%struct._class_t
 // CHECK: @"OBJC_METACLASS_$_MySecretNamespace.Message" ={{.*}} global 
%struct._class_t
 
+// CHECK: @OBJC_PROP_NAME_ATTR_ = private unnamed_addr constant [13 x i8] 
c"optionalProp\00", section "__TEXT,__objc_methname,cstring_literals", align 1
+// CHECK-NEXT: @OBJC_PROP_NAME_ATTR_.11 = private unnamed_addr constant [7 x 
i8] c"T@,?,&\00", section "__TEXT,__objc_methname,cstring_literals", align 1
+
 // CHECK: private unnamed_addr constant [42 x i8] 
c"T@\22MySecretNamespace.Message\22,&,V_msgProp\00"
 // CHECK: private unnamed_addr constant [76 x i8] 
c"T@\22MySecretNamespace.Message\22,&,V_msgProtoProp\00"
 // CHECK: private unnamed_addr constant [50 x i8] 
c"T@\22\22,&,V_idProtoProp\00"
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -7828,6 +7828,7 @@
 /// kPropertyWeak = 'W'  // 'weak' property
 /// kPropertyStrong = 'P'// property GC'able
 /// kPropertyNonAtomic = 'N' // property non-atomic
+/// kPropertyOptional = '?'  // property optional
 /// };
 /// @endcode
 std::string
@@ -7853,6 +7854,10 @@
   // closely resembles encoding of ivars.
   getObjCEncodingForPropertyType(PD->getType(), S);
 
+  if (PD->isOptional()) {
+S += ",?";
+  }
+
   if (PD->isReadOnly()) {
 S += ",R";
 if (PD->getPropertyAttributes() & ObjCPropertyAttribute::kind_copy)


Index: clang/test/CodeGenObjC/objc-asm-attribute-test.m
===
--- clang/test/CodeGenObjC/objc-asm-attribute-test.m
+++ clang/test/CodeGenObjC/objc-asm-attribute-test.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -no-opaque-pointers -emit-llvm -triple x86_64-apple-darwin %s -o - | FileCheck %s
+// RUN: %clang_cc1 -no-opaque-pointers -Wno-objc-root-class -emit-llvm -triple x86_64-apple-darwin %s -o - | FileCheck %s
 // rdar://16462586
 
 __attribute__((objc_runtime_name("MySecretNamespace.Protocol")))
@@ -11,10 +11,17 @@
 @protocol Protocol2
 - (void) MethodP2;
 + (void) ClsMethodP2;
+
+@optional
+@property(retain) id optionalProp;
+
 @end
 
+@class Message;
+
 __attribute__((objc_runtime_name("MySecretNamespace.Protocol3")))
 @protocol Protocol3
+
 @end
 
 __attribute__((objc_runtime_name("MySecretNamespace.Message")))
@@ -56,9 +63,13 @@
 }
 
 // CHECK: @"OBJC_IVAR_$_MySecretNamespace.Message.MyIVAR" ={{.*}} global i64 0
+
 // CHECK: @"OBJC_CLASS_$_MySecretNamespace.Message" ={{.*}} global %struct._class_t
 // CHECK: @"OBJC_METACLASS_$_MySecretNamespace.Message" ={{.*}} global %struct._class_t
 
+// CHECK: @OBJC_PROP_NAME_ATTR_ = private unnamed_addr constant [13 x i8] c"optionalProp\00", section "__TEXT,__objc_methname,cstring_literals", align 1
+// CHECK-NEXT: @OBJC_PROP_NAME_ATTR_.11 = private unnamed_addr constant [7 x i8] c"T@,?,&\00", section "__TEXT,__objc_methname,cstring_literals", align 1
+
 // CHECK: private unnamed_addr constant [42 x i8] c"T@\22MySecretNamespace.Message\22,&,V_msgProp\00"
 // CHECK: private unnamed_addr constant [76 x i8] c"T@\22MySecretNamespace.Message\22,&,V_msgProtoProp\00"
 // CHECK: private unnamed_addr constant [50 x i8] c"T@\22\22,&,V_idProtoProp\00"
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -7828,6

[PATCH] D135273: [Clang][ObjC] Add optionality to property attribute strings.

2022-11-18 Thread Alastair Houghton via Phabricator via cfe-commits

al45tair updated this revision to Diff 476387.
al45tair marked 2 inline comments as done.
al45tair added a comment.

Updated following comments from @ahatanak.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135273

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/CodeGenObjC/objc-asm-attribute-test.m


Index: clang/test/CodeGenObjC/objc-asm-attribute-test.m
===
--- clang/test/CodeGenObjC/objc-asm-attribute-test.m
+++ clang/test/CodeGenObjC/objc-asm-attribute-test.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -no-opaque-pointers -emit-llvm -triple x86_64-apple-darwin 
%s -o - | FileCheck %s
+// RUN: %clang_cc1 -no-opaque-pointers -Wno-objc-root-class -emit-llvm -triple 
x86_64-apple-darwin %s -o - | FileCheck %s
 // rdar://16462586
 
 __attribute__((objc_runtime_name("MySecretNamespace.Protocol")))
@@ -11,6 +11,10 @@
 @protocol Protocol2
 - (void) MethodP2;
 + (void) ClsMethodP2;
+
+@optional
+@property(retain) id optionalProp;
+
 @end
 
 __attribute__((objc_runtime_name("MySecretNamespace.Protocol3")))
@@ -59,6 +63,10 @@
 // CHECK: @"OBJC_CLASS_$_MySecretNamespace.Message" ={{.*}} global 
%struct._class_t
 // CHECK: @"OBJC_METACLASS_$_MySecretNamespace.Message" ={{.*}} global 
%struct._class_t
 
+// CHECK: @OBJC_PROP_NAME_ATTR_ = private unnamed_addr constant [13 x i8] 
c"optionalProp\00"
+// CHECK-NEXT: @OBJC_PROP_NAME_ATTR_.11 = private unnamed_addr constant [7 x 
i8] c"T@,?,&\00"
+// CHECK: @"_OBJC_$_PROP_LIST_MySecretNamespace.Protocol2" ={{.*}} 
getelementptr inbounds ([13 x i8], [13 x i8]* @OBJC_PROP_NAME_ATTR_, i32 0, i32 
0),{{.*}} getelementptr inbounds ([7 x i8], [7 x i8]* @OBJC_PROP_NAME_ATTR_.11, 
i32 0, i32 0)
+
 // CHECK: private unnamed_addr constant [42 x i8] 
c"T@\22MySecretNamespace.Message\22,&,V_msgProp\00"
 // CHECK: private unnamed_addr constant [76 x i8] 
c"T@\22MySecretNamespace.Message\22,&,V_msgProtoProp\00"
 // CHECK: private unnamed_addr constant [50 x i8] 
c"T@\22\22,&,V_idProtoProp\00"
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -7865,6 +7865,7 @@
 /// kPropertyWeak = 'W'  // 'weak' property
 /// kPropertyStrong = 'P'// property GC'able
 /// kPropertyNonAtomic = 'N' // property non-atomic
+/// kPropertyOptional = '?'  // property optional
 /// };
 /// @endcode
 std::string
@@ -7890,6 +7891,9 @@
   // closely resembles encoding of ivars.
   getObjCEncodingForPropertyType(PD->getType(), S);
 
+  if (PD->isOptional())
+S += ",?";
+
   if (PD->isReadOnly()) {
 S += ",R";
 if (PD->getPropertyAttributes() & ObjCPropertyAttribute::kind_copy)


Index: clang/test/CodeGenObjC/objc-asm-attribute-test.m
===
--- clang/test/CodeGenObjC/objc-asm-attribute-test.m
+++ clang/test/CodeGenObjC/objc-asm-attribute-test.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -no-opaque-pointers -emit-llvm -triple x86_64-apple-darwin %s -o - | FileCheck %s
+// RUN: %clang_cc1 -no-opaque-pointers -Wno-objc-root-class -emit-llvm -triple x86_64-apple-darwin %s -o - | FileCheck %s
 // rdar://16462586
 
 __attribute__((objc_runtime_name("MySecretNamespace.Protocol")))
@@ -11,6 +11,10 @@
 @protocol Protocol2
 - (void) MethodP2;
 + (void) ClsMethodP2;
+
+@optional
+@property(retain) id optionalProp;
+
 @end
 
 __attribute__((objc_runtime_name("MySecretNamespace.Protocol3")))
@@ -59,6 +63,10 @@
 // CHECK: @"OBJC_CLASS_$_MySecretNamespace.Message" ={{.*}} global %struct._class_t
 // CHECK: @"OBJC_METACLASS_$_MySecretNamespace.Message" ={{.*}} global %struct._class_t
 
+// CHECK: @OBJC_PROP_NAME_ATTR_ = private unnamed_addr constant [13 x i8] c"optionalProp\00"
+// CHECK-NEXT: @OBJC_PROP_NAME_ATTR_.11 = private unnamed_addr constant [7 x i8] c"T@,?,&\00"
+// CHECK: @"_OBJC_$_PROP_LIST_MySecretNamespace.Protocol2" ={{.*}} getelementptr inbounds ([13 x i8], [13 x i8]* @OBJC_PROP_NAME_ATTR_, i32 0, i32 0),{{.*}} getelementptr inbounds ([7 x i8], [7 x i8]* @OBJC_PROP_NAME_ATTR_.11, i32 0, i32 0)
+
 // CHECK: private unnamed_addr constant [42 x i8] c"T@\22MySecretNamespace.Message\22,&,V_msgProp\00"
 // CHECK: private unnamed_addr constant [76 x i8] c"T@\22MySecretNamespace.Message\22,&,V_msgProtoProp\00"
 // CHECK: private unnamed_addr constant [50 x i8] c"T@\22\22,&,V_idProtoProp\00"
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -7865,6 +7865,7 @@
 /// kPropertyWeak = 'W'  // 'weak' property
 /// kPropertyStrong = 'P'// property GC'able
 /// kPropertyNonAtomic = 'N' // property non-atomic
+/// kPropertyOptional = '?'  // property optional
 /// };
 /// @endcode
 std::string
@@ -7890,6

[PATCH] D135273: [Clang][ObjC] Add optionality to property attribute strings.

2022-11-18 Thread Alastair Houghton via Phabricator via cfe-commits

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



Comment at: clang/test/CodeGenObjC/objc-asm-attribute-test.m:20
 
+@class Message;
+

ahatanak wrote:
> Do you need this change?
No, I think this is left-over from an earlier revision. I'll get rid of it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135273

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

[PATCH] D135273: [Clang][ObjC] Add optionality to property attribute strings.

2022-11-18 Thread Alastair Houghton via Phabricator via cfe-commits

al45tair added a comment.

@theraven Any chance you could glance over this and reassure us that it isn't 
going to break the GNU runtime if we do this? (We're adding an extra attribute 
in the property attribute string so that we can detect `@optional` properties 
in ObjC protocols at runtime.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135273

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

[PATCH] D135273: [Clang][ObjC] Add optionality to property attribute strings.

2022-11-22 Thread Alastair Houghton via Phabricator via cfe-commits

al45tair added a comment.

In D135273#3942011 , @theraven wrote:

> In D135273#3936297 , @al45tair 
> wrote:
>
>> @theraven Any chance you could glance over this and reassure us that it 
>> isn't going to break the GNU runtime if we do this? (We're adding an extra 
>> attribute in the property attribute string so that we can detect `@optional` 
>> properties in ObjC protocols at runtime.)
>
> It shouldn't, we ignore any unknown property attributes.  I'd be more 
> concerned about code outside the runtime.  Lots of things parse encoding 
> strings badly, but the property APIs make it much easier to query known 
> attributes and so I think that's a lot lower risk than changing anything in 
> encoding strings.  It's a shame to use ?, since that is unknown type in type 
> encodings and that may confuse some parsers..

I don't think using `?` is really an issue here; anyone parsing types from 
these strings already has to stop for the `,` character, since the string is 
defined to be a `,`-separated list of attributes, the first of which is the 
type attribute `T`. If they were going to be confused by the 
`?`, then they'd already get confused by the other attribute characters, many 
of which are also valid in a type encoding.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135273

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

[PATCH] D135273: [Clang][ObjC] Add optionality to property attribute strings.

[PATCH] D135273: [Clang][ObjC] Add optionality to property attribute strings.

[PATCH] D135273: [Clang][ObjC] Add optionality to property attribute strings.

[PATCH] D135273: [Clang][ObjC] Add optionality to property attribute strings.

[PATCH] D135273: [Clang][ObjC] Add optionality to property attribute strings.

5 matches

Site Navigation

Mail list logo

Footer information