Summary: | Make it easier to pass pointers to WTFCrashWithInfo. | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Lam <mark.lam> | ||||
Component: | JavaScriptCore | Assignee: | Mark Lam <mark.lam> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | benjamin, cdumez, cmarcelo, dbates, ews-watchlist, saam, webkit-bug-importer, ysuzuki | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Mark Lam
2019-08-20 18:19:34 PDT
Created attachment 376837 [details]
proposed patch.
Comment on attachment 376837 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=376837&action=review > Source/WTF/wtf/Assertions.h:625 > + static_assert(std::is_integral<T>::value || std::is_enum<T>::value || std::is_pointer<T>::value, "All types need to be bitwise_cast-able to integral type for logging"); seems like a bad name for a function that allows pointers. Comment on attachment 376837 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=376837&action=review r=me with comment too. > Source/WTF/wtf/Assertions.h:553 > +ALWAYS_INLINE uint64_t wtfCrashArg(T* arg) { return reinterpret_cast<uint64_t>(arg); } In ARM64_32, the pointer size is 32bit so we should cast it to 64bit by using static_cast. So, I suggest using `static_cast<uintptr_t>(arg)` here. Comment on attachment 376837 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=376837&action=review >> Source/WTF/wtf/Assertions.h:553 >> +ALWAYS_INLINE uint64_t wtfCrashArg(T* arg) { return reinterpret_cast<uint64_t>(arg); } > > In ARM64_32, the pointer size is 32bit so we should cast it to 64bit by using static_cast. > So, I suggest using `static_cast<uintptr_t>(arg)` here. Ah, no. reinterpret_cast<uintptr_t>(arg) is the way. Comment on attachment 376837 [details] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=376837&action=review >>> Source/WTF/wtf/Assertions.h:553 >>> +ALWAYS_INLINE uint64_t wtfCrashArg(T* arg) { return reinterpret_cast<uint64_t>(arg); } >> >> In ARM64_32, the pointer size is 32bit so we should cast it to 64bit by using static_cast. >> So, I suggest using `static_cast<uintptr_t>(arg)` here. > > Ah, no. reinterpret_cast<uintptr_t>(arg) is the way. Fixed. >> Source/WTF/wtf/Assertions.h:625 >> + static_assert(std::is_integral<T>::value || std::is_enum<T>::value || std::is_pointer<T>::value, "All types need to be bitwise_cast-able to integral type for logging"); > > seems like a bad name for a function that allows pointers. I'll rename it to isIntegralOrPointerType. Thanks for the reviews. Landed in r248930: <http://trac.webkit.org/r248930>. |