On Nov 21, 2014, at 4:45 AM, AlexDenisov <[email protected]> wrote:

>> - It is important to have a complete IRGen tests for all the supported boxed 
>> items.
> 
> Could you, please, point me to the right direction? I’ve found only few tests 
> for literals under CodeGenObjC: boxing.m and objc-literal-tests.m. Should I 
> put new one there as well or just create another file?
> 
> 
The best one to look at is arc-literals.m. And it is good to produce the test 
under -fobjc-arc and without. 
Generally, you need to compile your test with -emit-llvm -o -  and save the 
output and then put the CHECKS
around the code you care enough (and make the register references generic, 
etc.).
It will take several iterations to get it right. 
Since you need many tests of a variety of APIs it is good to have a brand new 
test.

>> - CGPoint, CGSize, CGRect are iOS APIs only. But I haven’t been able to find 
>> their APIs in
>>   the later version of iPhone SDKs. If they have been removed recently, then 
>> we should make sure to check
>>   the deployment target and issue proper diagnostics if need be. This is 
>> generally the case for all the APIs.
>>   If we are going to use auto boxing, then we need to make sure the APIs 
>> available in earlier (or later) versions
>>   of the SDKs.
> 
> CG-structs are part of the CoreGraphics.framework. On OS X NSValue' factory 
> methods is a part of Foundation.framework (NSGeometry.h, 
> NSValueGeometryExtensions), while on iOS they’re part of UIKit (UIGeometry.h, 
> NSValueUIGeometryExtensions). I guess they’re not going to disappear. Bu, I 
> totally agree - deployment target should be checked for methods/structs, 
> which is unavailable at some target version (e.g. NSEdgeInsets, UIOffset).
> 
> 
Great thanks.

>> - Have you though of supporting other item types? 
>>   NSEdgeInsets (CGEdgeInsets for iOS?) macosx 10.10,  iPhone 8.0 
>>   non-retaining Objective-C objects.
>>   Any C pointers
> 
> I wasn’t sure if this feature will be accepted (and still don't), so decided 
> to sent patch only for these structs. But, yes, in my opinion support for the 
> whole ‘NSValueBoxable’ data types should be implemented.

OK.  While you are focusing on the structs these can wait.

> 
> 
> Also, I've realized later, there should be support for modernizer to convert 
> ‘old’ syntax to the new one.

Definitely, we should do that too. (I can help with that).

> I will add more tests, and, probably add other data types support, if there 
> are no objections.

I would wait on the other data type support until we have gone through the 
review process as it might impact implementation.

> 
> Thank you for feedback.

Thank you so much for working on this.
- Fariborz

> -- 
> AlexDenisov
> Software Engineer, https://github.com/AlexDenisov
> 
> On 20 Nov 2014 at 20:14:39, jahanian ([email protected]) wrote:
> 
>> While, the review process is ongoing, I looked at the patch. It is generally 
>> very good and closely follows the convention for
>> doing auto boxing of other types. I have few general comments to make.
>> 
>> - It is important to have a complete IRGen tests for all the supported boxed 
>> items.
>> 
>> - CGPoint, CGSize, CGRect are iOS APIs only. But I haven’t been able to find 
>> their APIs in
>>   the later version of iPhone SDKs. If they have been removed recently, then 
>> we should make sure to check
>>   the deployment target and issue proper diagnostics if need be. This is 
>> generally the case for all the APIs.
>>   If we are going to use auto boxing, then we need to make sure the APIs 
>> available in earlier (or later) versions
>>   of the SDKs.
>> 
>> - Have you though of supporting other item types? 
>>   NSEdgeInsets (CGEdgeInsets for iOS?) macosx 10.10,  iPhone 8.0 
>>   non-retaining Objective-C objects.
>>   Any C pointers
>> 
>>  One code comment: getNSValueFactoryMethod has two default arguments but do 
>> not have non-default value.
>>  
>> - Fariborz
>> 
>>> On Nov 16, 2014, at 10:15 AM, AlexDenisov <[email protected]> wrote:
>>> 
>>> Patch extends boxing expressions to support NSValue.
>>> 
>>> Some C structures might be boxed into a NSValue, e.g.: NSPoint, CGPoint, 
>>> NSRect, etc.
>>> This patch extends boxing expressions to accept the structures, that could 
>>> be used to construct NSValue object:
>>> 
>>> NSPoint p;
>>> NSValue *point = @(p);
>>> CGRect r;
>>> NSValue rect = @(r);
>>> 
>>> Full list of supported structures:
>>> NSPoint, NSSize, NSRect, CGPoint, CGSize, CGRect, NSRange.
>>> 
>>> -- 
>>> AlexDenisov
>>> Software Engineer, https://github.com/AlexDenisov
>>> <nsvalue_literals.diff>_______________________________________________
>>> cfe-commits mailing list
>>> [email protected]
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to