Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2521513004:
Preparse inner functions: fix maybe_assigned (Closed)
|
Created:
4 years, 1 month ago by marja Modified:
4 years, 1 month ago Reviewers:
Toon Verwaest CC:
v8-reviews_googlegroups.com Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
DescriptionPreparse inner functions: fix maybe_assigned
... but be less pessimistic about context allocation (see below).
We might have just (pessimistically) context-allocated a variable based
on references coming from an inner function, but after that we still
need to set maybe_assigned (pessimistically).
This makes test-parsing/InnerAssignment pass with
FLAG_lazy_inner_functions.
This was undetected until now because we didn't have lazy parsing enabled
for small scripts.
Less pessimistic approach: now that inner functions laziness decisions
are stable (if we have once compiled a piece of code with lazy inner
functions, we never compile the same code with eager inner functions),
we don't need to be as pessimistic with context allocation as before.
BUG=v8:5501
Committed: https://crrev.com/d2e90c5d8100d4d7933c942111946645f3d61941
Cr-Commit-Position: refs/heads/master@{#41183}
Patch Set 1 #Patch Set 2 : less pessimistic #
Total comments: 1
Patch Set 3 : even better #
Messages
Total messages: 21 (15 generated)
The CQ bit was checked by marja@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 marja@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...
marja@chromium.org changed reviewers: + verwaest@chromium.org
ptal
Description was changed from ========== Preparse inner functions: fix maybe_assigned We might have just (pessimistically) context-allocated a variable based on references coming from an inner function, but after that we still need to set maybe_assigned (pessimistically). This makes test-parsing/InnerAssignment pass with FLAG_lazy_inner_functions. This was undetected until now because we didn't have lazy parsing enabed for small scripts. BUG=v8:5501 ========== to ========== Preparse inner functions: fix maybe_assigned ... but be less pessimistic about context allocation (see below). We might have just (pessimistically) context-allocated a variable based on references coming from an inner function, but after that we still need to set maybe_assigned (pessimistically). This makes test-parsing/InnerAssignment pass with FLAG_lazy_inner_functions. This was undetected until now because we didn't have lazy parsing enabed for small scripts. Less pessimistic approach: now that inner functions laziness decisions are stable (if we have once compiled a piece of code with lazy inner functions, we never compile the same code with eager inner functions), we don't need to be as pessimistic with context allocation as before. BUG=v8:5501 ==========
Description was changed from ========== Preparse inner functions: fix maybe_assigned ... but be less pessimistic about context allocation (see below). We might have just (pessimistically) context-allocated a variable based on references coming from an inner function, but after that we still need to set maybe_assigned (pessimistically). This makes test-parsing/InnerAssignment pass with FLAG_lazy_inner_functions. This was undetected until now because we didn't have lazy parsing enabed for small scripts. Less pessimistic approach: now that inner functions laziness decisions are stable (if we have once compiled a piece of code with lazy inner functions, we never compile the same code with eager inner functions), we don't need to be as pessimistic with context allocation as before. BUG=v8:5501 ========== to ========== Preparse inner functions: fix maybe_assigned ... but be less pessimistic about context allocation (see below). We might have just (pessimistically) context-allocated a variable based on references coming from an inner function, but after that we still need to set maybe_assigned (pessimistically). This makes test-parsing/InnerAssignment pass with FLAG_lazy_inner_functions. This was undetected until now because we didn't have lazy parsing enabled for small scripts. Less pessimistic approach: now that inner functions laziness decisions are stable (if we have once compiled a piece of code with lazy inner functions, we never compile the same code with eager inner functions), we don't need to be as pessimistic with context allocation as before. BUG=v8:5501 ==========
https://codereview.chromium.org/2521513004/diff/20001/src/ast/scopes.cc File src/ast/scopes.cc (right): https://codereview.chromium.org/2521513004/diff/20001/src/ast/scopes.cc#newco... src/ast/scopes.cc:1624: var->set_maybe_assigned(); I don't think we need to set is_used here, since ResolveTo will already set it. What about making proxy->is_assigned() be true if it comes from the preparser (through AnalyzePartially + FLAG_lazy_inner_functions I presume).
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by marja@chromium.org
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": 40001, "attempt_start_ts": 1479822591331470,
"parent_rev": "52be18577d5d2515b032e82499df68774b0b287a", "commit_rev":
"ddac80f4c9582645a7705e97efa270ccfd36b277"}
Message was sent while issue was closed.
Description was changed from ========== Preparse inner functions: fix maybe_assigned ... but be less pessimistic about context allocation (see below). We might have just (pessimistically) context-allocated a variable based on references coming from an inner function, but after that we still need to set maybe_assigned (pessimistically). This makes test-parsing/InnerAssignment pass with FLAG_lazy_inner_functions. This was undetected until now because we didn't have lazy parsing enabled for small scripts. Less pessimistic approach: now that inner functions laziness decisions are stable (if we have once compiled a piece of code with lazy inner functions, we never compile the same code with eager inner functions), we don't need to be as pessimistic with context allocation as before. BUG=v8:5501 ========== to ========== Preparse inner functions: fix maybe_assigned ... but be less pessimistic about context allocation (see below). We might have just (pessimistically) context-allocated a variable based on references coming from an inner function, but after that we still need to set maybe_assigned (pessimistically). This makes test-parsing/InnerAssignment pass with FLAG_lazy_inner_functions. This was undetected until now because we didn't have lazy parsing enabled for small scripts. Less pessimistic approach: now that inner functions laziness decisions are stable (if we have once compiled a piece of code with lazy inner functions, we never compile the same code with eager inner functions), we don't need to be as pessimistic with context allocation as before. BUG=v8:5501 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Preparse inner functions: fix maybe_assigned ... but be less pessimistic about context allocation (see below). We might have just (pessimistically) context-allocated a variable based on references coming from an inner function, but after that we still need to set maybe_assigned (pessimistically). This makes test-parsing/InnerAssignment pass with FLAG_lazy_inner_functions. This was undetected until now because we didn't have lazy parsing enabled for small scripts. Less pessimistic approach: now that inner functions laziness decisions are stable (if we have once compiled a piece of code with lazy inner functions, we never compile the same code with eager inner functions), we don't need to be as pessimistic with context allocation as before. BUG=v8:5501 ========== to ========== Preparse inner functions: fix maybe_assigned ... but be less pessimistic about context allocation (see below). We might have just (pessimistically) context-allocated a variable based on references coming from an inner function, but after that we still need to set maybe_assigned (pessimistically). This makes test-parsing/InnerAssignment pass with FLAG_lazy_inner_functions. This was undetected until now because we didn't have lazy parsing enabled for small scripts. Less pessimistic approach: now that inner functions laziness decisions are stable (if we have once compiled a piece of code with lazy inner functions, we never compile the same code with eager inner functions), we don't need to be as pessimistic with context allocation as before. BUG=v8:5501 Committed: https://crrev.com/d2e90c5d8100d4d7933c942111946645f3d61941 Cr-Commit-Position: refs/heads/master@{#41183} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/d2e90c5d8100d4d7933c942111946645f3d61941 Cr-Commit-Position: refs/heads/master@{#41183} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
