Don't perform final fields and top-level variables type propagation.

Don't perform final fields and top-level variables type propagation.

A similar feature is now implemented in strong mode, and users who
need it, should switch to strong mode.

When we are ready to drop propagated types altogethoer, we could
deprecate or remove UnlinkedVariable.propagatedTypeSlot in IDL.

R=brianwilkerson@google.com, paulberry@google.com
BUG= https://github.com/dart-lang/sdk/issues/26794

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

[scheglov, 2016-08-29 20:39:35]

Description was changed from

==========
Don't perform final fields and top-level variables type propagation.

A similar feature is now implemented in strong mode, and users who
need it, should switch to strong mode.

When we are ready to drop propagated types altogethoer, we could
deprecate or remove UnlinkedVariable.propagatedTypeSlot in IDL.

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

to

==========
Don't perform final fields and top-level variables type propagation.

A similar feature is now implemented in strong mode, and users who
need it, should switch to strong mode.

When we are ready to drop propagated types altogethoer, we could
deprecate or remove UnlinkedVariable.propagatedTypeSlot in IDL.

R=brianwilkerson@google.com, paulberry@google.com
BUG= https://github.com/dart-lang/sdk/issues/26794
==========

[Brian Wilkerson, 2016-08-29 20:49:04]

lgtm

https://codereview.chromium.org/2288223003/diff/1/pkg/analysis_server/test/co...
File pkg/analysis_server/test/completion_test.dart (right):

https://codereview.chromium.org/2288223003/diff/1/pkg/analysis_server/test/co...
pkg/analysis_server/test/completion_test.dart:2853: <String>["1+length"],
failingTests: '1');
It seems odd to have a single test that is expected to fail. It seems better to
just remove the test.

[Paul Berry, 2016-08-29 20:52:17]

https://codereview.chromium.org/2288223003/diff/1/pkg/analyzer/lib/src/task/d...
File pkg/analyzer/lib/src/task/dart.dart (right):

https://codereview.chromium.org/2288223003/diff/1/pkg/analyzer/lib/src/task/d...
pkg/analyzer/lib/src/task/dart.dart:857: final ResultDescriptor<CompilationUnit>
RESOLVED_UNIT8 =
To reduce the risk of subtle breakages to client code that may be reaching into
this file and referring to these result descriptors, I would rather that we
remove RESOLVED_UNIT8, but keep the numbering of the remaining RESOLVED_UNITs. 
That way clients that referred to RESOLVED_UNIT8 will be broken in an obvious
way (rather than a subtle one) and clients that referred to RESOLVED_UNIT9+
won't be broken at all.

It will also make it easier to be assured that this CL is correct, because we
wouldn't have to worry that we missed renumbering some existing references to
RESOLVED_UNITs.

(I realize that in an ideal world clients wouldn't be referring to these private
RESOLVED_UNIT* result descriptors, but I believe some clients do so).

https://codereview.chromium.org/2288223003/diff/1/pkg/analyzer/test/src/summa...
File pkg/analyzer/test/src/summary/summary_common.dart (left):

https://codereview.chromium.org/2288223003/diff/1/pkg/analyzer/test/src/summa...
pkg/analyzer/test/src/summary/summary_common.dart:7790:
test_fully_linked_references_follow_other_references() {
Rather than delete this test we should rewrite it in a way that doesn't make use
of propagated types.

Let me know if you'd rather I do this in a follow up CL.

https://codereview.chromium.org/2288223003/diff/1/pkg/analyzer/test/src/summa...
pkg/analyzer/test/src/summary/summary_common.dart:8714:
test_linked_reference_reuse() {
Ditto.

https://codereview.chromium.org/2288223003/diff/1/pkg/analyzer/test/src/summa...
pkg/analyzer/test/src/summary/summary_common.dart:8729:
test_linked_type_dependency_reuse() {
Ditto.

[Paul Berry, 2016-08-29 20:53:35]

https://codereview.chromium.org/2288223003/diff/1/pkg/analysis_server/test/co...
File pkg/analysis_server/test/completion_test.dart (right):

https://codereview.chromium.org/2288223003/diff/1/pkg/analysis_server/test/co...
pkg/analysis_server/test/completion_test.dart:2853: <String>["1+length"],
failingTests: '1');
On 2016/08/29 at 20:49:03, Brian Wilkerson wrote:
> It seems odd to have a single test that is expected to fail. It seems better
to just remove the test.

As a counterpoint to that, this is a test that we ought to be able to get to
pass once strong mode is the default.  Perhaps we should keep it until that
happens?

[scheglov, 2016-08-29 20:53:56]

https://codereview.chromium.org/2288223003/diff/1/pkg/analysis_server/test/co...
File pkg/analysis_server/test/completion_test.dart (right):

https://codereview.chromium.org/2288223003/diff/1/pkg/analysis_server/test/co...
pkg/analysis_server/test/completion_test.dart:2853: <String>["1+length"],
failingTests: '1');
On 2016/08/29 20:49:03, Brian Wilkerson wrote:
> It seems odd to have a single test that is expected to fail. It seems better
to
> just remove the test.

I thought we might want to keep it to re-enable when we switch to strong mode by
default / as the only mode.

[Brian Wilkerson, 2016-08-29 21:13:24]

https://codereview.chromium.org/2288223003/diff/1/pkg/analysis_server/test/co...
File pkg/analysis_server/test/completion_test.dart (right):

https://codereview.chromium.org/2288223003/diff/1/pkg/analysis_server/test/co...
pkg/analysis_server/test/completion_test.dart:2853: <String>["1+length"],
failingTests: '1');
Why not just set strong mode for this one test? (I hope we have some way of
testing completion under strong mode!)

https://codereview.chromium.org/2288223003/diff/1/pkg/analyzer/lib/src/task/d...
File pkg/analyzer/lib/src/task/dart.dart (right):

https://codereview.chromium.org/2288223003/diff/1/pkg/analyzer/lib/src/task/d...
pkg/analyzer/lib/src/task/dart.dart:857: final ResultDescriptor<CompilationUnit>
RESOLVED_UNIT8 =
Do we actually know of a problem? We've changed these names multiple times in
the past without causing problems. I'd rather keep them sequentially numbered if
there isn't an actual problem.

[Paul Berry, 2016-08-29 21:24:32]

lgtm (with concerns noted)

https://codereview.chromium.org/2288223003/diff/1/pkg/analyzer/lib/src/task/d...
File pkg/analyzer/lib/src/task/dart.dart (right):

https://codereview.chromium.org/2288223003/diff/1/pkg/analyzer/lib/src/task/d...
pkg/analyzer/lib/src/task/dart.dart:857: final ResultDescriptor<CompilationUnit>
RESOLVED_UNIT8 =
On 2016/08/29 at 21:13:24, Brian Wilkerson wrote:
> Do we actually know of a problem? We've changed these names multiple times in
the past without causing problems. I'd rather keep them sequentially numbered if
there isn't an actual problem.

No, I'm not aware of a specific problem, though my concerns remain.

[scheglov, 2016-08-29 22:01:47]

Paul, PTAL if the changes to summary tests are OK.

https://codereview.chromium.org/2288223003/diff/1/pkg/analyzer/lib/src/task/d...
File pkg/analyzer/lib/src/task/dart.dart (right):

https://codereview.chromium.org/2288223003/diff/1/pkg/analyzer/lib/src/task/d...
pkg/analyzer/lib/src/task/dart.dart:857: final ResultDescriptor<CompilationUnit>
RESOLVED_UNIT8 =
On 2016/08/29 21:24:32, Paul Berry wrote:
> On 2016/08/29 at 21:13:24, Brian Wilkerson wrote:
> > Do we actually know of a problem? We've changed these names multiple times
in
> the past without causing problems. I'd rather keep them sequentially numbered
if
> there isn't an actual problem.
> 
> No, I'm not aware of a specific problem, though my concerns remain.

I'm inclined to do what we always do, i.e. rename.

https://codereview.chromium.org/2288223003/diff/1/pkg/analyzer/test/src/summa...
File pkg/analyzer/test/src/summary/summary_common.dart (left):

https://codereview.chromium.org/2288223003/diff/1/pkg/analyzer/test/src/summa...
pkg/analyzer/test/src/summary/summary_common.dart:7790:
test_fully_linked_references_follow_other_references() {
On 2016/08/29 20:52:16, Paul Berry wrote:
> Rather than delete this test we should rewrite it in a way that doesn't make
use
> of propagated types.
> 
> Let me know if you'd rather I do this in a follow up CL.

I restored these 3 tests and changed to use 'inferredTypeSlot' instead.

[Paul Berry, 2016-08-29 22:07:47]

Test changes lgtm

[scheglov, 2016-08-29 22:18:17]

Description was changed from

==========
Don't perform final fields and top-level variables type propagation.

A similar feature is now implemented in strong mode, and users who
need it, should switch to strong mode.

When we are ready to drop propagated types altogethoer, we could
deprecate or remove UnlinkedVariable.propagatedTypeSlot in IDL.

R=brianwilkerson@google.com, paulberry@google.com
BUG= https://github.com/dart-lang/sdk/issues/26794
==========

to

==========
Don't perform final fields and top-level variables type propagation.

A similar feature is now implemented in strong mode, and users who
need it, should switch to strong mode.

When we are ready to drop propagated types altogethoer, we could
deprecate or remove UnlinkedVariable.propagatedTypeSlot in IDL.

R=brianwilkerson@google.com, paulberry@google.com
BUG= https://github.com/dart-lang/sdk/issues/26794

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

[scheglov, 2016-08-29 22:18:19]

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