[Clang][ObjC] Add optionality to property attribute strings.

[al45tair]

Summary

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

[ahatanak]

This LGTM for apple's runtime assuming it knows how to handle the new string.

clang/lib/AST/ASTContext.cpp

7857

You can omit braces here.

clang/test/CodeGenObjC/objc-asm-attribute-test.m

20

Do you need this change?

72

Can you use regular expression for the two globals above and check they are referenced in @"_OBJC_$_PROP_LIST_MySecretNamespace.Protocol2?

[al45tair]

Updated following comments from @ahatanak.

[al45tair]

clang/test/CodeGenObjC/objc-asm-attribute-test.m

20

No, I think this is left-over from an earlier revision. I'll get rid of it.

[al45tair]

@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.)

[theraven]

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

[al45tair]

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<type encoding>. 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.

clang/lib/AST/ASTContext.cpp

 /// kPropertyWeak = 'W'              // 'weak' property
 /// kPropertyStrong = 'P'            // property GC'able
 /// kPropertyNonAtomic = 'N'         // property non-atomic
+/// kPropertyOptional = '?'          // property optional
 /// };
 /// @endcode
 std::string

   // closely resembles encoding of ivars.
   getObjCEncodingForPropertyType(PD->getType(), S);
 
+  if (PD->isOptional())
+    S += ",?";
+
   if (PD->isReadOnly()) {
     S += ",R";
     if (PD->getPropertyAttributes() & ObjCPropertyAttribute::kind_copy)

clang/test/CodeGenObjC/objc-asm-attribute-test.m

-// 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")))

 @protocol Protocol2
 - (void) MethodP2;
 + (void) ClsMethodP2;
+
+@optional
+@property(retain) id optionalProp;
+
 @end
 
 __attribute__((objc_runtime_name("MySecretNamespace.Protocol3")))

 // 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<MySecretNamespace.Protocol3>\22,&,V_msgProtoProp\00"
 // CHECK: private unnamed_addr constant [50 x i8] c"T@\22<MySecretNamespace.Protocol3>\22,&,V_idProtoProp\00"