Bug 143313 – REGRESSION (r182121): SVG animation macros cause warnings in MSVC

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Alexey Proskuryakov]

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

[Alexey Proskuryakov]

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.

[Alexey Proskuryakov]

Created attachment 249937 [details]
proposed fix

[Darin Adler]

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.

[Darin Adler]

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.

[Darin Adler]

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.

[Alexey Proskuryakov]

Committed r182256 with a new title.

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