Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2559113006:
[wasm][asm.js] Ignore duplicate exports in asm.js. (Closed)
|
Created:
4 years ago by bradnelson Modified:
4 years ago CC:
v8-reviews_googlegroups.com Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[wasm][asm.js] Ignore duplicate exports in asm.js.
BUG=672789
R=titzer@chromium.org
Committed: https://crrev.com/be9ee2237d7278665c4204a4b72654122bc95e26
Cr-Commit-Position: refs/heads/master@{#41647}
Patch Set 1 #Patch Set 2 : merge #Patch Set 3 : fix #
Total comments: 2
Patch Set 4 : fix #
Messages
Total messages: 30 (22 generated)
The CQ bit was checked by bradnelson@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_android_arm_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/...) v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...) v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...) v8_linux64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_rel_ng/builds/17591) v8_linux_arm64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel_ng/build...) v8_linux_dbg_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_dbg_ng/builds/17650) v8_linux_gcc_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_gcc_compile_rel/bu...) v8_linux_mips64el_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_r...) v8_linux_mipsel_compile_rel on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mipsel_compile_rel...) v8_linux_nodcheck_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_nodcheck_rel_ng/bu...) v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/17565) v8_presubmit on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/30349) v8_win64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/19170) v8_win_compile_dbg on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_compile_dbg/builds/2...) v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...) v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/19296)
The CQ bit was checked by bradnelson@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by bradnelson@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== [wasm][asm.js] Forbid duplicate exports in asm.js. BUG=672789 R=titzer@chromium.org ========== to ========== [wasm][asm.js] Ignore duplicate exports in asm.js. BUG=672789 R=titzer@chromium.org ==========
PTAL (Doesn't seem to be a way to delete OwnProperties)
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
lgtm, although I don't know why you are crashing. https://codereview.chromium.org/2559113006/diff/40001/src/wasm/wasm-module.cc File src/wasm/wasm-module.cc (right): https://codereview.chromium.org/2559113006/diff/40001/src/wasm/wasm-module.cc... src/wasm/wasm-module.cc:1756: for (auto exp : module_->export_table) { Couldn't you just set it to module_->export_table.size()?
The CQ bit was checked by bradnelson@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
bradnelson@google.com changed reviewers: + bradnelson@google.com
https://codereview.chromium.org/2559113006/diff/40001/src/wasm/wasm-module.cc File src/wasm/wasm-module.cc (right): https://codereview.chromium.org/2559113006/diff/40001/src/wasm/wasm-module.cc... src/wasm/wasm-module.cc:1756: for (auto exp : module_->export_table) { On 2016/12/12 12:59:10, titzer wrote: > Couldn't you just set it to module_->export_table.size()? Ah, wasn't gating on the condition, that's why it was failing. Fixed.
The CQ bit was unchecked by bradnelson@google.com
The CQ bit was checked by bradnelson@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from titzer@chromium.org Link to the patchset: https://codereview.chromium.org/2559113006/#ps60001 (title: "fix")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481550842712090,
"parent_rev": "43dd50698ff253d029601734ad66d45c6f09c93f", "commit_rev":
"0885c5916010ab3995929a87de2a8e233f5d8a76"}
Message was sent while issue was closed.
Description was changed from ========== [wasm][asm.js] Ignore duplicate exports in asm.js. BUG=672789 R=titzer@chromium.org ========== to ========== [wasm][asm.js] Ignore duplicate exports in asm.js. BUG=672789 R=titzer@chromium.org Review-Url: https://codereview.chromium.org/2559113006 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== [wasm][asm.js] Ignore duplicate exports in asm.js. BUG=672789 R=titzer@chromium.org Review-Url: https://codereview.chromium.org/2559113006 ========== to ========== [wasm][asm.js] Ignore duplicate exports in asm.js. BUG=672789 R=titzer@chromium.org Committed: https://crrev.com/be9ee2237d7278665c4204a4b72654122bc95e26 Cr-Commit-Position: refs/heads/master@{#41647} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/be9ee2237d7278665c4204a4b72654122bc95e26 Cr-Commit-Position: refs/heads/master@{#41647} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
