Bug 161043

Summary: [Win] Warning fix.
Product: WebKit Reporter: Per Arne Vollan <pvollan>
Component: JavaScriptCoreAssignee: Per Arne Vollan <pvollan>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, bfulgham, commit-queue, keith_miller, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch mark.lam: review+

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.