<html><head><style>body{font-family:Helvetica,Arial;font-size:13px}</style></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div id="bloop_customfont" style="margin: 0px;"><div><div style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0);"><blockquote type="cite" class="clean_bq" style="color: rgb(0, 0, 0); font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div class="">- It is important to have a complete IRGen tests for all the supported boxed items.</div></div></blockquote></div><p style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0);">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?</p><div style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0);"><blockquote type="cite" class="clean_bq" style="color: rgb(0, 0, 0); font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div class="">- <font face="Helvetica, Arial" size="2" class="">CGPoint, CGSize, CGRect are iOS APIs only. But I haven’t been able to find their APIs in</font></div><div class=""><font face="Helvetica, Arial" size="2" class="">  the later version of iPhone SDKs. If they have been removed recently, then we should make sure to check</font></div><div class=""><font face="Helvetica, Arial" size="2" class="">  the deployment target and issue proper diagnostics if need be. This is generally the case for all the APIs.</font></div><div class=""><font face="Helvetica, Arial" size="2" class="">  If we are going to use auto boxing, then we need to make sure the APIs available in earlier (or later) versions</font></div><div class=""><font face="Helvetica, Arial" size="2" class="">  of the SDKs.</font></div></div></blockquote></div><p>CG-structs are part of the CoreGraphics.framework. On OS X NSValue' factory methods is a part of Foundation.framework (NSGeometry.h, <font face="Helvetica">NSValueGeometryExtensions</font>), while on iOS they’re part of UIKit (UIGeometry.h, <font face="Helvetica">NSValueUIGeometryExtensions</font>). 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. <font face="Helvetica">NSEdgeInsets, UIOffset</font>).</p><blockquote type="cite" class="clean_bq"><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div class=""><font face="Helvetica, Arial" size="2" class="">- Have you though of supporting other item types? </font></div><div class=""><font face="Helvetica, Arial" size="2" class="">  </font><span class="" style="font-family: Menlo; font-size: 11px;">NSEdgeInsets (</span>CGEdgeInsets for iOS?) macosx 10.10,  iPhone 8.0 </div><div class="">  non-retaining Objective-C objects.</div><div class="">  Any C pointers</div></div></blockquote><div style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0);"><br></div><div style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0);">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.</div><div style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0);"><br class="Apple-interchange-newline"></div></div><div style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0);"><br></div><div style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0);">Also, I've realized later, there should be support for modernizer to convert ‘old’ syntax to the new one.</div><div style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0);">I will add more tests, and, probably add other data types support, if there are no objections.</div><div style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0);"><br></div><div style="font-family: Helvetica, Arial; font-size: 13px; color: rgb(0, 0, 0);">Thank you for feedback.</div></div> <div id="bloop_sign_1416572057165234176" class="bloop_sign"><div style="font-family:helvetica,arial;font-size:13px">-- <br>AlexDenisov</div><div style="font-family:helvetica,arial;font-size:13px">Software Engineer, <a href="https://github.com/AlexDenisov">https://github.com/AlexDenisov</a></div></div> <br><p style="color:#000;">On 20 Nov 2014 at 20:14:39, jahanian (<a href="mailto:fjahanian@apple.com">fjahanian@apple.com</a>) wrote:</p> <blockquote type="cite" class="clean_bq"><span><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div></div><div>



<title></title>


While, the review process is ongoing, I looked at the patch. It is
generally very good and closely follows the convention for
<div class="">doing auto boxing of other types. I have few general
comments to make.</div>
<div class=""><br class=""></div>
<div class="">- It is important to have a complete IRGen tests for
all the supported boxed items.</div>
<div class=""><br class=""></div>
<div class="">- <font face="Helvetica, Arial" size="2" class="">CGPoint, CGSize, CGRect are iOS APIs only. But I haven’t been
able to find their APIs in</font></div>
<div class=""><font face="Helvetica, Arial" size="2" class="">  the later version of iPhone SDKs. If they have been
removed recently, then we should make sure to check</font></div>
<div class=""><font face="Helvetica, Arial" size="2" class="">  the deployment target and issue proper diagnostics
if need be. This is generally the case for all the
APIs.</font></div>
<div class=""><font face="Helvetica, Arial" size="2" class="">  If we are going to use auto boxing, then
we need to make sure the APIs available in earlier (or
later) versions</font></div>
<div class=""><font face="Helvetica, Arial" size="2" class="">  of the SDKs.</font></div>
<div class=""><font face="Helvetica, Arial" size="2" class=""><br class=""></font></div>
<div class=""><font face="Helvetica, Arial" size="2" class="">-
Have you though of supporting other item types? </font></div>
<div class=""><font face="Helvetica, Arial" size="2" class="">  </font><span style="font-family: Menlo; font-size: 11px;" class="">NSEdgeInsets
(</span>CGEdgeInsets for iOS?) macosx 10.10,  iPhone
8.0 </div>
<div class="">  non-retaining Objective-C objects.</div>
<div class="">  Any C pointers</div>
<div class=""><br class=""></div>
<div class=""> One code comment: getNSValueFactoryMethod
has two default arguments but do not have non-default value.</div>
<div class=""> </div>
<div class="">- Fariborz</div>
<div class=""><font face="Helvetica, Arial" size="2" class=""><br class=""></font></div>
<div class="">
<div>
<blockquote type="cite" class="">
<div class="">On Nov 16, 2014, at 10:15 AM, AlexDenisov
<<a href="mailto:1101.debian@gmail.com" class="">1101.debian@gmail.com</a>> wrote:</div>
<br class="Apple-interchange-newline">
<div class="">
<div id="bloop_customfont" style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; margin: 0px;" class="">
<div id="bloop_customfont" style="margin: 0px;" class="">Patch
extends boxing expressions to support NSValue.</div>
<div id="bloop_customfont" style="margin: 0px;" class=""><br class=""></div>
<div id="bloop_customfont" style="margin: 0px;" class="">Some C
structures might be boxed into a NSValue, e.g.: NSPoint, CGPoint,
NSRect, etc.</div>
<div id="bloop_customfont" style="margin: 0px;" class="">This patch
extends boxing expressions to accept the structures, that could be
used to construct NSValue object:</div>
<div id="bloop_customfont" style="margin: 0px;" class=""><br class=""></div>
<div id="bloop_customfont" style="margin: 0px;" class="">NSPoint
p;</div>
<div id="bloop_customfont" style="margin: 0px;" class="">NSValue
*point = @(p);</div>
<div id="bloop_customfont" style="margin: 0px;" class="">CGRect
r;</div>
<div id="bloop_customfont" style="margin: 0px;" class="">NSValue
rect = @(r);</div>
<div id="bloop_customfont" style="margin: 0px;" class=""><br class=""></div>
<div id="bloop_customfont" style="margin: 0px;" class="">Full list
of supported structures:</div>
<div id="bloop_customfont" style="margin: 0px;" class="">NSPoint,
NSSize, NSRect, CGPoint, CGSize, CGRect, NSRange.</div>
</div>
<br style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
<div id="bloop_sign_1416161623183923968" class="bloop_sign" style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">
<div style="font-family: helvetica, arial; font-size: 13px;" class="">-- <br class="">
AlexDenisov</div>
<div style="font-family: helvetica, arial; font-size: 13px;" class="">Software Engineer, <a href="https://github.com/AlexDenisov" class="">https://github.com/AlexDenisov</a></div>
</div>
<span id="cid:D1043764-A448-4BA1-A4F6-1231A9C5186B@apple.com"><nsvalue_literals.diff></span><span style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">_______________________________________________</span><br style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
<span style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">cfe-commits mailing list</span><br style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
<a href="mailto:cfe-commits@cs.uiuc.edu" style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">cfe-commits@cs.uiuc.edu</a><br style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" style="font-family: Helvetica, Arial; font-size: 13px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a></div>
</blockquote>
</div>
<br class=""></div>


</div></div></span></blockquote></body></html>