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

Issue 1375513002: dart2js cps: Add helpers for common IR manipulation. (Closed)

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

Description

dart2js cps: Add helpers for common IR manipulation. This adds four helpers: - InteriorExpression: remove, insertAbove, and insertBelow. - Primitive: destroy. As discussed elsewhere, using such helpers is a double-edged sword. They make the code easier to read and write, but it's easy to end up with helpers that look like they work for a particular purpose, but actually don't because of slight variations in different use-cases. These methods have a couple of pitfalls, like forgetting to unlink references after a call to `remove`, or accidentally using the body of an orphaned node. But these are generally caught quickly in checked-mode so, it seems like it will be worth it. BUG= R=kmillikin@google.com Committed: https://github.com/dart-lang/sdk/commit/85f25787834ec7b5c09ceb7f9d5a27196c0ad11b Reverted: https://github.com/dart-lang/sdk/commit/e4536dd274adf9cd3c8835ecfce98d4b75b5990e Committed: https://github.com/dart-lang/sdk/commit/808f5e2228df9c41a909fcc31c481334a14a1d2b

Patch Set 1 #

Patch Set 2 : Add extra assertion #

Patch Set 3 : Rebase #

Patch Set 4 : Revert + Unrevert #

Patch Set 5 : Bugfix #

Patch Set 6 : Fix type annotation #

Unified diffs Side-by-side diffs Delta from patch set Stats (+75 lines, -99 lines) Patch
M pkg/compiler/lib/src/cps_ir/cps_ir_nodes.dart View 1 2 3 4 5 2 chunks +42 lines, -0 lines 0 comments Download
M pkg/compiler/lib/src/cps_ir/insert_refinements.dart View 2 chunks +6 lines, -14 lines 0 comments Download
M pkg/compiler/lib/src/cps_ir/mutable_ssa.dart View 5 chunks +5 lines, -10 lines 0 comments Download
M pkg/compiler/lib/src/cps_ir/redundant_join.dart View 3 chunks +3 lines, -18 lines 0 comments Download
M pkg/compiler/lib/src/cps_ir/redundant_phi.dart View 1 2 3 4 2 chunks +4 lines, -16 lines 0 comments Download
M pkg/compiler/lib/src/cps_ir/remove_refinements.dart View 1 chunk +4 lines, -5 lines 0 comments Download
M pkg/compiler/lib/src/cps_ir/scalar_replacement.dart View 3 chunks +3 lines, -16 lines 0 comments Download
M pkg/compiler/lib/src/cps_ir/share_interceptors.dart View 4 chunks +8 lines, -20 lines 0 comments Download

Messages

Total messages: 7 (2 generated)
asgerf
5 years, 2 months ago (2015-09-28 12:40:54 UTC) #3
Kevin Millikin (Google)
LGTM! What's not to like, except for the backwards OO Yoda-talk that comes from putting ...
5 years, 2 months ago (2015-09-28 13:25:02 UTC) #4
asgerf
Committed patchset #3 (id:60001) manually as 85f25787834ec7b5c09ceb7f9d5a27196c0ad11b (presubmit successful).
5 years, 2 months ago (2015-09-29 09:26:24 UTC) #5
asgerf
I really should have tested in checked-mode after going on about how quickly these bugs ...
5 years, 2 months ago (2015-09-29 11:06:49 UTC) #6
asgerf
5 years, 2 months ago (2015-09-29 11:09:06 UTC) #7
Message was sent while issue was closed.
Committed patchset #6 (id:120001) manually as
808f5e2228df9c41a909fcc31c481334a14a1d2b (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698