[GTK][gtkdoc] remove -Wcast-align

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Víctor M. Jáquez L.]

Apparently gcc warns that GParamSpec is not castable to GParamSpecInt64/GParamSpecUInt64/GParamSpecDouble due they are 64bit, even though ARM hackers claim that those only need 4byte alignment. As long as gcc behaves that way, this warning is not very useful, beside to break the build.

[Víctor M. Jáquez L.]

Created attachment 232701 [details]
Patch

[Alberto Garcia]

Thanks, this also needs to be backported to the 2.4 branch.

There's no way to fix this somewhere else rather than remove the warning?

[Víctor M. Jáquez L.]

(In reply to comment #2)
> Thanks, this also needs to be backported to the 2.4 branch.
> 
> There's no way to fix this somewhere else rather than remove the warning?

As far as I know, there's no other alternative. It's a compiler's false positive warning.

For example, GStreamer, has decided to remove the -Walign-cast completely because of it:

http://lists.freedesktop.org/archives/gstreamer-commits/2010-April/041509.html

[Alberto Garcia]

(In reply to comment #3)
> As far as I know, there's no other alternative. It's a compiler's false positive warning.

lgtm then

[Build Bot]

Comment on attachment 232701 [details]
Patch

Attachment 232701 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6583488660635648

New failing tests:
media/W3C/video/currentSrc/currentSrc_empty_if_no_src.html

[Build Bot]

Created attachment 232708 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5

[Martin Robinson]

Comment on attachment 232701 [details]
Patch

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

> Tools/gtk/generate-gtkdoc:113
> +    cflags = [x for x in config.get(module_name, 'cflags').split() if x != '-Wcast-align']

You should put a comment here explaining this issue.

Why not do this?

cflags = config.get(module_name, 'cflags').split()
if '-Wcast-align' in cflags:
    cflags.remote('-Wcast-align')

Another option which might make more sense is to simply append -Wno-cast-align (or whatever option disables this warning)  at line 170. I think that might be cleaner in the end.

[Víctor M. Jáquez L.]

Created attachment 232782 [details]
Patch

[Víctor M. Jáquez L.]

(In reply to comment #7)
> (From update of attachment 232701 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=232701&action=review
> 
> > Tools/gtk/generate-gtkdoc:113
> > +    cflags = [x for x in config.get(module_name, 'cflags').split() if x != '-Wcast-align']
> 
> You should put a comment here explaining this issue.
> 
> Why not do this?
> 
> cflags = config.get(module_name, 'cflags').split()
> if '-Wcast-align' in cflags:
>     cflags.remote('-Wcast-align')
> 
> Another option which might make more sense is to simply append -Wno-cast-align (or whatever option disables this warning)  at line 170. I think that might be cleaner in the end.

I uploaded a new version of the patch with you later suggestion. It seems clearer.

Martin, one question: the line

'cflags': " ".join(config.get(module_name, 'cflags').split()),

looks odd to me. Wouldn't be simpler just have

'cflags': config.get(module_name, 'cflags')

??

[Build Bot]

Comment on attachment 232782 [details]
Patch

Attachment 232782 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6276175965978624

New failing tests:
media/track/w3c/interfaces/TextTrackCue/id.html
media/track/w3c/interfaces/TextTrackCue/pauseOnExit.html
media/track/w3c/interfaces/TextTrackCue/track.html
media/track/w3c/interfaces/TextTrackCue/startTime.html
media/track/w3c/interfaces/TextTrackCue/endTime.html
media/track/w3c/interfaces/TextTrackCue/align.html

[Build Bot]

Created attachment 232784 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.5

[Martin Robinson]

Comment on attachment 232701 [details]
Patch

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

>>> Tools/gtk/generate-gtkdoc:113
>>> +    cflags = [x for x in config.get(module_name, 'cflags').split() if x != '-Wcast-align']
>> 
>> You should put a comment here explaining this issue.
>> 
>> Why not do this?
>> 
>> cflags = config.get(module_name, 'cflags').split()
>> if '-Wcast-align' in cflags:
>>     cflags.remote('-Wcast-align')
>> 
>> Another option which might make more sense is to simply append -Wno-cast-align (or whatever option disables this warning)  at line 170. I think that might be cleaner in the end.
> 
> I uploaded a new version of the patch with you later suggestion. It seems clearer.
> 
> Martin, one question: the line
> 
> 'cflags': " ".join(config.get(module_name, 'cflags').split()),
> 
> looks odd to me. Wouldn't be simpler just have
> 
> 'cflags': config.get(module_name, 'cflags')
> 
> ??

This is to remove newlines, I believe.

[WebKit Commit Bot]

Comment on attachment 232782 [details]
Patch

Clearing flags on attachment: 232782

Committed r169768: <http://trac.webkit.org/changeset/169768>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.