Bug 200960 – Make it easier to pass pointers to WTFCrashWithInfo.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Mark Lam]

Now, we don't have to explicitly cast them to uint64_ts first.  The template wrappers will take care of it for us.

[Mark Lam]

Created attachment 376837 [details]
proposed patch.

[Saam Barati]

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.

[Yusuke Suzuki]

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.

[Yusuke Suzuki]

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.

[Mark Lam]

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.

[Mark Lam]

Thanks for the reviews.  Landed in r248930: <http://trac.webkit.org/r248930>.

[Radar WebKit Bug Importer]

<rdar://problem/54538595>