Serialize local functions and variables.

Serialize local functions and variables.

R=paulberry@google.com
BUG=

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

[Paul Berry, 2016-02-11 15:29:19]

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/lib/src/su...
File pkg/analyzer/lib/src/summary/idl.dart (right):

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/idl.dart:996: List<UnlinkedExecutable> get
executables;
I'd prefer if we called this "localFunctions" for clarity.

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/idl.dart:1096: * The list of local variable.
s/variable/variables/

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/idl.dart:1098: List<UnlinkedVariable> get
variables;
Similarly, this should be called "localVariables".

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/idl.dart:1101: * If a local function, the length of
the visible range; absent otherwise.
s/absent/zero/ here and in 3 other places below.

(The fact that ints with a value of zero don't take up any space in the
serialized output is an implementation detail of Flatbuffers which shouldn't be
leaked to this level of abstraction)

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/lib/src/su...
File pkg/analyzer/lib/src/summary/summarize_ast.dart (right):

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/summarize_ast.dart:547:
List<UnlinkedExecutableBuilder> oldExecutable = executables;
s/oldExecutable/oldExecutables/

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/summarize_ast.dart:845: serializeFunctionBody(b,
node.body);
Don't we need this in other places?  (visitFunctionDeclaration and
visitMethodDeclaration)?

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/test/src/s...
File pkg/analyzer/test/src/summary/summary_common.dart (right):

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/test/src/s...
pkg/analyzer/test/src/summary/summary_common.dart:3579:
test_executable_localFunctions() {
Also test local variables and local functions inside of a method (since methods
use a different AST structure) and inside of a property accessor (since property
accessors use a different kind of element).

[scheglov, 2016-02-11 15:50:08]

PTAL

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/lib/src/su...
File pkg/analyzer/lib/src/summary/idl.dart (right):

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/idl.dart:996: List<UnlinkedExecutable> get
executables;
On 2016/02/11 15:29:19, Paul Berry wrote:
> I'd prefer if we called this "localFunctions" for clarity.

Done!
Actually I named them as localFunctions and localVariables initially, but then
thought that maybe they should be named as other similar fields :-)

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/idl.dart:1096: * The list of local variable.
On 2016/02/11 15:29:19, Paul Berry wrote:
> s/variable/variables/

Done.

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/idl.dart:1098: List<UnlinkedVariable> get
variables;
On 2016/02/11 15:29:19, Paul Berry wrote:
> Similarly, this should be called "localVariables".

Done.

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/idl.dart:1101: * If a local function, the length of
the visible range; absent otherwise.
On 2016/02/11 15:29:19, Paul Berry wrote:
> s/absent/zero/ here and in 3 other places below.
> 
> (The fact that ints with a value of zero don't take up any space in the
> serialized output is an implementation detail of Flatbuffers which shouldn't
be
> leaked to this level of abstraction)

Done.

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/lib/src/su...
File pkg/analyzer/lib/src/summary/summarize_ast.dart (right):

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/summarize_ast.dart:547:
List<UnlinkedExecutableBuilder> oldExecutable = executables;
On 2016/02/11 15:29:19, Paul Berry wrote:
> s/oldExecutable/oldExecutables/

Done.

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/summarize_ast.dart:845: serializeFunctionBody(b,
node.body);
On 2016/02/11 15:29:19, Paul Berry wrote:
> Don't we need this in other places?  (visitFunctionDeclaration and
> visitMethodDeclaration)?

Yes, we call it also in serializeExecutable(), which is used in
visitFunctionDeclaration and visitMethodDeclaration.

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/test/src/s...
File pkg/analyzer/test/src/summary/summary_common.dart (right):

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/test/src/s...
pkg/analyzer/test/src/summary/summary_common.dart:3579:
test_executable_localFunctions() {
On 2016/02/11 15:29:19, Paul Berry wrote:
> Also test local variables and local functions inside of a method (since
methods
> use a different AST structure) and inside of a property accessor (since
property
> accessors use a different kind of element).

Done.

[Paul Berry, 2016-02-11 18:03:52]

lgtm

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/lib/src/su...
File pkg/analyzer/lib/src/summary/summarize_ast.dart (right):

https://codereview.chromium.org/1691693002/diff/20001/pkg/analyzer/lib/src/su...
pkg/analyzer/lib/src/summary/summarize_ast.dart:845: serializeFunctionBody(b,
node.body);
On 2016/02/11 15:50:08, scheglov wrote:
> On 2016/02/11 15:29:19, Paul Berry wrote:
> > Don't we need this in other places?  (visitFunctionDeclaration and
> > visitMethodDeclaration)?
> 
> Yes, we call it also in serializeExecutable(), which is used in
> visitFunctionDeclaration and visitMethodDeclaration.

Whoops, not sure how I missed that.  Thanks!

[scheglov, 2016-02-11 19:20:38]

Description was changed from

==========
Serialize local functions and variables.

R=paulberry@google.com
BUG=
==========

to

==========
Serialize local functions and variables.

R=paulberry@google.com
BUG=

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

[scheglov, 2016-02-11 19:20:40]

Committed patchset #3 (id:40001) manually as
201a8d55705d8b4a151aefc5821a3247ee515d2a (presubmit successful).