793314 - Remove PtrBits

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch v1

There really is no point to this typedef. Mounir, could you do dom/ and content/; roc, could you do layout/?

Comment 1

[Masatoshi Kimura [:emk]]

>-  NS_ASSERTION(sizeof(PtrBits) == sizeof(void*),
>-               "BAD! You'll need to adjust the size of PtrBits to the size "
>-               "of a pointer on your platform.");
>+  MOZ_STATIC_ASSERT(sizeof(uintptr_t) == sizeof(void*),
>+                    "BAD! You'll need to adjust the size of uintptr_t to the "
>+                    "size of a pointer on your platform.");
Can we change the size of uintptr_t? Moreover, it is not guaranteed to be the same as the pointer size. Maybe we don't support such an exotic environment, but the message can be improved.

>-  NS_ASSERTION(sizeof(PtrBits) == sizeof(void *),
>-               "Eeek! You'll need to adjust the size of PtrBits to the size "
>-               "of a pointer on your platform.");
>+  MOZ_STATIC_ASSERT(sizeof(uintptr_t) == sizeof(void*),
>+                    "Eeek! You'll need to adjust the size of uintptr_t to the "
>+                    "size of a pointer on your platform.");
Same above.

Comment 2

[Mounir Lamouri (:mounir)]

Comment on attachment 663562 [details] [diff] [review]
Patch v1

Review of attachment 663562 [details] [diff] [review]:
-----------------------------------------------------------------

It seems that uintptr_t is in C++11 but not in C++03 but most compilers support it since a long time. I assume that's the case for compilers we care about? Given that we already have that typedef, I guess so...

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Whether we should on uintptr_t (I think we should) is irrevant to this bug, since via the typedef we already do. This patch just makes it more obvious.

Comment 4

[Masatoshi Kimura [:emk]]

We can change the typedef if sizeof(uintptr_t) != sizeof(void*) on some platforms, but we can't change uintptr_t itself. That said, our code would assume sizeof(uintptr_t) == sizeof(void*) in other places anyway.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

If sizeof(uintptr_t) != sizeof(void*) then that platform's definition of uintptr_t is completely broken, and so is our use of PtrBits/uintptr_t before this patch and after this patch.

If the MOZ_STATIC_ASSERT is ever hit, then we'll know that platform is broken.

I cannot see any problem here.

Comment 6

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/7e5ee5365db9
https://hg.mozilla.org/mozilla-central/rev/c9a8f55d8541