Enable visitor in rewriter to replace statements.

Enable visitor in rewriter to replace statements.

This is in preparation of implementing ES6 completion semantics and
depends on #1362333002.

R=rossberg
BUG=

Committed: https://crrev.com/af33cccfc4b8b806cbc54a1c72a6b5682312df8d
Cr-Commit-Position: refs/heads/master@{#31041}

[rossberg, 2015-09-28 11:42:15]

https://codereview.chromium.org/1362363002/diff/1/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/1362363002/diff/1/src/ast.h#newcode1137
src/ast.h:1137: void set_then_statement(Statement* s) { then_statement_ = s; }
Can you add assertions DCHECK(s != nullptr) to all these setters?

https://codereview.chromium.org/1362363002/diff/1/src/rewriter.cc
File src/rewriter.cc (right):

https://codereview.chromium.org/1362363002/diff/1/src/rewriter.cc#newcode66
src/rewriter.cc:66: statements->Set(i, replacement_);
How about treating replacement_ as an option and changing the logic here (and in
the other places that set a subnode) to

  if (replacement_ != nullptr) {
    statements->Set(...);
    replacement_ = nullptr;
  }

Then you only need to assign something to replacement_ in the few visit methods
where you actually need to change the node.

[neis, 2015-09-28 15:18:45]

On 2015/09/28 11:42:15, rossberg wrote:
> https://codereview.chromium.org/1362363002/diff/1/src/ast.h
> File src/ast.h (right):
> 
> https://codereview.chromium.org/1362363002/diff/1/src/ast.h#newcode1137
> src/ast.h:1137: void set_then_statement(Statement* s) { then_statement_ = s; }
> Can you add assertions DCHECK(s != nullptr) to all these setters?

I'm doing the same checks as the corresponding constructors do, i.e. none :)

> https://codereview.chromium.org/1362363002/diff/1/src/rewriter.cc
> File src/rewriter.cc (right):
> 
> https://codereview.chromium.org/1362363002/diff/1/src/rewriter.cc#newcode66
> src/rewriter.cc:66: statements->Set(i, replacement_);
> How about treating replacement_ as an option and changing the logic here (and
in
> the other places that set a subnode) to
> 
>   if (replacement_ != nullptr) {
>     statements->Set(...);
>     replacement_ = nullptr;
>   }
> 
> Then you only need to assign something to replacement_ in the few visit
methods
> where you actually need to change the node.

As discussed, that would be in many places so in the end it doesn't really
matter.

I did, however, replace the static cast to Block with a call to AsBlock().

[neis, 2015-09-28 15:19:11]

On 2015/09/28 15:18:45, neis wrote:
> On 2015/09/28 11:42:15, rossberg wrote:
> > https://codereview.chromium.org/1362363002/diff/1/src/ast.h
> > File src/ast.h (right):
> > 
> > https://codereview.chromium.org/1362363002/diff/1/src/ast.h#newcode1137
> > src/ast.h:1137: void set_then_statement(Statement* s) { then_statement_ = s;
}
> > Can you add assertions DCHECK(s != nullptr) to all these setters?
> 
> I'm doing the same checks as the corresponding constructors do, i.e. none :)
> 
> > https://codereview.chromium.org/1362363002/diff/1/src/rewriter.cc
> > File src/rewriter.cc (right):
> > 
> > https://codereview.chromium.org/1362363002/diff/1/src/rewriter.cc#newcode66
> > src/rewriter.cc:66: statements->Set(i, replacement_);
> > How about treating replacement_ as an option and changing the logic here
(and
> in
> > the other places that set a subnode) to
> > 
> >   if (replacement_ != nullptr) {
> >     statements->Set(...);
> >     replacement_ = nullptr;
> >   }
> > 
> > Then you only need to assign something to replacement_ in the few visit
> methods
> > where you actually need to change the node.
> 
> As discussed, that would be in many places so in the end it doesn't really
> matter.
> 
> I did, however, replace the static cast to Block with a call to AsBlock().

Please ignore the changes that sneaked in from the other CL.

[neis, 2015-09-28 15:26:59]

The CQ bit was checked by neis@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-28 15:27:07]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1362363002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1362363002/20001

[commit-bot: I haz the power, 2015-09-28 15:28:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-28 15:28:08]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel/builds/...)
  v8_linux_arm64_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm64_rel/builds/1...)

[neis, 2015-09-28 15:44:12]

The CQ bit was checked by neis@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-28 15:44:25]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1362363002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1362363002/40001

[rossberg, 2015-09-28 16:01:03]

lgtm

[commit-bot: I haz the power, 2015-09-28 18:05:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-28 18:05:47]

Dry run: This issue passed the CQ dry run.

[neis, 2015-10-01 08:43:01]

The CQ bit was checked by neis@chromium.org

[neis, 2015-10-01 08:43:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
rossberg@chromium.org
Link to the patchset: https://codereview.chromium.org/1362363002/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2015-10-01 08:43:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1362363002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1362363002/60001

[commit-bot: I haz the power, 2015-10-01 09:06:11]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-10-01 09:06:24]

Patchset 4 (id:??) landed as
https://crrev.com/af33cccfc4b8b806cbc54a1c72a6b5682312df8d
Cr-Commit-Position: refs/heads/master@{#31041}