Functional JavaScript AST source position updates

Functional JavaScript AST source position updates

Functional update of source positions means that trees can be shared more freely.

R=floitsch@google.com

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

[sra1, 2014-04-26 17:14:19]

I have another CL that has no source position info on these nodes and has new
nodes to wrap Expressions and Statements with source positions.  It saves space
since source positions are somewhat sparse, but it ran into some ugly issues
with code that does is-checks on various nodes.  My thought is we revisit that
later.

[floitsch, 2014-04-28 12:28:11]

LGTM.

https://chromiumcodereview.appspot.com/251593006/diff/40001/sdk/lib/_internal...
File sdk/lib/_internal/compiler/implementation/js/nodes.dart (right):

https://chromiumcodereview.appspot.com/251593006/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/js/nodes.dart:181: Node _clone();  //
Single node clone
Comment.
Does it need to clone positions?...

https://chromiumcodereview.appspot.com/251593006/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/js/nodes.dart:183: Node
withPosition(var sourcePosition, var endSourcePosition) {
comments.

https://chromiumcodereview.appspot.com/251593006/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/js/nodes.dart:195: Node
withLocation(var sourcePosition) =>
comments.
In particular differentiating Position/Location.

https://chromiumcodereview.appspot.com/251593006/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/js/nodes.dart:769:
Prefix._internal(Expression target, List<xpression> arguments)
Expression.

https://chromiumcodereview.appspot.com/251593006/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/js/nodes.dart:786:
Postfix._internal(Expression target, List<xpression> arguments)
Expression

https://chromiumcodereview.appspot.com/251593006/diff/40001/sdk/lib/_internal...
File sdk/lib/_internal/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/251593006/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/ssa/codegen.dart:921: loop =
attachLocationRange(loop, info.sourcePosition, info.endSourcePosition);
long line.

[sra1, 2014-04-28 18:24:09]

https://chromiumcodereview.appspot.com/251593006/diff/40001/sdk/lib/_internal...
File sdk/lib/_internal/compiler/implementation/js/nodes.dart (right):

https://chromiumcodereview.appspot.com/251593006/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/js/nodes.dart:181: Node _clone();  //
Single node clone
On 2014/04/28 12:28:11, floitsch wrote:
> Comment.
> Does it need to clone positions?...

No, because it is used only by code that sets the position.
It is really just avoiding threading the positions through all the constructors.
 I want to eventually make the positions separate wrapping nodes.

https://chromiumcodereview.appspot.com/251593006/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/js/nodes.dart:183: Node
withPosition(var sourcePosition, var endSourcePosition) {
On 2014/04/28 12:28:11, floitsch wrote:
> comments.

Done.

https://chromiumcodereview.appspot.com/251593006/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/js/nodes.dart:195: Node
withLocation(var sourcePosition) =>
On 2014/04/28 12:28:11, floitsch wrote:
> comments.
> In particular differentiating Position/Location.

Done.

https://chromiumcodereview.appspot.com/251593006/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/js/nodes.dart:769:
Prefix._internal(Expression target, List<xpression> arguments)
On 2014/04/28 12:28:11, floitsch wrote:
> Expression.

Done.

https://chromiumcodereview.appspot.com/251593006/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/js/nodes.dart:786:
Postfix._internal(Expression target, List<xpression> arguments)
On 2014/04/28 12:28:11, floitsch wrote:
> Expression

Done.

https://chromiumcodereview.appspot.com/251593006/diff/40001/sdk/lib/_internal...
File sdk/lib/_internal/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/251593006/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/ssa/codegen.dart:921: loop =
attachLocationRange(loop, info.sourcePosition, info.endSourcePosition);
On 2014/04/28 12:28:11, floitsch wrote:
> long line.

Done.

[sra1, 2014-04-28 18:59:13]

Committed patchset #3 manually as r35491 (presubmit successful).

[floitsch, 2014-04-30 15:36:01]

https://chromiumcodereview.appspot.com/251593006/diff/40001/sdk/lib/_internal...
File sdk/lib/_internal/compiler/implementation/js/nodes.dart (right):

https://chromiumcodereview.appspot.com/251593006/diff/40001/sdk/lib/_internal...
sdk/lib/_internal/compiler/implementation/js/nodes.dart:181: Node _clone();  //
Single node clone
On 2014/04/28 18:24:09, sra1 wrote:
> On 2014/04/28 12:28:11, floitsch wrote:
> > Comment.
> > Does it need to clone positions?...
> 
> No, because it is used only by code that sets the position.
> It is really just avoiding threading the positions through all the
constructors.
>  I want to eventually make the positions separate wrapping nodes.

I got it (reading the source). Just wanted you to add it to the documentation.