Typecheck const classes in the context of the constructor call.

Typecheck const classes in the context of the constructor call.

Closes #23182

BUG=https://github.com/dart-lang/sdk/issues/23182
R=johnniwinther@google.com, sigmund@google.com

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

[Harry Terkelsen, 2015-06-16 17:36:14]

het@google.com changed reviewers:
+ sigmund@google.com

[Siggi Cherem (dart-lang), 2015-06-16 18:19:06]

lgtm

just some ideas below, but I'm OK addressing those separately.

https://codereview.chromium.org/1182663008/diff/1/tests/compiler/dart2js/type...
File tests/compiler/dart2js/type_variable_bound_test.dart (right):

https://codereview.chromium.org/1182663008/diff/1/tests/compiler/dart2js/type...
tests/compiler/dart2js/type_variable_bound_test.dart:241: final T x = const A();
mmm... we got a tricky situation here on how to report the error. 

It seems like we are reporting the error on this line, but the actual problem is
on the const Test<B>() below where we actually decide what T is. I wonder if we
should move the error location there and provide some context about this line?
(for example like we do with duplicate definitions and show both the duplicate
and the existing definition)

https://codereview.chromium.org/1182663008/diff/1/tests/compiler/dart2js/type...
tests/compiler/dart2js/type_variable_bound_test.dart:247: """, errors:
[MessageKind.NOT_ASSIGNABLE, MessageKind.NOT_ASSIGNABLE]);
not sure if it's worth it, but maybe we can add some extra logic to check where
the error is reported (similar to test2?)

[Johnni Winther, 2015-06-17 06:56:28]

johnniwinther@google.com changed reviewers:
+ johnniwinther@google.com

[Johnni Winther, 2015-06-17 06:56:29]

lgtm

https://codereview.chromium.org/1182663008/diff/1/pkg/compiler/lib/src/compil...
File pkg/compiler/lib/src/compile_time_constants.dart (right):

https://codereview.chromium.org/1182663008/diff/1/pkg/compiler/lib/src/compil...
pkg/compiler/lib/src/compile_time_constants.dart:157: {bool checkType: true}) {
This method overrides ConstantCompiler.compileConstant; add [checkType] to it,
or create a new internal variant of compileConstant.

Document [checkType].

https://codereview.chromium.org/1182663008/diff/1/pkg/compiler/lib/src/compil...
pkg/compiler/lib/src/compile_time_constants.dart:163: bool checkType: true}) {
Ditto.

https://codereview.chromium.org/1182663008/diff/1/tests/compiler/dart2js/type...
File tests/compiler/dart2js/type_variable_bound_test.dart (right):

https://codereview.chromium.org/1182663008/diff/1/tests/compiler/dart2js/type...
tests/compiler/dart2js/type_variable_bound_test.dart:12: var compiler =
compilerFor(source, uri, analyzeOnly: true, enableTypeAssertions: true);
Long line.

[Johnni Winther, 2015-08-07 12:24:01]

Shouldn't this CL be committed?

[Harry Terkelsen, 2015-08-10 23:49:35]

https://codereview.chromium.org/1182663008/diff/1/pkg/compiler/lib/src/compil...
File pkg/compiler/lib/src/compile_time_constants.dart (right):

https://codereview.chromium.org/1182663008/diff/1/pkg/compiler/lib/src/compil...
pkg/compiler/lib/src/compile_time_constants.dart:157: {bool checkType: true}) {
On 2015/06/17 06:56:29, Johnni Winther wrote:
> This method overrides ConstantCompiler.compileConstant; add [checkType] to it,
> or create a new internal variant of compileConstant.
> 
> Document [checkType].

Done.

https://codereview.chromium.org/1182663008/diff/1/pkg/compiler/lib/src/compil...
pkg/compiler/lib/src/compile_time_constants.dart:163: bool checkType: true}) {
On 2015/06/17 06:56:29, Johnni Winther wrote:
> Ditto.

Done.

https://codereview.chromium.org/1182663008/diff/1/tests/compiler/dart2js/type...
File tests/compiler/dart2js/type_variable_bound_test.dart (right):

https://codereview.chromium.org/1182663008/diff/1/tests/compiler/dart2js/type...
tests/compiler/dart2js/type_variable_bound_test.dart:12: var compiler =
compilerFor(source, uri, analyzeOnly: true, enableTypeAssertions: true);
On 2015/06/17 06:56:29, Johnni Winther wrote:
> Long line.

Done.

https://codereview.chromium.org/1182663008/diff/1/tests/compiler/dart2js/type...
tests/compiler/dart2js/type_variable_bound_test.dart:241: final T x = const A();
On 2015/06/16 18:19:06, Siggi Cherem (dart-lang) wrote:
> mmm... we got a tricky situation here on how to report the error. 
> 
> It seems like we are reporting the error on this line, but the actual problem
is
> on the const Test<B>() below where we actually decide what T is. I wonder if
we
> should move the error location there and provide some context about this line?
> (for example like we do with duplicate definitions and show both the duplicate
> and the existing definition)

I agree, but I added a TODO rather than implement this now. The issue is that
you only want to put the error on the construction invocation if the type error
is due to the concrete type variable in the constructor invocation.

[Harry Terkelsen, 2015-08-10 23:49:49]

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