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 > <https://github.com/AlexDenisov><nsvalue_literals.diff>_______________________________________________ > cfe-commits mailing list > [email protected] <mailto:[email protected]> > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
