Add a compile assert for the size of BidiContext
Created attachment 134903 [details] Patch
Unfortunately, very few people can review changes to this file :(
Comment on attachment 134903 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134903&action=review > Source/WebCore/platform/text/BidiContext.h:62 > - unsigned char m_level; > + unsigned m_level : 6; You should add a comment which explains that we only need to handle 64 bidi-levels. Do we have tests for overflow of this value?
(In reply to comment #3) > (From update of attachment 134903 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=134903&action=review > > > Source/WebCore/platform/text/BidiContext.h:62 > > - unsigned char m_level; > > + unsigned m_level : 6; > > You should add a comment which explains that we only need to handle 64 bidi-levels. Do we have tests for overflow of this value? Maybe we should define numBidiLevelBits?
Created attachment 134910 [details] Patch
Comment on attachment 134910 [details] Patch OK. LGTM.
Committed r112742: <http://trac.webkit.org/changeset/112742>
Comment on attachment 134910 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=134910&action=review > Source/WebCore/platform/text/BidiContext.cpp:31 > + uint32_t bitfields : 16; Tiny style question. Why use uint32_t here instead of unsigned?
(In reply to comment #8) > (From update of attachment 134910 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=134910&action=review > > > Source/WebCore/platform/text/BidiContext.cpp:31 > > + uint32_t bitfields : 16; > > Tiny style question. Why use uint32_t here instead of unsigned? I was just following the pattern used elsewhere. We can change all of them to use unsigned instead.