Chromium Code Reviews
|
|
Chromium Code Reviews|
Created:
4 years, 4 months ago by adamk Modified:
4 years, 4 months ago Reviewers:
neis CC:
v8-reviews_googlegroups.com, Toon Verwaest Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionRemove unnecessary checks for forced context allocation
If a whole scope has forced context allocation, MustAllocateInContext()
already takes care of returning true for all variables, there's no need
to individually call ForceContextAllocation() on them.
R=neis@chromium.org
Committed: https://crrev.com/86e141eb2ddf89b98f8629c73e94440f83651f7f
Cr-Commit-Position: refs/heads/master@{#38504}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Removed comment #
Created: 4 years, 4 months ago
Messages
Total messages: 15 (9 generated)
The CQ bit was checked by adamk@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.
lgtm, thanks! https://codereview.chromium.org/2222843005/diff/1/src/ast/scopes.cc File src/ast/scopes.cc (right): https://codereview.chromium.org/2222843005/diff/1/src/ast/scopes.cc#newcode1581 src/ast/scopes.cc:1581: // Force context allocation of the parameter. Maybe remove the comment too as it doesn't add any information.
Description was changed from ========== Remove unnecessary checks for forced context allocation If a whole scope has forced context allocation, MustAllocateInContext() already takes care of returning true for all variables, there's no need to individually call ForceContextAllocation() on them. R=neis@chromium.org ========== to ========== Remove unnecessary checks for forced context allocation If a whole scope has forced context allocation, MustAllocateInContext() already takes care of returning true for all variables, there's no need to individually call ForceContextAllocation() on them. R=neis@chromium.org ==========
The CQ bit was checked by adamk@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from neis@chromium.org Link to the patchset: https://codereview.chromium.org/2222843005/#ps20001 (title: "Removed comment")
https://codereview.chromium.org/2222843005/diff/1/src/ast/scopes.cc File src/ast/scopes.cc (right): https://codereview.chromium.org/2222843005/diff/1/src/ast/scopes.cc#newcode1581 src/ast/scopes.cc:1581: // Force context allocation of the parameter. On 2016/08/09 07:15:26, neis wrote: > Maybe remove the comment too as it doesn't add any information. Done.
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Remove unnecessary checks for forced context allocation If a whole scope has forced context allocation, MustAllocateInContext() already takes care of returning true for all variables, there's no need to individually call ForceContextAllocation() on them. R=neis@chromium.org ========== to ========== Remove unnecessary checks for forced context allocation If a whole scope has forced context allocation, MustAllocateInContext() already takes care of returning true for all variables, there's no need to individually call ForceContextAllocation() on them. R=neis@chromium.org ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Remove unnecessary checks for forced context allocation If a whole scope has forced context allocation, MustAllocateInContext() already takes care of returning true for all variables, there's no need to individually call ForceContextAllocation() on them. R=neis@chromium.org ========== to ========== Remove unnecessary checks for forced context allocation If a whole scope has forced context allocation, MustAllocateInContext() already takes care of returning true for all variables, there's no need to individually call ForceContextAllocation() on them. R=neis@chromium.org Committed: https://crrev.com/86e141eb2ddf89b98f8629c73e94440f83651f7f Cr-Commit-Position: refs/heads/master@{#38504} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/86e141eb2ddf89b98f8629c73e94440f83651f7f Cr-Commit-Position: refs/heads/master@{#38504} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
