Propagate variable type in TypeAnalyzer

Propagate variable type in TypeAnalyzer
Don't re-resolve in CompletionEngine to keep propagated types.


R=brianwilkerson@google.com,messick@google.com
BUG=
TEST=

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

[Brian Wilkerson, 2012-05-21 23:12:14]

LGTM

https://chromiumcodereview.appspot.com/10413041/diff/1/compiler/java/com/goog...
File compiler/java/com/google/dart/compiler/ast/DartNode.java (right):

https://chromiumcodereview.appspot.com/10413041/diff/1/compiler/java/com/goog...
compiler/java/com/google/dart/compiler/ast/DartNode.java:1: // Copyright (c)
2011, the Dart project authors.  Please see the AUTHORS file
nit: copyright year

https://chromiumcodereview.appspot.com/10413041/diff/1/compiler/java/com/goog...
File compiler/java/com/google/dart/compiler/resolver/Resolver.java (right):

https://chromiumcodereview.appspot.com/10413041/diff/1/compiler/java/com/goog...
compiler/java/com/google/dart/compiler/resolver/Resolver.java:721:
VariableElement variableElement = resolveVariable(variable,
node.getModifiers());
nit: I don't understand the purpose for this change. My understanding of the
culture (which could be wrong) is "don't change existing code unless there's a
reason to".

[messick, 2012-05-21 23:20:50]

LGTM

I like this change because it work globally instead of the purely local analysis
done by TypeRefiner. Unfortunately, this does not replace TypeRefiner. Is there
any way this analysis can be extended to cover type tests? TypeRefiner gives x a
precise type in this code:

foo(var x) {
  if (x is List) {
    // type of x is List in this branch, unless otherwise assigned
  }
}

Also, can some of the code completion tests that are dependent on the
experimental option be permanently enabled now?

https://chromiumcodereview.appspot.com/10413041/diff/1/compiler/java/com/goog...
File compiler/java/com/google/dart/compiler/resolver/Resolver.java (right):

https://chromiumcodereview.appspot.com/10413041/diff/1/compiler/java/com/goog...
compiler/java/com/google/dart/compiler/resolver/Resolver.java:721:
VariableElement variableElement = resolveVariable(variable,
node.getModifiers());
On 2012/05/21 23:12:14, Brian Wilkerson wrote:
> nit: I don't understand the purpose for this change. My understanding of the
> culture (which could be wrong) is "don't change existing code unless there's a
> reason to".

I found this puzzling, too, but assumed it made debugging easier.

[Brian Wilkerson, 2012-05-21 23:23:48]

https://chromiumcodereview.appspot.com/10413041/diff/1/compiler/java/com/goog...
File compiler/java/com/google/dart/compiler/resolver/Resolver.java (right):

https://chromiumcodereview.appspot.com/10413041/diff/1/compiler/java/com/goog...
compiler/java/com/google/dart/compiler/resolver/Resolver.java:721:
VariableElement variableElement = resolveVariable(variable,
node.getModifiers());
> I found this puzzling, too, but assumed it made debugging easier.

I assumed the same thing, but I try to always back out debugging only changes
unless I know that the need for easier debugging will be a long term issue.

[scheglov, 2012-05-22 00:59:10]

> I like this change because it work globally instead of the purely local
analysis
> done by TypeRefiner. Unfortunately, this does not replace TypeRefiner. Is
there
> any way this analysis can be extended to cover type tests? TypeRefiner gives x
a
> precise type in this code:
> 
> foo(var x) {
>   if (x is List) {
>     // type of x is List in this branch, unless otherwise assigned
>   }
> }

  Yes, I think we can add support such case in the next CL.


> Also, can some of the code completion tests that are dependent on the
> experimental option be permanently enabled now?

  Experimental option - DartCoreDebug.ENABLE_TYPE_REFINEMENT, right?

  What tests do you mean?
  Class name would be helpful.
  Are these tests the same which cover TypeRefiner?
  I've looked into CompletionEngineTest, but can not identify which tests
require type refinement.

[scheglov, 2012-05-22 00:59:32]

https://chromiumcodereview.appspot.com/10413041/diff/1/compiler/java/com/goog...
File compiler/java/com/google/dart/compiler/ast/DartNode.java (right):

https://chromiumcodereview.appspot.com/10413041/diff/1/compiler/java/com/goog...
compiler/java/com/google/dart/compiler/ast/DartNode.java:1: // Copyright (c)
2011, the Dart project authors.  Please see the AUTHORS file
On 2012/05/21 23:12:14, Brian Wilkerson wrote:
> nit: copyright year

Done.

https://chromiumcodereview.appspot.com/10413041/diff/1/compiler/java/com/goog...
File compiler/java/com/google/dart/compiler/resolver/Resolver.java (right):

https://chromiumcodereview.appspot.com/10413041/diff/1/compiler/java/com/goog...
compiler/java/com/google/dart/compiler/resolver/Resolver.java:721:
VariableElement variableElement = resolveVariable(variable,
node.getModifiers());
Yes, this is was part of initial hackish implementation, which I again forgot to
rollback. Fixed now.