Bug 5301

Summary: webcore+svg build fails with gcc3.3
Product: WebKit Reporter: Julien Palmas <julien.palmas>
Component: SVGAssignee: Eric Seidel (no email) <eric>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P4    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
Attachments:
Description Flags
Proposed patch
eric: review-
modified patch
eric: review-
modified patch eric: review+

Description Julien Palmas 2005-10-08 07:41:58 PDT
2 errors with gcc3.3

- kdom/core/NodeImpl.h
    This file is included by KWQSlot.mm before the macro #define id ID_HACK in DrawDocument.mm
    This might be a bug of gcc4.0

- WebCore+SVG/DrawView.mm
    Many "may not respond to ..." warnings because DrawViewPrivate methods are not declared in the 
@interface.
    Build fails as we treat warnings as errors.
    The definitions of those methods are before their use (in the same file), so I don't know if there 
should be a warning ...
Comment 1 Julien Palmas 2005-10-08 07:42:29 PDT
Created attachment 4257 [details]
Proposed patch
Comment 2 Eric Seidel (no email) 2005-10-08 12:42:16 PDT
Comment on attachment 4257 [details]
Proposed patch

The first change doesn't need to be in an #if block, since adding the forward
defnitions there probably should have been done in the first place (and
certainly doesn't hurt gcc 4.0).

For the second change you should instead put the #define id ID_HACK block in
the .mm file which is actually including this NodeImpl.h (likely through a long
chain of other includes).  See how this is done in DrawDocument.mm.  You won't
need to put them inside #if __GNUC__ <4 blocks then.

Please correct those two and post another patch.  Thanks!
Comment 3 Eric Seidel (no email) 2005-10-08 12:44:03 PDT
Now that I'm reading your comments:

- kdom/core/NodeImpl.h
    This file is included by KWQSlot.mm before the macro #define id ID_HACK in DrawDocument.mm
    This might be a bug of gcc4.0

Just add a id -> ID_HACK around the include in KWQSlot.mm (inside the SVG_SUPPORT block).
Comment 4 Julien Palmas 2005-10-09 18:36:56 PDT
Created attachment 4274 [details]
modified patch

Put the #define id ID_HACK in KWQSlot.mm, not anymore in NodeImpl.h
Put the #define id ID_HACK in DrawCanvasItem.mm.

#undef id was needed in KWQApplication.h for other includes from this file.
Comment 5 Eric Seidel (no email) 2005-10-09 21:46:29 PDT
I'm still not convinced the qapplication.h change was necessary.  If it is, it shouldn't be SVG specific.

Can you explain to me why?
Comment 6 Eric Seidel (no email) 2005-10-10 14:52:49 PDT
Comment on attachment 4274 [details]
modified patch

We talked about an alternative solution to the KWQApplication.h problem (by
removing the qapplication.h include from RenderStyle.h).

Otherwise the patch looks good. I look forward to landing the final version!
Comment 7 Julien Palmas 2005-10-10 18:09:21 PDT
Created attachment 4302 [details]
modified patch

Like eric suggested, added #import <qapplication.h> before the #define id
ID_HACK macro in DrawCanvasItem.mm
Comment 8 Eric Seidel (no email) 2005-10-10 18:11:12 PDT
Comment on attachment 4302 [details]
modified patch

Looks great!  r=me