Bug 30958 – Turn on warnings for QtWebKit for gcc

• Component: Platform
• Status: RESOLVED FIXED

[Laszlo Gombos]

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.

[Kenneth Rohde Christiansen]

I agree!

[Laszlo Gombos]

Created attachment 42231 [details]
proposed patch

[Eric Seidel (no email)]

Comment on attachment 42231 [details]
proposed patch

LGTM.

[WebKit Commit Bot]

Comment on attachment 42231 [details]
proposed patch

Clearing flags on attachment: 42231

Committed r50392: <http://trac.webkit.org/changeset/50392>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Thiago Macieira]

-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.

[Laszlo Gombos]

(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.

[Antonio Gomes]

> ... some of the other ports are treat these warnings as errors. 

Should we consider that, as well? At least on trunk for i386?

[Laszlo Gombos]

(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.