RESOLVED FIXED 133640
[GTK][gtkdoc] remove -Wcast-align
https://bugs.webkit.org/show_bug.cgi?id=133640
Summary [GTK][gtkdoc] remove -Wcast-align
Víctor M. Jáquez L.
Reported 2014-06-09 04:15:08 PDT
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.
Attachments
Patch (2.20 KB, patch)
2014-06-09 04:22 PDT, Víctor M. Jáquez L.
no flags
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (635.91 KB, application/zip)
2014-06-09 09:25 PDT, Build Bot
no flags
Patch (1.94 KB, patch)
2014-06-10 02:01 PDT, Víctor M. Jáquez L.
no flags
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion (548.86 KB, application/zip)
2014-06-10 04:27 PDT, Build Bot
no flags
Víctor M. Jáquez L.
Comment 1 2014-06-09 04:22:38 PDT
Alberto Garcia
Comment 2 2014-06-09 04:27:46 PDT
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.
Comment 3 2014-06-09 04:41:48 PDT
(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
Comment 4 2014-06-09 05:51:55 PDT
(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 5 2014-06-09 09:25:02 PDT
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
Comment 6 2014-06-09 09:25:05 PDT
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 7 2014-06-09 10:38:02 PDT
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.
Comment 8 2014-06-10 02:01:34 PDT
Víctor M. Jáquez L.
Comment 9 2014-06-10 02:07:35 PDT
(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 10 2014-06-10 04:27:13 PDT
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
Comment 11 2014-06-10 04:27:17 PDT
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 12 2014-06-10 07:14:36 PDT
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 13 2014-06-10 14:40:37 PDT
Comment on attachment 232782 [details] Patch Clearing flags on attachment: 232782 Committed r169768: <http://trac.webkit.org/changeset/169768>
WebKit Commit Bot
Comment 14 2014-06-10 14:40:42 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.