Don't serialize local variables.

Don't serialize and resynthesize local variables.

It does not affect completion, because we create local variable
elements in a unit during resolution of this unit.

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

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

[Paul Berry, 2017-06-29 21:34:34]

lgtm assuming my comment in pkg/analyzer/test/src/dart/analysis/driver_test.dart
is addressed.

Your call as far as what to do about my other comment.

https://codereview.chromium.org/2963183002/diff/1/pkg/analyzer/lib/src/summar...
File pkg/analyzer/lib/src/summary/summarize_ast.dart (right):

https://codereview.chromium.org/2963183002/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/summarize_ast.dart:1394: //   
serializeVariables(node.variables, false, null, null, false);
Should we just remove this override entirely?

https://codereview.chromium.org/2963183002/diff/1/pkg/analyzer/test/src/dart/...
File pkg/analyzer/test/src/dart/analysis/driver_test.dart (right):

https://codereview.chromium.org/2963183002/diff/1/pkg/analyzer/test/src/dart/...
pkg/analyzer/test/src/dart/analysis/driver_test.dart:1338: // 
solo_test_getResult_XXX() async {
Remove this

[scheglov, 2017-06-29 21:39:54]

https://codereview.chromium.org/2963183002/diff/1/pkg/analyzer/lib/src/summar...
File pkg/analyzer/lib/src/summary/summarize_ast.dart (right):

https://codereview.chromium.org/2963183002/diff/1/pkg/analyzer/lib/src/summar...
pkg/analyzer/lib/src/summary/summarize_ast.dart:1394: //   
serializeVariables(node.variables, false, null, null, false);
On 2017/06/29 21:34:33, Paul Berry wrote:
> Should we just remove this override entirely?

We cannot do this.
If there are function expressions in initializers, we will serialize them.
I will add TODO to return to this later.

https://codereview.chromium.org/2963183002/diff/1/pkg/analyzer/test/src/dart/...
File pkg/analyzer/test/src/dart/analysis/driver_test.dart (right):

https://codereview.chromium.org/2963183002/diff/1/pkg/analyzer/test/src/dart/...
pkg/analyzer/test/src/dart/analysis/driver_test.dart:1338: // 
solo_test_getResult_XXX() async {
On 2017/06/29 21:34:34, Paul Berry wrote:
> Remove this

Done.

[Brian Wilkerson, 2017-06-29 21:51:42]

https://codereview.chromium.org/2963183002/diff/20001/pkg/analyzer/lib/src/da...
File pkg/analyzer/lib/src/dart/element/element.dart (right):

https://codereview.chromium.org/2963183002/diff/20001/pkg/analyzer/lib/src/da...
pkg/analyzer/lib/src/dart/element/element.dart:3794: List<LocalVariableElement>
get localVariables {
If this method is always going to return an empty list, then can we remove
`_localVariables`?

More generally, though, I don't like the idea that one representation never has
local variable and others do. If we're removing them from the API, then it seems
like we should mark the getter as being deprecated and return an empty list from
all of the implementations. Am I missing something?

Should we remove other local elements as well (or at least mark the getters /
methods as deprecated)?

[scheglov, 2017-06-29 22:09:12]

https://codereview.chromium.org/2963183002/diff/20001/pkg/analyzer/lib/src/da...
File pkg/analyzer/lib/src/dart/element/element.dart (right):

https://codereview.chromium.org/2963183002/diff/20001/pkg/analyzer/lib/src/da...
pkg/analyzer/lib/src/dart/element/element.dart:3794: List<LocalVariableElement>
get localVariables {
On 2017/06/29 21:51:41, Brian Wilkerson wrote:
> If this method is always going to return an empty list, then can we remove
> `_localVariables`?
> 
> More generally, though, I don't like the idea that one representation never
has
> local variable and others do. If we're removing them from the API, then it
seems
> like we should mark the getter as being deprecated and return an empty list
from
> all of the implementations. Am I missing something?
> 
> Should we remove other local elements as well (or at least mark the getters /
> methods as deprecated)?

This is an interesting question.
Now as I search to "localVariable" usages, I see that it is used only in a
couple of places.
We mostly SET it, but very rarely GET.

Tests use it pretty often, mostly to verify type inference; plus tests of
elements builders.

OK, I will try to clean up these clients first, and then we will get a change
that removes this API.

[scheglov, 2017-07-06 00:08:53]

Description was changed from

==========
Don't serialize and resynthesize local variables.

It does not affect completion, because we create local variable
elements in a unit during resolution of this unit.

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

to

==========
Don't serialize and resynthesize local variables.

It does not affect completion, because we create local variable
elements in a unit during resolution of this unit.

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

[scheglov, 2017-07-06 00:10:15]

PTAL

We don't resynthesize local variables anymore, and
ExecutableElement.localElement was removed.

So, this CL removes serializing local variables.

[Brian Wilkerson, 2017-07-06 00:42:13]

lgtm

[scheglov, 2017-07-06 00:57:31]

Description was changed from

==========
Don't serialize and resynthesize local variables.

It does not affect completion, because we create local variable
elements in a unit during resolution of this unit.

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

to

==========
Don't serialize and resynthesize local variables.

It does not affect completion, because we create local variable
elements in a unit during resolution of this unit.

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

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

[scheglov, 2017-07-06 00:57:32]

Committed patchset #3 (id:40001) manually as
2bfde5a34eea096cc7bdbc80852da75c0cfa2edd (presubmit successful).