Bug 143760

Summary: [W32] Compile-time assertion failure: RenderBlock_should_stay_small
Product: WebKit Reporter: LRN <lrn1986>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, lrn1986, mcatanzaro, mcrha, tpopela
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 133028    
Attachments:
Description Flags
Move enum LineLayoutPath in RenderBlock cgarcia: review+, cgarcia: commit-queue-

Description LRN 2015-04-15 06:22:15 PDT
Presumably as a result of the way bitfields are packed on Windows (-mms-bitfields is the default these days).
Comment 1 LRN 2015-04-15 09:54:08 PDT
Created attachment 250805 [details]
Move enum LineLayoutPath in RenderBlock

This removes the bitfield interruption, allowing more efficient
packing.
Otherwise compile-time assertion fails:
In file included from ../webkitgtk-2.4.8/Source/WTF/wtf/PossiblyNull.h:29:0,
                 from ../webkitgtk-2.4.8/Source/WTF/wtf/FastMalloc.h:27,
                 from ../webkitgtk-2.4.8/Source/WebCore/config.h:74,
                 from ../webkitgtk-2.4.8/Source/WebCore/rendering/RenderBlock.cpp:24:
../webkitgtk-2.4.8/Source/WTF/wtf/Assertions.h:326:35: error: static assertion failed: RenderBlock_should_stay_small
 #define COMPILE_ASSERT(exp, name) static_assert((exp), #name)
                                   ^
../webkitgtk-2.4.8/Source/WebCore/rendering/RenderBlock.cpp:88:1: note: in expansion of macro 'COMPILE_ASSERT'
 COMPILE_ASSERT(sizeof(RenderBlock) == sizeof(SameSizeAsRenderBlock), RenderBlock_should_stay_small);
 ^
Comment 2 Darin Adler 2015-04-15 10:34:45 PDT
Comment on attachment 250805 [details]
Move enum LineLayoutPath in RenderBlock

View in context: https://bugs.webkit.org/attachment.cgi?id=250805&action=review

> Source/WebCore/rendering/RenderBlock.h:-614
> -    enum LineLayoutPath { UndeterminedPath, SimpleLinesPath, LineBoxesPath, ForceLineBoxesPath };

It’s very strange the defining an enum type (not a data member, just a type) causes a “bitfield interruption”. Which compiler has this problem?
Comment 3 LRN 2015-04-15 10:41:01 PDT
(In reply to comment #2)
> Comment on attachment 250805 [details]
> Move enum LineLayoutPath in RenderBlock
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=250805&action=review
> 
> > Source/WebCore/rendering/RenderBlock.h:-614
> > -    enum LineLayoutPath { UndeterminedPath, SimpleLinesPath, LineBoxesPath, ForceLineBoxesPath };
> 
> It’s very strange the defining an enum type (not a data member, just a type)
> causes a “bitfield interruption”. Which compiler has this problem?

i686-w64-mingw32-gcc-4.9.2 is what i've used

The patch originated from https://bugs.webkit.org/show_bug.cgi?id=133028 , where the original author used 4.8.something
Comment 4 Milan Crha 2015-04-17 16:20:39 PDT
(In reply to comment #3)
> The patch originated from https://bugs.webkit.org/show_bug.cgi?id=133028 ,
> where the original author used 4.8.something

I still use this change with the webkit-2.4 branch at revision 182543.

@LRN: so you've split my change (and other changes), but still want to receive credits for it? I'd say it's not a good habit by any means... :-/
Comment 5 LRN 2015-04-17 17:36:34 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > The patch originated from https://bugs.webkit.org/show_bug.cgi?id=133028 ,
> > where the original author used 4.8.something
> 
> I still use this change with the webkit-2.4 branch at revision 182543.
> 
> @LRN: so you've split my change (and other changes), but still want to
> receive credits for it? I'd say it's not a good habit by any means... :-/

The problem with the original bug was that it had multiple conflicting patches from 2 different authors, which made it difficult for upstream to sort through.

So i've split the changes into separate, manageable chunks and filed a lot bugs. I do admit that i haven't tracked copyrights or credit though (partially because many patches had no authorship information in them, i.e. were plain diffs; partially because i was more interested in figuring out which changes to apply and which to drop), and it did bother me somewhat when i was filing the bugs (especially on patches with lots of non-trivial code that i'm incapable of writing myself). So if you feel slighted, do comment where appropriate, and hopefully webkit devs will do something (no idea what's the protocol for such things; i assume there is one, because webkit seems to have lots of unapparent rules about committing).

Ironically, half of these changes probably won't make it past 2.4 anyway :(
Comment 6 Michael Catanzaro 2015-04-17 20:22:31 PDT
Just please credit the original author somewhere in the changelog entry. :)  "Patch by Person" or "Based on work by Person," something like that.
Comment 7 Carlos Garcia Campos 2015-05-18 23:31:01 PDT
Comment on attachment 250805 [details]
Move enum LineLayoutPath in RenderBlock

I don't understand it either, but it's harmless in any case.
Comment 8 Carlos Garcia Campos 2015-05-18 23:31:36 PDT
Committed to 2.4 http://trac.webkit.org/changeset/184548