Issue 10916232: Implement checked mode for const constructors.

Issue 10916232: Implement checked mode for const constructors. (Closed)

Created:
8 years, 3 months ago by ngeoffray

Modified:
8 years, 3 months ago

Reviewers:
kasperl

CC:
reviews_dartlang.org

Base URL:
http://dart.googlecode.com/svn/branches/bleeding_edge/dart/

Visibility:
Public.

More Reviews

Description

Implement checked mode for const constructors. Committed: https://code.google.com/p/dart/source/detail?r=12370

Patch Set 5 : #

Created: 8 years, 3 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+118 lines, -4 lines)			Patch
	lib/compiler/implementation/compile_time_constants.dart	View	1 2 3 4	4 chunks	+25 lines, -2 lines	0 comments	Download
M	lib/compiler/implementation/constant_system.dart	View	1 2 3 4	1 chunk	+4 lines, -0 lines	0 comments	Download
M	lib/compiler/implementation/constant_system_dart.dart	View	1 2 3 4	1 chunk	+5 lines, -1 line	0 comments	Download
M	lib/compiler/implementation/js_backend/constant_system_javascript.dart	View	1 2 3 4	1 chunk	+12 lines, -1 line	0 comments	Download
M	tests/co19/co19-dart2js.status	View	1 2 3 4	1 chunk	+5 lines, -0 lines	0 comments	Download
A	tests/language/compile_time_constant_checked2_test.dart	View		1 chunk	+43 lines, -0 lines	0 comments	Download
M	tests/language/language.status	View	1 2 3 4	1 chunk	+12 lines, -0 lines	0 comments	Download
M	tests/language/language_dart2js.status	View	1 2 3 4	1 chunk	+12 lines, -0 lines	0 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

kasperl

LGTM. http://codereview.chromium.org/10916232/diff/3001/lib/compiler/implementation/compile_time_constants.dart File lib/compiler/implementation/compile_time_constants.dart (right): http://codereview.chromium.org/10916232/diff/3001/lib/compiler/implementation/compile_time_constants.dart#newcode696 lib/compiler/implementation/compile_time_constants.dart:696: // assignments have already been checked on checked ...

8 years, 3 months ago (2012-09-13 13:59:24 UTC) #3

ngeoffray

Thanks Kasper http://codereview.chromium.org/10916232/diff/3001/lib/compiler/implementation/compile_time_constants.dart File lib/compiler/implementation/compile_time_constants.dart (right): http://codereview.chromium.org/10916232/diff/3001/lib/compiler/implementation/compile_time_constants.dart#newcode696 lib/compiler/implementation/compile_time_constants.dart:696: // assignments have already been checked on ...

8 years, 3 months ago (2012-09-13 14:09:19 UTC) #4

ngeoffray

I had to change the isSubtype check in the compile time constant handler, since our ...

8 years, 3 months ago (2012-09-13 14:46:03 UTC) #5

kasperl

SLGTM. https://chromiumcodereview.appspot.com/10916232/diff/3002/lib/compiler/implementation/constant_system.dart File lib/compiler/implementation/constant_system.dart (right): https://chromiumcodereview.appspot.com/10916232/diff/3002/lib/compiler/implementation/constant_system.dart#newcode53 lib/compiler/implementation/constant_system.dart:53: bool isSubtype(Compiler compiler, DartType s, DartType t); Add ...

8 years, 3 months ago (2012-09-14 05:53:34 UTC) #6

ngeoffray

8 years, 3 months ago (2012-09-14 09:44:56 UTC) #7

Thanks Kasper

http://codereview.chromium.org/10916232/diff/3002/lib/compiler/implementation...
File lib/compiler/implementation/constant_system.dart (right):

http://codereview.chromium.org/10916232/diff/3002/lib/compiler/implementation...
lib/compiler/implementation/constant_system.dart:53: bool isSubtype(Compiler
compiler, DartType s, DartType t);
On 2012/09/14 05:53:34, kasperl wrote:
> Add a comment that explains why the constant system is involved in the subtype
> decision.

Done.

http://codereview.chromium.org/10916232/diff/3002/lib/compiler/implementation...
File lib/compiler/implementation/js_backend/constant_system_javascript.dart
(right):

http://codereview.chromium.org/10916232/diff/3002/lib/compiler/implementation...
lib/compiler/implementation/js_backend/constant_system_javascript.dart:222: if
(s.element == compiler.intClass && t.element == compiler.doubleClass) {
On 2012/09/14 05:53:34, kasperl wrote:
> Add a comment that explains why this is -- and why we do not need it in the
> other direction.

Done.

Expand Messages | Collapse Messages