...
Created attachment 297152 [details] patch
Attachment 297152 [details] did not pass style-queue: ERROR: Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:455: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:461: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] ERROR: Source/JavaScriptCore/wasm/WasmModuleParser.cpp:516: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/JavaScriptCore/wasm/WasmModuleParser.cpp:523: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:132: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] ERROR: Source/JavaScriptCore/wasm/js/WebAssemblyModuleRecord.cpp:140: Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons. [readability/comparison_to_zero] [5] Total errors found: 6 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 297152 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=297152&action=review Looks good otherwise. > JSTests/wasm/Builder.js:196 > + assert.isNumber(index, `Memory exports only support number indices right now`); FIXME + bug? > JSTests/wasm/js-api/table.js:93 > + assertBadBinary(builder, badExportString); I'd really rather use assert.throws, I have to update the other tests to it right now. > JSTests/wasm/js-api/test_memory.js:156 > + return; ?
Comment on attachment 297152 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=297152&action=review >> JSTests/wasm/js-api/test_memory.js:156 >> + return; > > ? This should be removed.
Comment on attachment 297152 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=297152&action=review r=me. > JSTests/wasm/js-api/table.js:279 > + assert.truthy(table === instance.exports.table); > + assert.truthy(table === instance.exports.table2); can't you do assert.eq? >> JSTests/wasm/js-api/test_memory.js:156 >> + return; > > ? Test passes! > Source/JavaScriptCore/wasm/WasmB3IRGenerator.cpp:462 > + Value* partialResult = m_currentBlock->appendNew<Value>(m_proc, BitAnd, Origin(), > + value, > + m_currentBlock->appendNew<Const32Value>(m_proc, Origin(), 0x0000ffff)); Nit, I would do this as: Value* partialResult = m_currentBlock->appendNew<Value>(m_proc, BitAnd, Origin(), value, m_currentBlock->appendNew<Const32Value>(m_proc, Origin(), 0x0000ffff));
landed in: https://trac.webkit.org/changeset/209850
*** Bug 165684 has been marked as a duplicate of this bug. ***
*** Bug 165671 has been marked as a duplicate of this bug. ***
*** Bug 165782 has been marked as a duplicate of this bug. ***