There were several things off here, both in the tests and the Cocoa.cpp change. Sorry for all the breakage.
I’ve updated https://reviews.llvm.org/D80150 <https://reviews.llvm.org/D80150%C2%A0to> to address these issues and written up a summary there of what went wrong. vedant > On May 20, 2020, at 9:23 AM, Eric Christopher <echri...@gmail.com> wrote: > > 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? > > On Wed, May 20, 2020, 2:07 AM Pavel Labath via Phabricator via lldb-commits > <lldb-commits@lists.llvm.org <mailto:lldb-commits@lists.llvm.org>> wrote: > labath added a comment. > > In D80150#2045364 <https://reviews.llvm.org/D80150#2045364 > <https://reviews.llvm.org/D80150#2045364>>, @vsk wrote: > > > @labath Agreed on all points, I've addressed the feedback in 82dbf4aca84 > > <https://reviews.llvm.org/rG82dbf4aca84ec889d0dc390674ff44e30441bcfd > > <https://reviews.llvm.org/rG82dbf4aca84ec889d0dc390674ff44e30441bcfd>> by > > moving "DataFormatters/Mock.h" to "Plugins/Language/ObjC/Utilities.h", and > > adding a separate LanguageObjCTests unit test. > > > Cool. Thanks. > > > > ================ > Comment at: lldb/unittests/DataFormatter/MockTests.cpp:30 > + // Can't convert the date_value to a time_t. > + EXPECT_EQ(formatDateValue(std::numeric_limits<time_t>::max() + 1), > + llvm::None); > ---------------- > vsk wrote: > > labath wrote: > > > 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` ? > > Yes, thank you! It looks like Eric caught this before I did. > Actually, thinking about that further, (for 64-bit `time_t`s), > `double(numeric_limits<time_t>::max())` is [[ https://godbolt.org/z/t3iSd7 > <https://godbolt.org/z/t3iSd7> | 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). > > > Repository: > rG LLVM Github Monorepo > > CHANGES SINCE LAST ACTION > https://reviews.llvm.org/D80150/new/ <https://reviews.llvm.org/D80150/new/> > > https://reviews.llvm.org/D80150 <https://reviews.llvm.org/D80150> > > > > _______________________________________________ > lldb-commits mailing list > lldb-commits@lists.llvm.org <mailto:lldb-commits@lists.llvm.org> > https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits > <https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits>
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
