WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
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
Details
Patch
(1.94 KB, patch)
2014-06-10 02:01 PDT
,
Víctor M. Jáquez L.
no flags
Details
Formatted Diff
Diff
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
Details
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Víctor M. Jáquez L.
Comment 1
2014-06-09 04:22:38 PDT
Created
attachment 232701
[details]
Patch
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
Created
attachment 232782
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug