Avoid writing to the same parts of the element model in two tasks.

Avoid writing to the same parts of the element model in two tasks.

Previous to this change, ResolveUnitTypeNamesTask would set types to
`dynamic` if there was no explicit type in the source code, and
InferInstanceMembersInUnitTask would overwrite those types with
inferred types.  This caused a problem if the result of
ResolveUnitTypeNamesTask got flushed due to memory pressure, but the
result of InferInstanceMembersInUnitTask did not.  In that case, the
task model would come back at a later time and re-run
ResolveUnitTypeNamesTask, causing the inferred types to be set back to
`dynamic`.  But it would not re-run InferInstanceMembersInUnitTask.
As a result, the inferred type would be lost.  This manifested as a
sporadic failure to inferred types in analysis server.  Annoyingly,
edits to the file(s) involved would frequently cover up the problem
(temporarily) since they would invalidate the cache entries, forcing
the task model to re-run both tasks.

This CL fixes the problem by adding _declaredType and
_declaredReturnType fields to the element model;
ResolveUnitTypeNamesTask writes to those.
InferInstanceMembersInUnitTask continues to write to _type and
_returnType, as it always has.  When a type or return type is
requested from the element model, the getter consults _type (or
_returnType) first; only if it is null does it fall back to
_declaredType (or _declaredReturnType).

This gives us the behavior we need (inferred types override the
implied "dynamic" type stored by ResolveUnitTypeNamesTask), but since
the two tasks are now writing to different fields in the element
model, it is now safe to re-run ResolveUnitTypeNamesTask--it will no
longer overwrite the inferred types.

Fixes #27482.

R=scheglov@google.com

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

[Paul Berry, 2016-11-04 20:25:45]

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

[scheglov, 2016-11-04 20:33:41]

LGTM!

[Paul Berry, 2016-11-04 23:44:32]

Description was changed from

==========
Avoid writing to the same parts of the element model in two tasks.

Previous to this change, ResolveUnitTypeNamesTask would set types to
`dynamic` if there was no explicit type in the source code, and
InferInstanceMembersInUnitTask would overwrite those types with
inferred types.  This caused a problem if the result of
ResolveUnitTypeNamesTask got flushed due to memory pressure, but the
result of InferInstanceMembersInUnitTask did not.  In that case, the
task model would come back at a later time and re-run
ResolveUnitTypeNamesTask, causing the inferred types to be set back to
`dynamic`.  But it would not re-run InferInstanceMembersInUnitTask.
As a result, the inferred type would be lost.  This manifested as a
sporadic failure to inferred types in analysis server.  Annoyingly,
edits to the file(s) involved would frequently cover up the problem
(temporarily) since they would invalidate the cache entries, forcing
the task model to re-run both tasks.

This CL fixes the problem by adding _declaredType and
_declaredReturnType fields to the element model;
ResolveUnitTypeNamesTask writes to those.
InferInstanceMembersInUnitTask continues to write to _type and
_returnType, as it always has.  When a type or return type is
requested from the element model, the getter consults _type (or
_returnType) first; only if it is null does it fall back to
_declaredType (or _declaredReturnType).

This gives us the behavior we need (inferred types override the
implied "dynamic" type stored by ResolveUnitTypeNamesTask), but since
the two tasks are now writing to different fields in the element
model, it is now safe to re-run ResolveUnitTypeNamesTask--it will no
longer overwrite the inferred types.

Fixes #27482.
==========

to

==========
Avoid writing to the same parts of the element model in two tasks.

Previous to this change, ResolveUnitTypeNamesTask would set types to
`dynamic` if there was no explicit type in the source code, and
InferInstanceMembersInUnitTask would overwrite those types with
inferred types.  This caused a problem if the result of
ResolveUnitTypeNamesTask got flushed due to memory pressure, but the
result of InferInstanceMembersInUnitTask did not.  In that case, the
task model would come back at a later time and re-run
ResolveUnitTypeNamesTask, causing the inferred types to be set back to
`dynamic`.  But it would not re-run InferInstanceMembersInUnitTask.
As a result, the inferred type would be lost.  This manifested as a
sporadic failure to inferred types in analysis server.  Annoyingly,
edits to the file(s) involved would frequently cover up the problem
(temporarily) since they would invalidate the cache entries, forcing
the task model to re-run both tasks.

This CL fixes the problem by adding _declaredType and
_declaredReturnType fields to the element model;
ResolveUnitTypeNamesTask writes to those.
InferInstanceMembersInUnitTask continues to write to _type and
_returnType, as it always has.  When a type or return type is
requested from the element model, the getter consults _type (or
_returnType) first; only if it is null does it fall back to
_declaredType (or _declaredReturnType).

This gives us the behavior we need (inferred types override the
implied "dynamic" type stored by ResolveUnitTypeNamesTask), but since
the two tasks are now writing to different fields in the element
model, it is now safe to re-run ResolveUnitTypeNamesTask--it will no
longer overwrite the inferred types.

Fixes #27482.

R=scheglov@google.com

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

[Paul Berry, 2016-11-04 23:44:33]

Committed patchset #1 (id:1) manually as
f9c55af608019c5e69f4a19ffab6b6f54e4ac9c7 (presubmit successful).

[Brian Wilkerson, 2016-11-06 17:38:17]

lgtm