Bug 30958 - Turn on warnings for QtWebKit for gcc
Summary: Turn on warnings for QtWebKit for gcc
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords: Qt
Depends on:
Blocks:
 
Reported: 2009-10-30 11:28 PDT by Laszlo Gombos
Modified: 2010-07-29 09:14 PDT (History)
8 users (show)

See Also:


Attachments
proposed patch (5.45 KB, patch)
2009-10-30 13:49 PDT, Laszlo Gombos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Laszlo Gombos 2009-10-30 11:28:39 PDT
As part of some previous work QtWebKit compilation warnings (for gcc) has been greatly reduced. It seems like a good time to turn on -Wall and some other compiler warnings for QtWebKit as well - as it is done for other ports.
Comment 1 Kenneth Rohde Christiansen 2009-10-30 13:43:27 PDT
I agree!
Comment 2 Laszlo Gombos 2009-10-30 13:49:12 PDT
Created attachment 42231 [details]
proposed patch
Comment 3 Eric Seidel (no email) 2009-10-30 15:18:42 PDT
Comment on attachment 42231 [details]
proposed patch

LGTM.
Comment 4 WebKit Commit Bot 2009-11-01 12:49:14 PST
Comment on attachment 42231 [details]
proposed patch

Clearing flags on attachment: 42231

Committed r50392: <http://trac.webkit.org/changeset/50392>
Comment 5 WebKit Commit Bot 2009-11-01 12:49:21 PST
All reviewed patches have been landed.  Closing bug.
Comment 6 Thiago Macieira 2010-05-03 05:32:14 PDT
-Wcast-align needs to be removed.

With GCC 4.4, these two warning crops up all the time on ARM:

webkit/JavaScriptCore/wtf/Vector.h:484: warning: cast from 'WTF::AlignedBufferChar*' to '<insert type here>*' increases required alignment of target type
webkit/JavaScriptCore/wtf/ListHashSet.h:174: warning: cast from 'char*' to 'WTF::ListHashSetNode<insert type here>*' increases required alignment of target type

I know that WTF::Vector aligns by construction, but GCC doesn't know it. And there's no way to tell it.

This option also triggers the warning in Qt code:
src/corelib/kernel/qobject.h:453: warning: cast from 'QObject*' to 'const WebCore::GraphicsLayerQtImpl*' increases required alignment of target type
src/corelib/tools/qmap.h:180: warning: cast from 'char*' to 'QMapNode<float, WebCore::KeyframeValueQt<WebCore::TransformOperations> >*' increases required alignment of target type

In both cases the pointers are properly aligned for the type at hand.

The one thing in common between these four places is the use of reinterpret_cast.
Comment 7 Laszlo Gombos 2010-05-03 06:38:55 PDT
(In reply to comment #6)
> -Wcast-align needs to be removed.
> 
> With GCC 4.4, these two warning crops up all the time on ARM:

Thiago, these issues are being looked at bug 38045. Two good reason why we should continue having these warnings on the _trunk_
 a./ Other WebKit ports have them enabled as well; some of the other ports are treat these warnings as errors. If we do not have them enabled building Qt port of WebKit will give no warning about a potential build break on other WebKit ports.
  b./ I think in long term we really want to fix these - and the best way to get attention is to have the warnings turned on.

If these really can not be addresses, we might want to consider turning the warnings off just for ARM platforms.

Also, it might make sense to turn the warnings of for production branches, but I think we should keep them on for the trunk.
Comment 8 Antonio Gomes 2010-07-25 08:09:41 PDT
> ... some of the other ports are treat these warnings as errors. 

Should we consider that, as well? At least on trunk for i386?
Comment 9 Laszlo Gombos 2010-07-28 11:37:41 PDT
(In reply to comment #8)
> > ... some of the other ports are treat these warnings as errors. 
> 
> Should we consider that, as well? At least on trunk for i386?

My only concern would be the extra pressure to fix these warnings to keep the bot green. CCing Ossy to see what he think.

A somewhat relaxed version of this - which I think we should do - is to start treating warnings as errors by default (at least on platforms where this is realistic like i386) but have a provision (e.g. environment variable) not to do this for the build bot.

This will force most developers to look into the warning but does not put extra pressure on the bot.