Bug 197000 - Make roundUpToPowerOfTwo work with Checked arithmetic classes
Summary: Make roundUpToPowerOfTwo work with Checked arithmetic classes
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords:
Depends on:
Blocks: 196935
  Show dependency treegraph
 
Reported: 2019-04-16 22:20 PDT by Ryosuke Niwa
Modified: 2019-04-17 23:39 PDT (History)
5 users (show)

See Also:


Attachments
Patch (21.15 KB, patch)
2019-04-16 22:30 PDT, Ryosuke Niwa
no flags Details | Formatted Diff | Diff
GCC build fix attempt (21.28 KB, patch)
2019-04-16 22:47 PDT, Ryosuke Niwa
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews112 for mac-highsierra (456.85 KB, application/zip)
2019-04-17 00:01 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews123 for ios-simulator-wk2 (117.55 MB, application/zip)
2019-04-17 00:29 PDT, EWS Watchlist
no flags Details
Fixed the tests (22.13 KB, patch)
2019-04-17 19:09 PDT, Ryosuke Niwa
rniwa: review?
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2019-04-16 22:20:20 PDT
In order to make HashTable code safe, we need to make roundUpToPowerOfTwo work with Checked<T, U> classes.
Comment 1 Ryosuke Niwa 2019-04-16 22:30:41 PDT
Created attachment 367613 [details]
Patch
Comment 2 EWS Watchlist 2019-04-16 22:32:41 PDT
Attachment 367613 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/CheckedArithmetic.h:395:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WTF/wtf/CheckedArithmetic.h:463:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WTF/wtf/CheckedArithmetic.h:471:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WTF/wtf/CheckedArithmetic.h:1002:  Missing spaces around |  [whitespace/operators] [3]
ERROR: Source/WTF/wtf/CheckedArithmetic.h:1040:  Missing spaces around |  [whitespace/operators] [3]
ERROR: Source/WTF/wtf/CheckedArithmetic.h:1060:  Missing spaces around |  [whitespace/operators] [3]
ERROR: Source/WTF/wtf/CheckedArithmetic.h:1123:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 7 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Ryosuke Niwa 2019-04-16 22:47:21 PDT
Created attachment 367614 [details]
GCC build fix attempt
Comment 4 EWS Watchlist 2019-04-16 22:48:48 PDT
Attachment 367614 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/CheckedArithmetic.h:395:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WTF/wtf/CheckedArithmetic.h:463:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WTF/wtf/CheckedArithmetic.h:471:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WTF/wtf/CheckedArithmetic.h:1002:  Missing spaces around |  [whitespace/operators] [3]
ERROR: Source/WTF/wtf/CheckedArithmetic.h:1040:  Missing spaces around |  [whitespace/operators] [3]
ERROR: Source/WTF/wtf/CheckedArithmetic.h:1060:  Missing spaces around |  [whitespace/operators] [3]
ERROR: Source/WTF/wtf/CheckedArithmetic.h:1123:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 7 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 EWS Watchlist 2019-04-17 00:01:20 PDT
Comment on attachment 367614 [details]
GCC build fix attempt

Attachment 367614 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/11896632

Number of test failures exceeded the failure limit.
Comment 6 EWS Watchlist 2019-04-17 00:01:23 PDT
Created attachment 367617 [details]
Archive of layout-test-results from ews112 for mac-highsierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-highsierra  Platform: Mac OS X 10.13.6
Comment 7 EWS Watchlist 2019-04-17 00:29:33 PDT
Comment on attachment 367614 [details]
GCC build fix attempt

Attachment 367614 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/11896717

Number of test failures exceeded the failure limit.
Comment 8 EWS Watchlist 2019-04-17 00:29:38 PDT
Created attachment 367618 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 9 Ryosuke Niwa 2019-04-17 01:00:31 PDT
Comment on attachment 367614 [details]
GCC build fix attempt

Oops, looks like I broke things.
Comment 10 Ryosuke Niwa 2019-04-17 19:09:00 PDT
Created attachment 367713 [details]
Fixed the tests
Comment 11 EWS Watchlist 2019-04-17 19:11:40 PDT
Attachment 367713 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/CheckedArithmetic.h:395:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WTF/wtf/CheckedArithmetic.h:463:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WTF/wtf/CheckedArithmetic.h:471:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WTF/wtf/CheckedArithmetic.h:1002:  Missing spaces around |  [whitespace/operators] [3]
ERROR: Source/WTF/wtf/CheckedArithmetic.h:1040:  Missing spaces around |  [whitespace/operators] [3]
ERROR: Source/WTF/wtf/CheckedArithmetic.h:1060:  Missing spaces around |  [whitespace/operators] [3]
ERROR: Source/WTF/wtf/CheckedArithmetic.h:1123:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 7 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Saam Barati 2019-04-17 23:39:46 PDT
Comment on attachment 367713 [details]
Fixed the tests

This seems like way more code than just doing the dumb way of checking for unsigned overflow...