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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 2188863002: Force local context allocation in finally clauses (fixes #26948). (Closed)

Created:
4 years, 4 months ago by regis
Modified:
4 years, 4 months ago
CC:
reviews_dartlang.org, vm-dev_dartlang.org
Base URL:
git@github.com:dart-lang/sdk.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

Force local context allocation in finally clauses (fixes #26948). Add regression test. R=fschneider@google.com, hausner@google.com Committed: https://github.com/dart-lang/sdk/commit/11d340c51d7d916c3c4068c341c6e5fe48a6bf40

Patch Set 1 #

Patch Set 2 : work in progress #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+36 lines, -1 line) Patch
M runtime/vm/parser.cc View 1 chunk +10 lines, -1 line 2 comments Download
A tests/language/regress_26948_test.dart View 1 1 chunk +26 lines, -0 lines 0 comments Download

Messages

Total messages: 9 (3 generated)
regis
This is a temporary fix until we avoid reparsing finally clauses.
4 years, 4 months ago (2016-07-27 20:57:36 UTC) #2
hausner
Very nice! LGTM.
4 years, 4 months ago (2016-07-27 21:00:10 UTC) #3
Florian Schneider
Lgtm!
4 years, 4 months ago (2016-07-27 21:00:47 UTC) #4
regis
Committed patchset #2 (id:20001) manually as 11d340c51d7d916c3c4068c341c6e5fe48a6bf40 (presubmit successful).
4 years, 4 months ago (2016-07-27 21:03:30 UTC) #6
zra
Thanks! https://codereview.chromium.org/2188863002/diff/20001/runtime/vm/parser.cc File runtime/vm/parser.cc (right): https://codereview.chromium.org/2188863002/diff/20001/runtime/vm/parser.cc#newcode9304 runtime/vm/parser.cc:9304: // See issue #26948. This is a temporary ...
4 years, 4 months ago (2016-07-27 21:07:23 UTC) #8
regis
4 years, 4 months ago (2016-07-27 21:27:09 UTC) #9
Message was sent while issue was closed. 
https://codereview.chromium.org/2188863002/diff/20001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/2188863002/diff/20001/runtime/vm/parser.cc#ne...
runtime/vm/parser.cc:9304: // See issue #26948. This is a temporary fix until we
eliminate reparsing.
On 2016/07/27 21:07:23, zra wrote:
> DBC: Probably a good idea to go ahead and file an issue on github for this.

Done. #26975

Powered by Google App Engine
This is Rietveld 408576698