<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">There were several things off here, both in the tests and the Cocoa.cpp change. Sorry for all the breakage.<div class=""><br class=""></div><div class="">I’ve updated <a href="https://reviews.llvm.org/D80150%C2%A0to" class="">https://reviews.llvm.org/D80150</a> to address these issues and written up a summary there of what went wrong.</div><div class=""><br class=""></div><div class="">vedant<br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On May 20, 2020, at 9:23 AM, Eric Christopher <<a href="mailto:echristo@gmail.com" class="">echristo@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="auto" class="">Agreed. Something is off here. My change was only to silence a few warnings, but they're definitely highlighting a conversion issue. What's up with NSDate conversions here. Does the API have a way to convert from time_t?</div><br class=""><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, May 20, 2020, 2:07 AM Pavel Labath via Phabricator via lldb-commits <<a href="mailto:lldb-commits@lists.llvm.org" class="">lldb-commits@lists.llvm.org</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">labath added a comment.<br class="">
<br class="">
In D80150#2045364 <<a href="https://reviews.llvm.org/D80150#2045364" rel="noreferrer noreferrer" target="_blank" class="">https://reviews.llvm.org/D80150#2045364</a>>, @vsk wrote:<br class="">
<br class="">
> @labath Agreed on all points, I've addressed the feedback in 82dbf4aca84 <<a href="https://reviews.llvm.org/rG82dbf4aca84ec889d0dc390674ff44e30441bcfd" rel="noreferrer noreferrer" target="_blank" class="">https://reviews.llvm.org/rG82dbf4aca84ec889d0dc390674ff44e30441bcfd</a>> by moving "DataFormatters/Mock.h" to "Plugins/Language/ObjC/Utilities.h", and adding a separate LanguageObjCTests unit test.<br class="">
<br class="">
<br class="">
Cool. Thanks.<br class="">
<br class="">
<br class="">
<br class="">
================<br class="">
Comment at: lldb/unittests/DataFormatter/MockTests.cpp:30<br class="">
+ // Can't convert the date_value to a time_t.<br class="">
+ EXPECT_EQ(formatDateValue(std::numeric_limits<time_t>::max() + 1),<br class="">
+ llvm::None);<br class="">
----------------<br class="">
vsk wrote:<br class="">
> labath wrote:<br class="">
> > Isn't this actually `std::numeric_limits<time_t>::min()` (and UB due to singed wraparound) ? Did you want to convert to double before doing the `+1` ?<br class="">
> Yes, thank you! It looks like Eric caught this before I did.<br class="">
Actually, thinking about that further, (for 64-bit `time_t`s), `double(numeric_limits<time_t>::max())` is [[ <a href="https://godbolt.org/z/t3iSd7" rel="noreferrer noreferrer" target="_blank" class="">https://godbolt.org/z/t3iSd7</a> | exactly the same value ]] as `double(numeric_limits<time_t>::max())+1.0` because `double` doesn't have enough bits to represent the value precisely. So, I have a feeling these checks are still not testing the exact thing you want to test (though I'm not sure what that is exactly).<br class="">
<br class="">
<br class="">
Repository:<br class="">
rG LLVM Github Monorepo<br class="">
<br class="">
CHANGES SINCE LAST ACTION<br class="">
<a href="https://reviews.llvm.org/D80150/new/" rel="noreferrer noreferrer" target="_blank" class="">https://reviews.llvm.org/D80150/new/</a><br class="">
<br class="">
<a href="https://reviews.llvm.org/D80150" rel="noreferrer noreferrer" target="_blank" class="">https://reviews.llvm.org/D80150</a><br class="">
<br class="">
<br class="">
<br class="">
_______________________________________________<br class="">
lldb-commits mailing list<br class="">
<a href="mailto:lldb-commits@lists.llvm.org" target="_blank" rel="noreferrer" class="">lldb-commits@lists.llvm.org</a><br class="">
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits" rel="noreferrer noreferrer" target="_blank" class="">https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits</a><br class="">
</blockquote></div>
</div></blockquote></div><br class=""></div></body></html>