RESOLVED FIXED 161043
[Win] Warning fix. 
https://bugs.webkit.org/show_bug.cgi?id=161043
Summary [Win] Warning fix.
Per Arne Vollan
Reported 2016-08-22 06:03:08 PDT
MSVC gives the following warning in PCToCodeOriginMap.cpp, line 55: warning C4333: '>>': right shift by too large amount, data loss
Attachments
Patch (1.30 KB, patch)
2016-08-22 06:12 PDT, Per Arne Vollan 		no flags
DetailsFormatted DiffDiff
Patch (1.35 KB, patch)
2016-08-22 08:27 PDT, Per Arne Vollan 		no flags
DetailsFormatted DiffDiff
Patch (1.01 KB, patch)
2016-08-22 09:08 PDT, Per Arne Vollan 		mark.lam: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Per Arne Vollan
Comment 1 2016-08-22 06:12:05 PDT
Created attachment 286594 [details] Patch
Brent Fulgham
Comment 2 2016-08-22 07:53:50 PDT
Comment on attachment 286594 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=286594&action=review > Source/JavaScriptCore/PlatformWin.cmake:47 > +set_source_files_properties(jit/PCToCodeOriginMap.cpp PROPERTIES COMPILE_FLAGS /Wd"4333") This seems to break the JSC build: cl : Command line error D8021: invalid numeric argument '/Wd4333'
Per Arne Vollan
Comment 3 2016-08-22 08:27:54 PDT
Created attachment 286597 [details] Patch
Alex Christensen
Comment 4 2016-08-22 08:39:40 PDT
Wouldn't this be better done by a pragma?
Per Arne Vollan
Comment 5 2016-08-22 09:08:14 PDT
Created attachment 286599 [details] Patch
Per Arne Vollan
Comment 6 2016-08-22 09:09:53 PDT
(In reply to comment #4) > Wouldn't this be better done by a pragma? Thanks! Updated patch.
Mark Lam
Comment 7 2016-08-22 09:27:16 PDT
Comment on attachment 286599 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=286599&action=review r=me > Source/JavaScriptCore/jit/PCToCodeOriginMap.cpp:37 > +#if COMPILER(MSVC) > +#pragma warning(disable: 4333) > +#endif It would be good to include https://msdn.microsoft.com/en-us/library/4wz07268.aspx in a comment here so that the reader can easily look up what the 4333 means in the future.
Per Arne Vollan
Comment 8 2016-08-23 00:12:58 PDT
(In reply to comment #7) > Comment on attachment 286599 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=286599&action=review > > r=me > > > Source/JavaScriptCore/jit/PCToCodeOriginMap.cpp:37 > > +#if COMPILER(MSVC) > > +#pragma warning(disable: 4333) > > +#endif > > It would be good to include > https://msdn.microsoft.com/en-us/library/4wz07268.aspx in a comment here so > that the reader can easily look up what the 4333 means in the future. Thanks for reviewing :) I will add the link.
Per Arne Vollan
Comment 9 2016-08-23 00:15:31 PDT
Committed r204769: <http://trac.webkit.org/changeset/204769>
Note You need to log in before you can comment on or make changes to this bug.