Status

()

Core
General
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Ms2ger, Assigned: Ms2ger)

Tracking

Trunk
mozilla18
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Created attachment 663562 [details] [diff] [review]
Patch v1

There really is no point to this typedef. Mounir, could you do dom/ and content/; roc, could you do layout/?
Attachment #663562 - Flags: review?(roc)
Attachment #663562 - Flags: review?(mounir)
>-  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 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...
Attachment #663562 - Flags: review?(mounir) → review+
Attachment #663562 - Flags: review?(roc) → review+
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.
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.
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.
(Assignee)

Comment 6

5 years ago
https://hg.mozilla.org/mozilla-central/rev/7e5ee5365db9
https://hg.mozilla.org/mozilla-central/rev/c9a8f55d8541
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.