Remove const instance creation expressions from the dependency graph.

Remove const instance creation expressions from the dependency graph.

Instead of treating them as separate nodes in the dependency graph,
evaluate them directly in the ConstantVisitor at the time they are
needed.  This makes the dependency graph more uniform, since
everything in it is now associated with an element.  This should make
it easier to adapt constant evaluation to the new task model.

In order to avoid introducing regressions with this change, I had to
implement proper reporting of cycles in compile-time constants.
(Previously, this was unimplemented but worked correctly in most cases
by luck.)

Note: technically this constitutes a breaking API change, since it
removes the field constantHandle and the getter evaluationResult from
the InstanceCreationExpression class.  I believe these members are not
used outside of analyzer, so I'm leaving the analyzer version number
as is.

R=brianwilkerson@google.com

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

[Paul Berry, 2015-05-05 21:00:18]

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

[Brian Wilkerson, 2015-05-05 21:25:24]

LGTM

https://codereview.chromium.org/1124733008/diff/1/pkg/analyzer/lib/src/genera...
File pkg/analyzer/lib/src/generated/constant.dart (right):

https://codereview.chromium.org/1124733008/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/constant.dart:1220:
CompileTimeErrorCode.RECURSIVE_COMPILE_TIME_CONSTANT, element, []);
It would be really nice if we could extract enough information from the 'cycle'
argument to provide the user with a description of the cycle. Perhaps a TODO
comment for now so that we don't forget it.

https://codereview.chromium.org/1124733008/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/constant.dart:1237: "Constant value computer
trying to report a cycle error for an unexpected node");
It would be useful for the error message to include the type of the node,
something like:
  ... for a node of type ${constNode.runtimeType}");

https://codereview.chromium.org/1124733008/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/constant.dart:1238: return;
Not sure why we have an explicit return here.

Not sure why we have "assert(false);" in the two previous 'else if' clauses but
not here.

Not sure why we don't log an error message in the previous two 'else if'
clauses.

Perhaps drop the other two clauses and move the 'assert' to the 'else'.

https://codereview.chromium.org/1124733008/diff/1/pkg/analyzer/test/generated...
File pkg/analyzer/test/generated/compile_time_error_code_test.dart (right):

https://codereview.chromium.org/1124733008/diff/1/pkg/analyzer/test/generated...
pkg/analyzer/test/generated/compile_time_error_code_test.dart:1354:
CompileTimeErrorCode.INVALID_CONSTANT
This new error (both here and below) also seems to be redundant.

[Paul Berry, 2015-05-05 21:46:32]

https://codereview.chromium.org/1124733008/diff/1/pkg/analyzer/lib/src/genera...
File pkg/analyzer/lib/src/generated/constant.dart (right):

https://codereview.chromium.org/1124733008/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/constant.dart:1220:
CompileTimeErrorCode.RECURSIVE_COMPILE_TIME_CONSTANT, element, []);
On 2015/05/05 21:25:24, Brian Wilkerson wrote:
> It would be really nice if we could extract enough information from the
'cycle'
> argument to provide the user with a description of the cycle. Perhaps a TODO
> comment for now so that we don't forget it.

Done.

https://codereview.chromium.org/1124733008/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/constant.dart:1237: "Constant value computer
trying to report a cycle error for an unexpected node");
On 2015/05/05 21:25:24, Brian Wilkerson wrote:
> It would be useful for the error message to include the type of the node,
> something like:
>   ... for a node of type ${constNode.runtimeType}");

Done.

https://codereview.chromium.org/1124733008/diff/1/pkg/analyzer/lib/src/genera...
pkg/analyzer/lib/src/generated/constant.dart:1238: return;
On 2015/05/05 21:25:24, Brian Wilkerson wrote:
> Not sure why we have an explicit return here.
> 
> Not sure why we have "assert(false);" in the two previous 'else if' clauses
but
> not here.
> 
> Not sure why we don't log an error message in the previous two 'else if'
> clauses.
> 
> Perhaps drop the other two clauses and move the 'assert' to the 'else'.

Done.

https://codereview.chromium.org/1124733008/diff/1/pkg/analyzer/test/generated...
File pkg/analyzer/test/generated/compile_time_error_code_test.dart (right):

https://codereview.chromium.org/1124733008/diff/1/pkg/analyzer/test/generated...
pkg/analyzer/test/generated/compile_time_error_code_test.dart:1354:
CompileTimeErrorCode.INVALID_CONSTANT
On 2015/05/05 21:25:24, Brian Wilkerson wrote:
> This new error (both here and below) also seems to be redundant.

Agreed.  I've added TODO comments.

[Paul Berry, 2015-05-05 21:46:39]

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