Remove printDebugInfo from Unparser

Remove printDebugInfo from Unparser


BUG=
TEST=

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

[Roman, 2012-06-05 09:29:44]

ping. Or did I forgot to mail this issue to you guys?

[Anton Muhin, 2012-06-05 09:35:19]

almost LGTM

https://chromiumcodereview.appspot.com/10456080/diff/1/lib/compiler/implement...
File lib/compiler/implementation/tree/nodes.dart (right):

https://chromiumcodereview.appspot.com/10456080/diff/1/lib/compiler/implement...
lib/compiler/implementation/tree/nodes.dart:106: String unparse(bool
printDebugInfo) {
do we really need to bundle both?

I'd rather have prettyprint and unparse methods.

https://chromiumcodereview.appspot.com/10456080/diff/1/lib/compiler/implement...
File lib/compiler/implementation/tree/unparser.dart (left):

https://chromiumcodereview.appspot.com/10456080/diff/1/lib/compiler/implement...
lib/compiler/implementation/tree/unparser.dart:22: if (node !== null) {
probably it's time to go single line now: if (node !== null) node.accept(this);

[Roman, 2012-06-05 11:09:06]

https://chromiumcodereview.appspot.com/10456080/diff/1/lib/compiler/implement...
File lib/compiler/implementation/tree/nodes.dart (right):

https://chromiumcodereview.appspot.com/10456080/diff/1/lib/compiler/implement...
lib/compiler/implementation/tree/nodes.dart:106: String unparse(bool
printDebugInfo) {
On 2012/06/05 09:35:19, antonmuhin wrote:
> do we really need to bundle both?
> 
> I'd rather have prettyprint and unparse methods.

I'd rather too, but there was a usage in tests that could benefit from printing
both.
However, I don't think that's important, there is only one place where
"toDebugString" is used (tests), so I just removed that thing.

https://chromiumcodereview.appspot.com/10456080/diff/1/lib/compiler/implement...
File lib/compiler/implementation/tree/unparser.dart (left):

https://chromiumcodereview.appspot.com/10456080/diff/1/lib/compiler/implement...
lib/compiler/implementation/tree/unparser.dart:22: if (node !== null) {
On 2012/06/05 09:35:19, antonmuhin wrote:
> probably it's time to go single line now: if (node !== null)
node.accept(this);

Done.

By the way, Dart style guide is ambiguous:
http://www.dartlang.org/articles/style-guide/

On one hand, it says: "DO use curly braces for all flow control structures.",
but later there is "green example":
if (obj is! SomeType) print('not SomeType');

[Anton Muhin, 2012-06-05 11:39:21]

lgtm

https://chromiumcodereview.appspot.com/10456080/diff/1/lib/compiler/implement...
File lib/compiler/implementation/tree/unparser.dart (left):

https://chromiumcodereview.appspot.com/10456080/diff/1/lib/compiler/implement...
lib/compiler/implementation/tree/unparser.dart:22: if (node !== null) {
You may want to file a bug.

Historically, at least AAR people use single line variants a lot.

On 2012/06/05 11:09:07, Roman wrote:
> On 2012/06/05 09:35:19, antonmuhin wrote:
> > probably it's time to go single line now: if (node !== null)
> node.accept(this);
> 
> Done.
> 
> By the way, Dart style guide is ambiguous:
> http://www.dartlang.org/articles/style-guide/
> 
> On one hand, it says: "DO use curly braces for all flow control structures.",
> but later there is "green example":
> if (obj is! SomeType) print('not SomeType');

[Roman, 2012-06-07 13:28:26]

Adding Nicolas because Peter is too busy with other issues.

[ahe, 2012-06-08 07:43:35]

https://chromiumcodereview.appspot.com/10456080/diff/4001/lib/compiler/implem...
File lib/compiler/implementation/tree/nodes.dart (left):

https://chromiumcodereview.appspot.com/10456080/diff/4001/lib/compiler/implem...
lib/compiler/implementation/tree/nodes.dart:95: toDebugString() =>
unparse(true);
I assumed we would keep toDebugString, but use your new pretty printer instead.

[Roman, 2012-06-08 08:13:12]

https://chromiumcodereview.appspot.com/10456080/diff/4001/lib/compiler/implem...
File lib/compiler/implementation/tree/nodes.dart (left):

https://chromiumcodereview.appspot.com/10456080/diff/4001/lib/compiler/implem...
lib/compiler/implementation/tree/nodes.dart:95: toDebugString() =>
unparse(true);
On 2012/06/08 07:43:36, ahe wrote:
> I assumed we would keep toDebugString, but use your new pretty printer
instead.

Yes, that was the original plan. But then I saw that it is only used in one
test. Do you still think that it would be useful to keep this method?

[ahe, 2012-06-08 08:16:56]

https://chromiumcodereview.appspot.com/10456080/diff/4001/lib/compiler/implem...
File lib/compiler/implementation/tree/nodes.dart (left):

https://chromiumcodereview.appspot.com/10456080/diff/4001/lib/compiler/implem...
lib/compiler/implementation/tree/nodes.dart:95: toDebugString() =>
unparse(true);
On 2012/06/08 08:13:12, Roman wrote:
> On 2012/06/08 07:43:36, ahe wrote:
> > I assumed we would keep toDebugString, but use your new pretty printer
> instead.
> 
> Yes, that was the original plan. But then I saw that it is only used in one
> test. Do you still think that it would be useful to keep this method?

Yes. We often use the method for debugging by "print".

[Roman, 2012-06-08 12:33:33]

https://chromiumcodereview.appspot.com/10456080/diff/4001/lib/compiler/implem...
File lib/compiler/implementation/tree/nodes.dart (left):

https://chromiumcodereview.appspot.com/10456080/diff/4001/lib/compiler/implem...
lib/compiler/implementation/tree/nodes.dart:95: toDebugString() =>
unparse(true);
On 2012/06/08 08:16:56, ahe wrote:
> On 2012/06/08 08:13:12, Roman wrote:
> > On 2012/06/08 07:43:36, ahe wrote:
> > > I assumed we would keep toDebugString, but use your new pretty printer
> > instead.
> > 
> > Yes, that was the original plan. But then I saw that it is only used in one
> > test. Do you still think that it would be useful to keep this method?
> 
> Yes. We often use the method for debugging by "print".

I see. Got back toDebugString() that returns node tree.

[ahe, 2012-06-08 13:05:18]

LGTM!

https://chromiumcodereview.appspot.com/10456080/diff/10005/lib/compiler/imple...
File lib/compiler/implementation/tree_validator.dart (right):

https://chromiumcodereview.appspot.com/10456080/diff/10005/lib/compiler/imple...
lib/compiler/implementation/tree_validator.dart:63: String nodeString = new
Unparser().unparse(node);
I think this should be node.toDebugString().

[Roman, 2012-06-08 13:18:51]

https://chromiumcodereview.appspot.com/10456080/diff/10005/lib/compiler/imple...
File lib/compiler/implementation/tree_validator.dart (right):

https://chromiumcodereview.appspot.com/10456080/diff/10005/lib/compiler/imple...
lib/compiler/implementation/tree_validator.dart:63: String nodeString = new
Unparser().unparse(node);
On 2012/06/08 13:05:18, ahe wrote:
> I think this should be node.toDebugString().

Done.