Add data structures to AST to indicate potentially mutated variables.

Add data structures to AST to indicate potentially mutated variables.

These data structures provide the same functionality as
VariableElement.isPotentiallyMutatedInClosure and
VariableElement.isPotentiallyMutatedInScope, but are only accessible
from a resolved AST.  In a later CL,
VariableElement.isPotentiallyMutatedInClosure and
VariableElement.isPotentiallyMutatedInScope will be deprecated.

R=brianwilkerson@google.com, jmesserly@google.com

Committed: https://github.com/dart-lang/sdk/commit/c3d98acbeb36621831dad7a19f9a8a045c1da584

[Paul Berry, 2016-02-16 19:34:36]

paulberry@google.com changed reviewers:
+ brianwilkerson@google.com, jmesserly@google.com

[Paul Berry, 2016-02-16 19:34:37]

John, can you confirm whether this new API would be sufficient for DDC's needs?

[Jennifer Messerly, 2016-02-16 20:12:02]

LGTM

[Brian Wilkerson, 2016-02-16 21:13:39]

https://codereview.chromium.org/1700263002/diff/1/pkg/analyzer/lib/dart/ast/a...
File pkg/analyzer/lib/dart/ast/ast.dart (right):

https://codereview.chromium.org/1700263002/diff/1/pkg/analyzer/lib/dart/ast/a...
pkg/analyzer/lib/dart/ast/ast.dart:3783: LocalVariableInfo get
localVariableInfo;
I'm wondering whether it might not be better to avoid adding another class to
the API and to simply have a pair of methods added to FunctionBody:

bool isPotentiallyMutatedInClosure(VariableElement element)
bool isPotentiallyMutatedInScope(VariableElement element)

That would leave open the possibility of not pre-computing the whole list of
mutated elements.

https://codereview.chromium.org/1700263002/diff/1/pkg/analyzer/lib/src/genera...
File pkg/analyzer/lib/src/generated/resolver.dart (right):

https://codereview.chromium.org/1700263002/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/resolver.dart:12486: _localVariableInfo ??= new
LocalVariableInfoImpl();
Would it make sense to move the creation of the VariableInfo to the methods
visitBlockFunctionBody and visitExpressionFunctionBody? Having it in fewer
places seems like a good thing.

[Paul Berry, 2016-02-16 21:49:47]

https://codereview.chromium.org/1700263002/diff/1/pkg/analyzer/lib/dart/ast/a...
File pkg/analyzer/lib/dart/ast/ast.dart (right):

https://codereview.chromium.org/1700263002/diff/1/pkg/analyzer/lib/dart/ast/a...
pkg/analyzer/lib/dart/ast/ast.dart:3783: LocalVariableInfo get
localVariableInfo;
On 2016/02/16 21:13:39, Brian Wilkerson wrote:
> I'm wondering whether it might not be better to avoid adding another class to
> the API and to simply have a pair of methods added to FunctionBody:
> 
> bool isPotentiallyMutatedInClosure(VariableElement element)
> bool isPotentiallyMutatedInScope(VariableElement element)
> 
> That would leave open the possibility of not pre-computing the whole list of
> mutated elements.

Good idea.  Not only does this keep the API smaller, it also prevents clients
from accidentally modifying data structures they shouldn't touch.  PTAL.

https://codereview.chromium.org/1700263002/diff/1/pkg/analyzer/lib/src/genera...
File pkg/analyzer/lib/src/generated/resolver.dart (right):

https://codereview.chromium.org/1700263002/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/resolver.dart:12486: _localVariableInfo ??= new
LocalVariableInfoImpl();
On 2016/02/16 21:13:39, Brian Wilkerson wrote:
> Would it make sense to move the creation of the VariableInfo to the methods
> visitBlockFunctionBody and visitExpressionFunctionBody? Having it in fewer
> places seems like a good thing.

(Capturing the result of an in-person discussion about this)

We decided to leave this as is, on the grounds that following your suggestion
would make it harder to understand why the code was correct, because a reader
would worry about function parameters.  However, in a follow up CL I will add
assertions to make sure that whenever the visitor reaches a function body,
body.localVariableInfo has been set correctly.

[Brian Wilkerson, 2016-02-16 21:59:51]

LGTM

[Paul Berry, 2016-02-16 22:04:29]

Description was changed from

==========
Add data structures to AST to indicate potentially mutated variables.

These data structures provide the same functionality as
VariableElement.isPotentiallyMutatedInClosure and
VariableElement.isPotentiallyMutatedInScope, but are only accessible
from a resolved AST.  In a later CL,
VariableElement.isPotentiallyMutatedInClosure and
VariableElement.isPotentiallyMutatedInScope will be deprecated.
==========

to

==========
Add data structures to AST to indicate potentially mutated variables.

These data structures provide the same functionality as
VariableElement.isPotentiallyMutatedInClosure and
VariableElement.isPotentiallyMutatedInScope, but are only accessible
from a resolved AST.  In a later CL,
VariableElement.isPotentiallyMutatedInClosure and
VariableElement.isPotentiallyMutatedInScope will be deprecated.

R=brianwilkerson@google.com, jmesserly@google.com

Committed:
https://github.com/dart-lang/sdk/commit/c3d98acbeb36621831dad7a19f9a8a045c1da584
==========

[Paul Berry, 2016-02-16 22:04:31]

Committed patchset #2 (id:20001) manually as
c3d98acbeb36621831dad7a19f9a8a045c1da584 (presubmit successful).