143313 2015-04-01 12:00:39 -0700 REGRESSION (r182121): SVG animation macros cause warnings in MSVC 2015-04-01 15:14:21 -0700 1 1 1 Unclassified WebKit SVG 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 ap ap darin sabouhallawa zimmermann oldest_to_newest 1082071 0 ap 2015-04-01 12:00:39 -0700 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 1082072 1 ap 2015-04-01 12:03:21 -0700 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. 1082075 2 249937 ap 2015-04-01 12:06:00 -0700 Created attachment 249937 proposed fix 1082091 3 249937 darin 2015-04-01 13:40:04 -0700 Comment on attachment 249937 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. 1082093 4 darin 2015-04-01 13:42:24 -0700 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. 1082094 5 darin 2015-04-01 13:43:26 -0700 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. 1082115 6 ap 2015-04-01 15:14:21 -0700 Committed r182256 with a new title. If this keeps happening, we should probably just silence the warning. 249937 2015-04-01 12:06:00 -0700 2015-04-01 13:40:04 -0700 proposed fix ConstBool.txt text/plain 1314 ap SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDE4MjI0NykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEwIEBACisyMDE1LTA0LTAxICBBbGV4ZXkg UHJvc2t1cnlha292ICA8YXBAYXBwbGUuY29tPgorCisgICAgICAgIFJFR1JFU1NJT04gKHIxODIx MjEpOiBTVkcgYW5pbWF0aW9uIG1hY3JvcyBzZWVtIHRvIGRvIGluaGVyaXRhbmNlIHdyb25nCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNDMzMTMKKwor ICAgICAgICAqIHN2Zy9TVkdFeHRlcm5hbFJlc291cmNlc1JlcXVpcmVkLmg6IFVuZG8gdGhlIGNo YW5nZSB0aGF0IGNhdXNlZCBhIHR5cGUgbWlzbWF0Y2guCisKIDIwMTUtMDQtMDEgIFlvYXYgV2Vp c3MgIDx5b2F2QHlvYXYud3M+CiAKICAgICAgICAgQXN5bmMgbG9hZGluZyBvZiBpbWFnZSByZXNv dXJjZXMKSW5kZXg6IFNvdXJjZS9XZWJDb3JlL3N2Zy9TVkdFeHRlcm5hbFJlc291cmNlc1JlcXVp cmVkLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvc3ZnL1NWR0V4dGVybmFsUmVzb3Vy Y2VzUmVxdWlyZWQuaAkocmV2aXNpb24gMTgyMjExKQorKysgU291cmNlL1dlYkNvcmUvc3ZnL1NW R0V4dGVybmFsUmVzb3VyY2VzUmVxdWlyZWQuaAkod29ya2luZyBjb3B5KQpAQCAtNDYsNyArNDYs NyBAQCBwdWJsaWM6CiAKIHByb3RlY3RlZDoKICAgICAvLyBUaGVzZSB0eXBlcyBsb29rIGEgYml0 IGF3a3dhcmQsIGJ1dCBoYXZlIHRvIG1hdGNoIHRoZSBnZW5lcmljIHR5cGVzIG9mIHRoZSBTVkdB bmltYXRlZFByb3BlcnR5IG1hY3Jvcy4KLSAgICB2aXJ0dWFsIHZvaWQgc2V0RXh0ZXJuYWxSZXNv dXJjZXNSZXF1aXJlZEJhc2VWYWx1ZShjb25zdCBib29sJiwgYm9vbCB2YWxpZFZhbHVlID0gdHJ1 ZSkgPSAwOworICAgIHZpcnR1YWwgdm9pZCBzZXRFeHRlcm5hbFJlc291cmNlc1JlcXVpcmVkQmFz ZVZhbHVlKGNvbnN0IGJvb2wmLCBjb25zdCBib29sIHZhbGlkVmFsdWUgPSB0cnVlKSA9IDA7CiAg ICAgdmlydHVhbCBib29sJiBleHRlcm5hbFJlc291cmNlc1JlcXVpcmVkQmFzZVZhbHVlKCkgY29u c3QgPSAwOwogCiAgICAgdmlydHVhbCB2b2lkIHNldEhhdmVGaXJlZExvYWRFdmVudChib29sKSB7 IH0K