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

Issue 2586463002: Add CreateDataPropertyInLiteralFlags. (Closed)

Created:
4 years ago by Franzi
Modified:
4 years ago
Reviewers:
Benedikt Meurer
CC:
v8-reviews_googlegroups.com, rmcilroy
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Add CreateDataPropertyInLiteralFlags. Encode the PropertyAttribute and whether the function names must be set as a flag instead of setting two registers. BUG=v8:5624 Review-Url: https://codereview.chromium.org/2586463002 Cr-Commit-Position: refs/heads/master@{#41812} Committed: https://chromium.googlesource.com/v8/v8/+/86eafdd4c0a7f3f184cf89a252e14547f5070ea4

Patch Set 1 #

Patch Set 2 : Minor refactoring. #

Patch Set 3 : 4 is not 5. #

Patch Set 4 : Delete unused function #

Patch Set 5 : Do not use anonymous bools in flag. #

Patch Set 6 : Rebase. #

Patch Set 7 : Rebaseline bytecode expectations. #

Total comments: 3

Patch Set 8 : Address review comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+76 lines, -85 lines) Patch
M src/compiler/bytecode-graph-builder.h View 1 2 3 1 chunk +0 lines, -6 lines 0 comments Download
M src/compiler/bytecode-graph-builder.cc View 1 2 3 4 5 1 chunk +2 lines, -4 lines 0 comments Download
M src/compiler/js-operator.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M src/globals.h View 1 2 3 4 5 6 7 2 chunks +12 lines, -0 lines 0 comments Download
M src/interpreter/bytecode-array-builder.h View 1 2 3 4 5 1 chunk +3 lines, -4 lines 0 comments Download
M src/interpreter/bytecode-array-builder.cc View 1 2 3 4 5 1 chunk +3 lines, -2 lines 0 comments Download
M src/interpreter/bytecode-generator.cc View 1 2 3 4 5 6 7 2 chunks +12 lines, -9 lines 0 comments Download
M src/interpreter/bytecodes.h View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M src/interpreter/interpreter.cc View 1 2 3 4 5 2 chunks +6 lines, -9 lines 0 comments Download
M src/runtime/runtime.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M src/runtime/runtime-object.cc View 1 2 3 4 5 1 chunk +10 lines, -4 lines 0 comments Download
M test/cctest/interpreter/bytecode_expectations/ClassDeclarations.golden View 1 2 3 4 5 6 9 chunks +12 lines, -20 lines 0 comments Download
M test/cctest/interpreter/bytecode_expectations/ObjectLiterals.golden View 8 chunks +10 lines, -22 lines 0 comments Download
M test/unittests/interpreter/bytecode-array-builder-unittest.cc View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 33 (29 generated)
Franzi
PTAL Thanks, Franzi
4 years ago (2016-12-19 09:19:45 UTC) #16
Benedikt Meurer
Very nice. LGTM % nits. https://codereview.chromium.org/2586463002/diff/120001/src/globals.h File src/globals.h (right): https://codereview.chromium.org/2586463002/diff/120001/src/globals.h#newcode1314 src/globals.h:1314: enum class DataPropertyInLiteralFlag { ...
4 years ago (2016-12-19 15:47:35 UTC) #27
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/2586463002/140001
4 years ago (2016-12-19 16:00:49 UTC) #30
commit-bot: I haz the power
4 years ago (2016-12-19 16:30:45 UTC) #33
Message was sent while issue was closed.
Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/v8/v8/+/86eafdd4c0a7f3f184cf89a252e14547f50...

Powered by Google App Engine
This is Rietveld 408576698