include fields in verify_deps, refactor graph dfs

include fields in verify_deps, refactor graph dfs

BUG=
R=sigmund@google.com

Committed: https://github.com/dart-lang/dart2js_info/commit/0eb54e7a1278ff35725de26827638fe54b731d56

[Harry Terkelsen, 2015-10-16 22:55:11]

het@google.com changed reviewers:
+ sigmund@google.com

[Siggi Cherem (dart-lang), 2015-10-16 23:07:41]

lgtm, just some questions/suggestions below.

https://codereview.chromium.org/1412903002/diff/1/lib/src/graph.dart
File lib/src/graph.dart (right):

https://codereview.chromium.org/1412903002/diff/1/lib/src/graph.dart#newcode49
lib/src/graph.dart:49: targetsOf(n).forEach(helper);
should we switch this implementation to avoid recursion as well?

https://codereview.chromium.org/1412903002/diff/1/lib/src/graph.dart#newcode56
lib/src/graph.dart:56: /// Returns an iterable of all nodes reachable from
[root].
maybe indicate that they are in preorder?

https://codereview.chromium.org/1412903002/diff/1/lib/src/graph.dart#newcode57
lib/src/graph.dart:57: Iterable<N> reachableFrom(N root) sync* {
maybe rename this and/or `postOrder` above so we have a consistent naming
convention among the two?

https://codereview.chromium.org/1412903002/diff/1/lib/src/graph.dart#newcode63
lib/src/graph.dart:63: yield next;
nice use of sync*/yield :)

[Harry Terkelsen, 2015-10-19 22:11:12]

https://codereview.chromium.org/1412903002/diff/1/lib/src/graph.dart
File lib/src/graph.dart (right):

https://codereview.chromium.org/1412903002/diff/1/lib/src/graph.dart#newcode49
lib/src/graph.dart:49: targetsOf(n).forEach(helper);
On 2015/10/16 23:07:41, Siggi Cherem (dart-lang) wrote:
> should we switch this implementation to avoid recursion as well?

I've refactored it to use yield*, what do you think?

https://codereview.chromium.org/1412903002/diff/1/lib/src/graph.dart#newcode56
lib/src/graph.dart:56: /// Returns an iterable of all nodes reachable from
[root].
On 2015/10/16 23:07:41, Siggi Cherem (dart-lang) wrote:
> maybe indicate that they are in preorder?

Done.

https://codereview.chromium.org/1412903002/diff/1/lib/src/graph.dart#newcode57
lib/src/graph.dart:57: Iterable<N> reachableFrom(N root) sync* {
On 2015/10/16 23:07:41, Siggi Cherem (dart-lang) wrote:
> maybe rename this and/or `postOrder` above so we have a consistent naming
> convention among the two?

Done.

https://codereview.chromium.org/1412903002/diff/1/lib/src/graph.dart#newcode63
lib/src/graph.dart:63: yield next;
On 2015/10/16 23:07:41, Siggi Cherem (dart-lang) wrote:
> nice use of sync*/yield :)

thanks!

[Harry Terkelsen, 2015-10-19 22:20:28]

Committed patchset #3 (id:40001) manually as
0eb54e7a1278ff35725de26827638fe54b731d56 (presubmit successful).