Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(715)

Issue 2611643002: [modules] Add an IsModule flag to ScriptOriginOptions. (Closed)

Created:
3 years, 11 months ago by neis
Modified:
3 years, 11 months ago
CC:
v8-reviews_googlegroups.com, rmcilroy
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[modules] Add an IsModule flag to ScriptOriginOptions. Since the script origin is part of the key used in the compilation cache, this ensures that the cache never confuses a module with a non-module script. BUG=v8:1569, v8:5685 Review-Url: https://codereview.chromium.org/2611643002 Cr-Commit-Position: refs/heads/master@{#42490} Committed: https://chromium.googlesource.com/v8/v8/+/b0f5abbea372b0cc4d27f267de271766f5c8a11a

Patch Set 1 #

Total comments: 1

Patch Set 2 : Rebase. #

Patch Set 3 : Remove convenience function from API. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+70 lines, -38 lines) Patch
M include/v8.h View 1 2 6 chunks +26 lines, -8 lines 0 comments Download
M src/api.cc View 1 6 chunks +14 lines, -11 lines 0 comments Download
M src/bootstrapper.cc View 1 2 chunks +2 lines, -3 lines 0 comments Download
M src/compiler.h View 1 1 chunk +1 line, -1 line 0 comments Download
M src/compiler.cc View 1 2 chunks +2 lines, -3 lines 0 comments Download
M src/d8.cc View 1 2 1 chunk +3 lines, -1 line 0 comments Download
M src/objects.h View 1 1 chunk +1 line, -1 line 0 comments Download
M test/cctest/compiler/test-linkage.cc View 1 chunk +1 line, -1 line 0 comments Download
M test/cctest/interpreter/bytecode-expectations-printer.cc View 1 2 1 chunk +5 lines, -1 line 0 comments Download
M test/cctest/test-compiler.cc View 1 1 chunk +1 line, -1 line 0 comments Download
M test/cctest/test-modules.cc View 1 2 5 chunks +12 lines, -4 lines 0 comments Download
M test/cctest/test-serialize.cc View 1 2 chunks +2 lines, -3 lines 0 comments Download

Messages

Total messages: 21 (15 generated)
neis
3 years, 11 months ago (2017-01-03 16:00:16 UTC) #8
adamk
This looks right to me except for the ScriptOrigin::ModuleOrigin helper. https://codereview.chromium.org/2611643002/diff/1/include/v8.h File include/v8.h (right): https://codereview.chromium.org/2611643002/diff/1/include/v8.h#newcode1009 ...
3 years, 11 months ago (2017-01-03 18:41:32 UTC) #9
neis
On 2017/01/03 18:41:32, adamk wrote: > This looks right to me except for the ScriptOrigin::ModuleOrigin ...
3 years, 11 months ago (2017-01-18 12:13:27 UTC) #10
adamk
lgtm
3 years, 11 months ago (2017-01-18 17:34:00 UTC) #16
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2611643002/40001
3 years, 11 months ago (2017-01-19 06:57:23 UTC) #18
commit-bot: I haz the power
3 years, 11 months ago (2017-01-19 06:59:26 UTC) #21
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/v8/v8/+/b0f5abbea372b0cc4d27f267de271766f5c...

Powered by Google App Engine
This is Rietveld 408576698