Avoid printing braces for one-statement blocks in js-printer.

Avoid printing braces for one-statement blocks in js-printer.

BUG=
R=floitsch@google.com

Committed: https://code.google.com/p/dart/source/detail?r=43956

Reverted: https://codereview.chromium.org/954493002/

Committed: https://code.google.com/p/dart/source/detail?r=44016

[sigurdm, 2015-02-23 08:46:41]

Patchset #1 (id:1) has been deleted

[sigurdm, 2015-02-23 08:47:01]

sigurdm@google.com changed reviewers:
+ floitsch@google.com

[floitsch, 2015-02-23 13:46:08]

I think this broke the dangling-else.

Not sure what the best fix for the dangling-else is.
Options (afaics):
- remove the surrounding blocks as an optimization earlier.
- update the dangling-else visitor.
- be more conservative if 'if' has an 'else'.

https://codereview.chromium.org/949753004/diff/20001/pkg/compiler/lib/src/js/...
File pkg/compiler/lib/src/js/printer.dart (right):

https://codereview.chromium.org/949753004/diff/20001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/printer.dart:278: bool needsBracesInThen = hasElse
bool needsBracesInThen =
    hasElse && (then.accept(danglingElseVisitor) || then is Do);

https://codereview.chromium.org/949753004/diff/20001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/printer.dart:294: } else {
This probably broke the dangling-else:

if (x) {
  if (y) print('foo');
} else print('bar');

The danglingElseVisitor says that 'node.then' is a block and therefore doesn't
need braces.

However, the 'blockBody' will now remove it.

[sigurdm, 2015-02-23 14:13:22]

PTAL

https://codereview.chromium.org/949753004/diff/20001/pkg/compiler/lib/src/js/...
File pkg/compiler/lib/src/js/printer.dart (right):

https://codereview.chromium.org/949753004/diff/20001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/printer.dart:278: bool needsBracesInThen = hasElse
On 2015/02/23 13:46:08, floitsch wrote:
> bool needsBracesInThen =
>     hasElse && (then.accept(danglingElseVisitor) || then is Do);

Acknowledged.
This change has been undone.

https://codereview.chromium.org/949753004/diff/20001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/printer.dart:294: } else {
On 2015/02/23 13:46:08, floitsch wrote:
> This probably broke the dangling-else:
> 
> if (x) {
>   if (y) print('foo');
> } else print('bar');
> 
> The danglingElseVisitor says that 'node.then' is a block and therefore doesn't
> need braces.
> 
> However, the 'blockBody' will now remove it.
Yes, I just realized that. I will remove the blocks as a pass before.

[floitsch, 2015-02-23 14:26:05]

LGTM.

https://codereview.chromium.org/949753004/diff/40001/pkg/compiler/lib/src/js/...
File pkg/compiler/lib/src/js/printer.dart (right):

https://codereview.chromium.org/949753004/diff/40001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/printer.dart:207: Statement
unwrapIfSingleStatement(Statement body) {
unwrapBlockIfSingleStatement ?

[sigurdm, 2015-02-23 14:32:36]

New patchsets have been uploaded after l-g-t-m from floitsch@google.com

[sigurdm, 2015-02-23 14:34:21]

https://codereview.chromium.org/949753004/diff/40001/pkg/compiler/lib/src/js/...
File pkg/compiler/lib/src/js/printer.dart (right):

https://codereview.chromium.org/949753004/diff/40001/pkg/compiler/lib/src/js/...
pkg/compiler/lib/src/js/printer.dart:207: Statement
unwrapIfSingleStatement(Statement body) {
On 2015/02/23 14:26:05, floitsch wrote:
> unwrapBlockIfSingleStatement ?

Done.

[sigurdm, 2015-02-23 14:51:08]

Committed patchset #3 (id:60001) manually as 43956 (presubmit successful).

[sigurdm, 2015-02-24 11:42:59]

PTAL

[floitsch, 2015-02-24 20:02:10]

LGTM.

https://codereview.chromium.org/949753004/diff/100001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js/printer.dart (right):

https://codereview.chromium.org/949753004/diff/100001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js/printer.dart:510: Statement safeBody = (body is Break &&
body.targetLabel == node.label)
Shouldn't we just drop the statement and make it a no-op statement ";"?

[sigurdm, 2015-02-25 08:38:03]

https://codereview.chromium.org/949753004/diff/100001/pkg/compiler/lib/src/js...
File pkg/compiler/lib/src/js/printer.dart (right):

https://codereview.chromium.org/949753004/diff/100001/pkg/compiler/lib/src/js...
pkg/compiler/lib/src/js/printer.dart:510: Statement safeBody = (body is Break &&
body.targetLabel == node.label)
On 2015/02/24 20:02:10, floitsch wrote:
> Shouldn't we just drop the statement and make it a no-op statement ";"?

Ok, I'll do that. Initially my thought was not to do non-trivial optimizations
in the printer - but on second thought this could possibly be considered a
trivial optimization.

[sigurdm, 2015-02-25 08:38:23]

Committed patchset #6 (id:120001) manually as 44016 (presubmit successful).