Adds a class for computing the dependencies that a given JS AstNode has.

Adds a class for computing the dependencies that a given JS AstNode has.

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

[fabiomfv, 2011-11-18 19:15:45]

LGTM. few comments.

http://codereview.chromium.org/8561015/diff/1/compiler/java/com/google/dart/c...
File
compiler/java/com/google/dart/compiler/backend/js/analysis/DependencyComputer.java
(right):

http://codereview.chromium.org/8561015/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/analysis/DependencyComputer.java:98:
if (scope.getAstRoot() != scope) {
I am curious why we need to do this. just walking the parent scope chain isnt
enough? isnt the topmost scope the same as scope.getAstRoot() ?

http://codereview.chromium.org/8561015/diff/1/compiler/javatests/com/google/d...
File
compiler/javatests/com/google/dart/compiler/backend/js/analysis/DependencyComputerTest.java
(right):

http://codereview.chromium.org/8561015/diff/1/compiler/javatests/com/google/d...
compiler/javatests/com/google/dart/compiler/backend/js/analysis/DependencyComputerTest.java:157:
}
could you add a test variation for:
B.foo = function() { }
B.prototype.foo = function() { }

'static' B.foo and protoype with both instantiation and non-instantiated cases
(B)?

[zundel, 2011-11-18 20:36:37]

LGTM

http://codereview.chromium.org/8561015/diff/1/compiler/java/com/google/dart/c...
File
compiler/java/com/google/dart/compiler/backend/js/analysis/DependencyComputer.java
(right):

http://codereview.chromium.org/8561015/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/analysis/DependencyComputer.java:69:
while (instantiatedMember != null) {
Can you move this loop before the instantiatedMember.setInstantiated(), then
short-circuit  the for loop if the member already has the instantiated bit set? 
Currently this is order n^2 over the number of members in the program.

http://codereview.chromium.org/8561015/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/analysis/DependencyComputer.java:156:
* Return a static name if the {@link PropertyGet} matches the patter x.y or
patter --> pattern

http://codereview.chromium.org/8561015/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/analysis/DependencyComputer.java:216:
addVirtualDependency(identifier);
In the general case, this could be in instantiation, I think.

E.g. using the constructor() member function to create an object.

[mmendez, 2011-11-18 21:59:48]

Thanks for the reviews guys.

http://codereview.chromium.org/8561015/diff/1/compiler/java/com/google/dart/c...
File
compiler/java/com/google/dart/compiler/backend/js/analysis/DependencyComputer.java
(right):

http://codereview.chromium.org/8561015/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/analysis/DependencyComputer.java:69:
while (instantiatedMember != null) {
No, unfortunately I cannot.  What happens is that when we see virtual names, or
names that appear to be used virtually ,we record them and process any method on
any class that has been instantiated by that point in the program.

The issue is that later on a new class can be instantiated and it may have a
member which matches a previously seen virtual name.  So everytime that we
encounter a new we check all members up the hierarchy whose name matches.  I
could be less conservative and restrict it to virtual members though.

I also renamed the local variables to make it clearer as to what is a class vs a
class member.
On 2011/11/18 20:36:37, zundel wrote:
> Can you move this loop before the instantiatedMember.setInstantiated(), then
> short-circuit  the for loop if the member already has the instantiated bit
set? 
> Currently this is order n^2 over the number of members in the program.

http://codereview.chromium.org/8561015/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/analysis/DependencyComputer.java:98:
if (scope.getAstRoot() != scope) {
Unfortunately not.  The parent scope doesn't link to the AstRoot.

On 2011/11/18 19:15:45, fabiomfv wrote:
> I am curious why we need to do this. just walking the parent scope chain isnt
> enough? isnt the topmost scope the same as scope.getAstRoot() ?

http://codereview.chromium.org/8561015/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/analysis/DependencyComputer.java:156:
* Return a static name if the {@link PropertyGet} matches the patter x.y or
On 2011/11/18 20:36:37, zundel wrote:
> patter --> pattern

Done.

http://codereview.chromium.org/8561015/diff/1/compiler/java/com/google/dart/c...
compiler/java/com/google/dart/compiler/backend/js/analysis/DependencyComputer.java:216:
addVirtualDependency(identifier);
There are native JS objects that can be created via new func() of simply func. 
In the code generated by dartc we do not use that pattern though and neither do
our libraries.

On 2011/11/18 20:36:37, zundel wrote:
> In the general case, this could be in instantiation, I think.
> 
> E.g. using the constructor() member function to create an object.

http://codereview.chromium.org/8561015/diff/1/compiler/javatests/com/google/d...
File
compiler/javatests/com/google/dart/compiler/backend/js/analysis/DependencyComputerTest.java
(right):

http://codereview.chromium.org/8561015/diff/1/compiler/javatests/com/google/d...
compiler/javatests/com/google/dart/compiler/backend/js/analysis/DependencyComputerTest.java:157:
}
I could, but that would not be legal dart code as the static and instance foo's
would result in a name clash so I'm not sure that it is worth it.

On 2011/11/18 19:15:45, fabiomfv wrote:
> could you add a test variation for:
> B.foo = function() { }
> B.prototype.foo = function() { }
> 
> 'static' B.foo and protoype with both instantiation and non-instantiated cases
> (B)?