Transitive inference using SCC

Transitive inference using SCC

BUG=
R=vsm@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/97984fa6bd10c8438b9da8af56dc5360a7fa80e6

[Siggi Cherem (dart-lang), 2015-03-20 20:42:08]

Patchset #1 (id:1) has been deleted

[Siggi Cherem (dart-lang), 2015-03-20 20:42:12]

Patchset #1 (id:20001) has been deleted

[Siggi Cherem (dart-lang), 2015-03-20 20:42:17]

Patchset #1 (id:40001) has been deleted

[Siggi Cherem (dart-lang), 2015-03-20 20:42:22]

Patchset #1 (id:60001) has been deleted

[Siggi Cherem (dart-lang), 2015-03-20 20:42:28]

Patchset #1 (id:80001) has been deleted

[Siggi Cherem (dart-lang), 2015-03-20 20:48:27]

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

[Siggi Cherem (dart-lang), 2015-03-20 20:48:28]

this uses transitive inference for consts and statics, not for instance fields,
however it propagates results from consts/statics before going to infer instance
fields (we just don't do transitive inference between instance fields).

I uploaded 2 patches so you can see in patchset #1 what happens when we modify
sunflower.dart. Without transitive inference the result gets worse, but if you
turn it on, you get back the sunflower.js we generate today.

https://codereview.chromium.org/1028793002/diff/40002/pubspec.yaml
File pubspec.yaml (right):

https://codereview.chromium.org/1028793002/diff/40002/pubspec.yaml#newcode29
pubspec.yaml:29: path: ../ddc_analyzer
will revert this before submitting (I needed a small change to make some members
public)

[vsm, 2015-03-21 15:52:19]

lgtm

https://codereview.chromium.org/1028793002/diff/40002/lib/src/checker/resolve...
File lib/src/checker/resolver.dart (right):

https://codereview.chromium.org/1028793002/diff/40002/lib/src/checker/resolve...
lib/src/checker/resolver.dart:173: var globals = <VariableDeclaration>[];
nit: maybe globals -> globalsAndStatics?

https://codereview.chromium.org/1028793002/diff/40002/lib/src/checker/resolve...
lib/src/checker/resolver.dart:274: void _reanalizeVar(Map<Source,
RestrictedResolverVisitor> visitors,
_reanalize -> _reanalyze

https://codereview.chromium.org/1028793002/diff/40002/lib/src/options.dart
File lib/src/options.dart (right):

https://codereview.chromium.org/1028793002/diff/40002/lib/src/options.dart#ne...
lib/src/options.dart:47: /// always.
Can you update the comment?  :-)

Have you seen any noticeable analysis perf impact from turning this on?

[Siggi Cherem (dart-lang), 2015-03-23 20:06:31]

Thanks Vijay,

https://codereview.chromium.org/1028793002/diff/40002/lib/src/checker/resolve...
File lib/src/checker/resolver.dart (right):

https://codereview.chromium.org/1028793002/diff/40002/lib/src/checker/resolve...
lib/src/checker/resolver.dart:173: var globals = <VariableDeclaration>[];
On 2015/03/21 15:52:19, vsm wrote:
> nit: maybe globals -> globalsAndStatics?

oh funny, when I was writing this I was thinking that, since statics are globals
too, I could just call them globals. But then I didn't rename the functions
below to just say _inferGlobals instead of _inferGlobalsAndStatics... maybe
because I felt it was more explicit :-)

Not sure if you have a preference either way. For now I added the `AndStatics`,
but I'm good without it too :-)

https://codereview.chromium.org/1028793002/diff/40002/lib/src/checker/resolve...
lib/src/checker/resolver.dart:274: void _reanalizeVar(Map<Source,
RestrictedResolverVisitor> visitors,
On 2015/03/21 15:52:19, vsm wrote:
> _reanalize -> _reanalyze

Done.

https://codereview.chromium.org/1028793002/diff/40002/lib/src/options.dart
File lib/src/options.dart (right):

https://codereview.chromium.org/1028793002/diff/40002/lib/src/options.dart#ne...
lib/src/options.dart:47: /// always.
On 2015/03/21 15:52:19, vsm wrote:
> Can you update the comment?  :-)
Good point. done.

> Have you seen any noticeable analysis perf impact from turning this on?

I haven't noticed much change, no. I think it is because we still have a pretty
tight bound on how many times we analyze/resolve a piece of code.

Before this change we were analyzing every initializer twice:
  - once before inference
  - once more after inference
the latter was to improve codegen of the initializer expression given the new
types and to resolve portions of initializers that were skipped over at first,
like function expressions.

With this change, now we likely still do 2 passes over an initializer, but at
most 3 times:
  - once before inference
  - once after inferring the type of the dependencies
  - possibly once more if the initializer contained a function expression with a
body.

So I don't think we are adding that much more to what we were doing already.

[Siggi Cherem (dart-lang), 2015-03-23 20:11:38]

Committed patchset #3 (id:130001) manually as
97984fa6bd10c8438b9da8af56dc5360a7fa80e6 (presubmit successful).