Bug 143313 - REGRESSION (r182121): SVG animation macros cause warnings in MSVC
Summary: REGRESSION (r182121): SVG animation macros cause warnings in MSVC
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-01 12:00 PDT by Alexey Proskuryakov
Modified: 2015-04-01 15:14 PDT (History)
3 users (show)

See Also:


Attachments
proposed fix (1.28 KB, patch)
2015-04-01 12:06 PDT, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2015-04-01 12:00:39 PDT
MSVC warns about what sounds like a real bug in SVG code:

C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\svg\SVGCursorElement.h(64): warning C4373: 'WebCore::SVGCursorElement::setExternalResourcesRequiredBaseValue': virtual function overrides 'WebCore::SVGExternalResourcesRequired::setExternalResourcesRequiredBaseValue', previous versions of the compiler did not override when parameters only differed by const/volatile qualifiers (..\css\CSSAllInOne.cpp) [C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\WebCore.vcxproj\WebCore.vcxproj]
C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\svg\SVGFontElement.h(95): warning C4373: 'WebCore::SVGFontElement::setExternalResourcesRequiredBaseValue': virtual function overrides 'WebCore::SVGExternalResourcesRequired::setExternalResourcesRequiredBaseValue', previous versions of the compiler did not override when parameters only differed by const/volatile qualifiers (..\css\CSSAllInOne.cpp) [C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\WebCore.vcxproj\WebCore.vcxproj]
C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\svg\SVGSVGElement.h(46): warning C4373: 'WebCore::SVGSVGElement::setExternalResourcesRequiredBaseValue': virtual function overrides 'WebCore::SVGExternalResourcesRequired::setExternalResourcesRequiredBaseValue', previous versions of the compiler did not override when parameters only differed by const/volatile qualifiers (..\accessibility\AccessibilityAllInOne.cpp) [C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\WebCore.vcxproj\WebCore.vcxproj]
C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\svg\SVGSVGElement.h(46): warning C4373: 'WebCore::SVGSVGElement::setExternalResourcesRequiredBaseValue': virtual function overrides 'WebCore::SVGExternalResourcesRequired::setExternalResourcesRequiredBaseValue', previous versions of the compiler did not override when parameters only differed by const/volatile qualifiers (..\css\CSSAllInOne.cpp) [C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\WebCore.vcxproj\WebCore.vcxproj]
C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\svg\SVGUseElement.h(46): warning C4373: 'WebCore::SVGUseElement::setExternalResourcesRequiredBaseValue': virtual function overrides 'WebCore::SVGExternalResourcesRequired::setExternalResourcesRequiredBaseValue', previous versions of the compiler did not override when parameters only differed by const/volatile qualifiers (..\dom\DOMAllInOne.cpp) [C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\WebCore.vcxproj\WebCore.vcxproj]
C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\svg\SVGScriptElement.h(82): warning C4373: 'WebCore::SVGScriptElement::setExternalResourcesRequiredBaseValue': virtual function overrides 'WebCore::SVGExternalResourcesRequired::setExternalResourcesRequiredBaseValue', previous versions of the compiler did not override when parameters only differed by const/volatile qualifiers (..\dom\DOMAllInOne.cpp) [C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\WebCore.vcxproj\WebCore.vcxproj]
...

Full log at https://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/68854/steps/compile-webkit/logs/stdio
Comment 1 Alexey Proskuryakov 2015-04-01 12:03:21 PDT
This seems to be almost certainly a regression from <http://trac.webkit.org/changeset/182121>.

I'm not sure how to observe this, or even whether there was any change in behavior with either clang or MSVC.
Comment 2 Alexey Proskuryakov 2015-04-01 12:06:00 PDT
Created attachment 249937 [details]
proposed fix
Comment 3 Darin Adler 2015-04-01 13:40:04 PDT
Comment on attachment 249937 [details]
proposed fix

Oops! We really need to add the “override” to the overrides of this function. That would have caused this to be a compilation failure rather than a subtle cause of failing tests.
Comment 4 Darin Adler 2015-04-01 13:42:24 PDT
Wait, no this is a big misunderstanding.

The warning is telling you about a behavior change where the Microsoft tools development team fixed a bug in the old version of Visual Studio. The warning simply indicates that this code would have had different behavior *in an old version of MSVC*. I don’t care about this passionately one way or another, but there’s nothing wrong here at all. We could just turn off this warning instead.
Comment 5 Darin Adler 2015-04-01 13:43:26 PDT
I think the title of this bug is misleading. I don’t know quite what to title it. It’s more appropriate to say “MSVC started warning, but there is no problem other than the warning”.

Unless there is some symptom here other than the warning that I’m unaware of? Can’t imagine there is given my understanding of the situation.
Comment 6 Alexey Proskuryakov 2015-04-01 15:14:21 PDT
Committed r182256 with a new title.

If this keeps happening, we should probably just silence the warning.