WebCore doesn't build with recent clang.
Created attachment 151096 [details] fix warnings by the new -Wunused-private-field I commented out the field in GraphicsContext3DPrivate instead of removing it since it has a comment saying that eventually it will be used in the future.
Comment on attachment 151096 [details] fix warnings by the new -Wunused-private-field Attachment 151096 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/13156171
Comment on attachment 151096 [details] fix warnings by the new -Wunused-private-field Attachment 151096 [details] did not pass qt-wk2-ews (qt): Output: http://queues.webkit.org/results/13142531
Comment on attachment 151096 [details] fix warnings by the new -Wunused-private-field Attachment 151096 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/13140545
Created attachment 151101 [details] fix warnings by the new -Wunused-private-field (take #2)
Attachment 151101 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/platform/graphics/cg/ImageSourceCG.cpp:79: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/platform/graphics/cg/ImageSourceCG.cpp:80: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 2 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 151103 [details] fix warnings by the new -Wunused-private-field (take #3)
Did you want to mark this for review, not just cq?
(In reply to comment #8) > Did you want to mark this for review, not just cq? yes, thanks! I still don't master all these things of webkit..
Comment on attachment 151103 [details] fix warnings by the new -Wunused-private-field (take #3) View in context: https://bugs.webkit.org/attachment.cgi?id=151103&action=review > Source/WebCore/platform/graphics/cg/ImageSourceCG.cpp:80 > + (void)m_alphaOption; > + (void)m_gammaAndColorProfileOption; Why did you choose to keep these? The (void) trick is not something we normally do in WebKit code. If this is really necessary, please add a comment. > Source/WebCore/platform/graphics/mac/GraphicsContext3DMac.mm:61 > + /* : m_graphicsContext3D(graphicsContext3D) */ As a strong rule, we don't check in commented out code. Not sure who can comment about the refactoring plans for this class. 94083 cmarrin@apple.com // FIXME: This class is currently empty on Mac, but will get populated as 94083 cmarrin@apple.com // the restructuring in https://bugs.webkit.org/show_bug.cgi?id=66903 is done
Comment on attachment 151103 [details] fix warnings by the new -Wunused-private-field (take #3) View in context: https://bugs.webkit.org/attachment.cgi?id=151103&action=review >> Source/WebCore/platform/graphics/cg/ImageSourceCG.cpp:80 >> + (void)m_gammaAndColorProfileOption; > > Why did you choose to keep these? The (void) trick is not something we normally do in WebKit code. > > If this is really necessary, please add a comment. AFAICT, there are several implementations of ImageSource. Other implementation do use these parameters, so they cannot be removed. Moreover, there's a comment just one line above (that cannot be seen here in the diff) that says: " // FIXME: m_premultiplyAlpha is ignored in cg at the moment. " So I assume someone will want to use the alpha stuff in the future. >> Source/WebCore/platform/graphics/mac/GraphicsContext3DMac.mm:61 >> + /* : m_graphicsContext3D(graphicsContext3D) */ > > As a strong rule, we don't check in commented out code. Not sure who can comment about the refactoring plans for this class. > > 94083 cmarrin@apple.com // FIXME: This class is currently empty on Mac, but will get populated as > 94083 cmarrin@apple.com // the restructuring in https://bugs.webkit.org/show_bug.cgi?id=66903 is done I've no clue about this; I was just obeying the comment!
> AFAICT, there are several implementations of ImageSource. Other implementation do use these parameters, so they cannot be removed. Normally, we'd ifdef the member variables out when they are not needed.
Comment on attachment 151103 [details] fix warnings by the new -Wunused-private-field (take #3) r- ap's comment. Please upload a new patch addressing Alexey's comments.
Can this bug be closed? WebKit successfully builds with clang that's presumably newer than what we had in July 2012.
Looks like this has been fixed.