Create a task in the new task model to compute constant dependencies.

Create a task in the new task model to compute constant dependencies.

This is the first of several planned tasks for constant evaluation.

R=brianwilkerson@google.com, scheglov@google.com

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

[Paul Berry, 2015-05-07 22:26:10]

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

[Brian Wilkerson, 2015-05-07 22:46:59]

LGTM

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

https://codereview.chromium.org/1131953004/diff/1/pkg/analyzer/lib/src/task/d...
pkg/analyzer/lib/src/task/dart.dart:1650: static const String UNIT_INPUT =
'UNIT_INPUT';
Perhaps "RESOLVED_UNIT_INPUT" to be consistent?

https://codereview.chromium.org/1131953004/diff/1/pkg/analyzer/lib/src/task/d...
pkg/analyzer/lib/src/task/dart.dart:1676: // to ensure that resolution has
occurred before we attempto to determine
"attempto" --> "attempt to"

https://codereview.chromium.org/1131953004/diff/1/pkg/analyzer/lib/src/task/d...
pkg/analyzer/lib/src/task/dart.dart:1681: TypeProvider typeProvider =
context.typeProvider;
I believe that you want to have a TYPE_PROVIDER input rather than getting the
type provider directly from the context.

https://codereview.chromium.org/1131953004/diff/1/pkg/analyzer/test/src/task/...
File pkg/analyzer/test/src/task/dart_test.dart (right):

https://codereview.chromium.org/1131953004/diff/1/pkg/analyzer/test/src/task/...
pkg/analyzer/test/src/task/dart_test.dart:1200: // Find the elements for the
constant x and y.
"constant" --> "constants"

[scheglov, 2015-05-07 22:52:09]

LGTM

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

https://codereview.chromium.org/1131953004/diff/1/pkg/analyzer/lib/src/task/d...
pkg/analyzer/lib/src/task/dart.dart:1676: // to ensure that resolution has
occurred before we attempto to determine
attempto -> attempt

[Paul Berry, 2015-05-08 00:24:35]

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

https://codereview.chromium.org/1131953004/diff/1/pkg/analyzer/lib/src/task/d...
pkg/analyzer/lib/src/task/dart.dart:1650: static const String UNIT_INPUT =
'UNIT_INPUT';
On 2015/05/07 22:46:59, Brian Wilkerson wrote:
> Perhaps "RESOLVED_UNIT_INPUT" to be consistent?

Personally I don't see much benefit in the longer name, since the names are
local to the task.  Also, most of the existing tasks that take a RESOLVED_UNIT
as an input call it UNIT_INPUT.

Leaving as is for now.

https://codereview.chromium.org/1131953004/diff/1/pkg/analyzer/lib/src/task/d...
pkg/analyzer/lib/src/task/dart.dart:1676: // to ensure that resolution has
occurred before we attempto to determine
On 2015/05/07 22:52:09, scheglov wrote:
> attempto -> attempt

Done.

https://codereview.chromium.org/1131953004/diff/1/pkg/analyzer/lib/src/task/d...
pkg/analyzer/lib/src/task/dart.dart:1681: TypeProvider typeProvider =
context.typeProvider;
On 2015/05/07 22:46:59, Brian Wilkerson wrote:
> I believe that you want to have a TYPE_PROVIDER input rather than getting the
> type provider directly from the context.

Hmm, I believe you are right, since some analysis of dart:core and dart:async
needs to be done before the type provider can be returned.  I will address this
in a follow-up CL.

Should my follow-up CL make this change to all tasks?  I notice that
GenerateHintsTask, ResolveReferencesTask, ResolveUnitTypeNamesTask,
ResolveVariableReferencesTask, and VerifyUnitTask all get the type provider
directly from the context.

https://codereview.chromium.org/1131953004/diff/1/pkg/analyzer/test/src/task/...
File pkg/analyzer/test/src/task/dart_test.dart (right):

https://codereview.chromium.org/1131953004/diff/1/pkg/analyzer/test/src/task/...
pkg/analyzer/test/src/task/dart_test.dart:1200: // Find the elements for the
constant x and y.
On 2015/05/07 22:46:59, Brian Wilkerson wrote:
> "constant" --> "constants"

Done.

[Paul Berry, 2015-05-08 00:36:58]

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

[scheglov, 2015-05-08 03:53:00]

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

https://codereview.chromium.org/1131953004/diff/1/pkg/analyzer/lib/src/task/d...
pkg/analyzer/lib/src/task/dart.dart:1681: TypeProvider typeProvider =
context.typeProvider;
On 2015/05/08 00:24:35, Paul Berry wrote:
> On 2015/05/07 22:46:59, Brian Wilkerson wrote:
> > I believe that you want to have a TYPE_PROVIDER input rather than getting
the
> > type provider directly from the context.
> 
> Hmm, I believe you are right, since some analysis of dart:core and dart:async
> needs to be done before the type provider can be returned.  I will address
this
> in a follow-up CL.
> 
> Should my follow-up CL make this change to all tasks?  I notice that
> GenerateHintsTask, ResolveReferencesTask, ResolveUnitTypeNamesTask,
> ResolveVariableReferencesTask, and VerifyUnitTask all get the type provider
> directly from the context.

I don't think we want to request all information we want to access as _direct_
inputs.

For example here we request RESOLVED_UNIT.
But we don't request also RESOLVED_UNIT1, RESOLVED_UNIT2, etc.
The reason is that we know that RESOLVED_UNIT will also have all information
prepared for RESOLVED_UNIT1, RESOLVED_UNIT2, etc.

In the same way RESOLVED_UNIT means that TYPE_PROVIDER was computed for the
context, because every RESOLVED_UNIT is created from RESOLVED_UNIT2, and every
RESOLVED_UNIT2 requests TYPE_PROVIDER.

[Brian Wilkerson, 2015-05-08 14:11:29]

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

https://codereview.chromium.org/1131953004/diff/1/pkg/analyzer/lib/src/task/d...
pkg/analyzer/lib/src/task/dart.dart:1681: TypeProvider typeProvider =
context.typeProvider;
> I don't think we want to request all information we want to access as _direct_
> inputs.

But not doing so could (although I admit the probability is small) cause
problems later when we change the task structure to improve performance.
Basically, we're hard-coding the assumption that a previous task had depended on
the type provider, and that seems dangerous (though again, the risk is
admittedly low).

Is there a reason to *not* make the dependency explicit?

[scheglov, 2015-05-08 15:14:36]

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

https://codereview.chromium.org/1131953004/diff/1/pkg/analyzer/lib/src/task/d...
pkg/analyzer/lib/src/task/dart.dart:1681: TypeProvider typeProvider =
context.typeProvider;
On 2015/05/08 14:11:29, Brian Wilkerson wrote:
> > I don't think we want to request all information we want to access as
_direct_
> > inputs.
> 
> But not doing so could (although I admit the probability is small) cause
> problems later when we change the task structure to improve performance.
> Basically, we're hard-coding the assumption that a previous task had depended
on
> the type provider, and that seems dangerous (though again, the risk is
> admittedly low).
> 
> Is there a reason to *not* make the dependency explicit?

No, there isn't.
Just consistency.