<div dir="auto">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><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">lldb-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">labath added a comment.<br>
<br>
In D80150#2045364 <<a href="https://reviews.llvm.org/D80150#2045364" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D80150#2045364</a>>, @vsk wrote:<br>
<br>
> @labath Agreed on all points, I've addressed the feedback in 82dbf4aca84 <<a href="https://reviews.llvm.org/rG82dbf4aca84ec889d0dc390674ff44e30441bcfd" rel="noreferrer noreferrer" target="_blank">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>
<br>
<br>
Cool. Thanks.<br>
<br>
<br>
<br>
================<br>
Comment at: lldb/unittests/DataFormatter/MockTests.cpp:30<br>
+  // Can't convert the date_value to a time_t.<br>
+  EXPECT_EQ(formatDateValue(std::numeric_limits<time_t>::max() + 1),<br>
+            llvm::None);<br>
----------------<br>
vsk wrote:<br>
> labath wrote:<br>
> > 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>
> Yes, thank you! It looks like Eric caught this before I did.<br>
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">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>
<br>
<br>
Repository:<br>
  rG LLVM Github Monorepo<br>
<br>
CHANGES SINCE LAST ACTION<br>
  <a href="https://reviews.llvm.org/D80150/new/" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D80150/new/</a><br>
<br>
<a href="https://reviews.llvm.org/D80150" rel="noreferrer noreferrer" target="_blank">https://reviews.llvm.org/D80150</a><br>
<br>
<br>
<br>
_______________________________________________<br>
lldb-commits mailing list<br>
<a href="mailto:lldb-commits@lists.llvm.org" target="_blank" rel="noreferrer">lldb-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits" rel="noreferrer noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits</a><br>
</blockquote></div>