labath added a comment.
I don't know how widespread the llrint behavior is. Clamping to the
largest/smallest int seems reasonable, so it might be reasonable to depend on
that -- though I don't know what will that do for NaNs...
I actually don't think the previous approach was wrong. IIUC, it only became a
problem when Eric added an explicit int cast to the input value -- it should
have been done the other way around, casting the integer to the float type.
Then we'd just need to be careful in the test to select a number that is
sufficiently bigger than UINT64_MAX, so that the difference is observable after
conversion to floating point.
In a way, I think that the real problem here is that we're using host floating
point arithmetic on numbers that are: a) untrusted; b) come from a system which
has potentially different floating point encoding/semantics. A more principled
approach would be to do it like the compiler, and work with APFloats with
semantics explicitly configured from based on the target.
================
Comment at: lldb/source/Plugins/Language/ObjC/Cocoa.cpp:796
+ static time_t epoch = 0;
+#ifdef __APPLE__
+ if (!epoch) {
----------------
I think that changing !_WIN32 to __APPLE__ is a step in the wrong direction.
Ideally, this function should work the same everywhere. The way to achieve that
would be to have a host function which selects between `gmtime` and `_mkgmtime`
depending on the platform. But failing that, I don't see a reason to restrict
the scope of this further.
================
Comment at: lldb/unittests/Language/AppleObjC/UtilitiesTests.cpp:43
+ // If the date_value `double` cannot be represented in a `time_t`, give up.
+ EXPECT_EQ(formatDateValue(greater_than_max_time), llvm::None);
+ EXPECT_EQ(formatDateValue(lesser_than_min_time), llvm::None);
----------------
What about infinities and NaNs? (I can believe this will do the right thing for
the former, but I have no clue what will happen in the NaN case).
================
Comment at: lldb/unittests/Language/CMakeLists.txt:4-6
+if (APPLE)
+ add_subdirectory(AppleObjC)
+endif()
----------------
I don't think the distinction between AppleObjC and ObjC is very clear, but if
the code under test lives in `ObjC`, I think it makes sense to call the test
folder the same way.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80150/new/
https://reviews.llvm.org/D80150
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
