Issue 10908287: Make identical(a,b) a compile-time constant in dart2js.

Issue 10908287: Make identical(a,b) a compile-time constant in dart2js. (Closed)

Created:
8 years, 3 months ago by Lasse Reichstein Nielsen

Modified:
8 years, 3 months ago

Reviewers:
ahe, ngeoffray

CC:
reviews_dartlang.org

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

Visibility:
Public.

More Reviews

Description

Make identical(a,b) a compile-time constant in dart2js. Introduces the 'identical' top-level function in dart:core. Committed: https://code.google.com/p/dart/source/detail?r=12465

Patch Set 2 : Address review comment. #

Total comments: 2

Created: 8 years, 3 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+152 lines, -29 lines)			Patch
M	lib/compiler/implementation/compile_time_constants.dart	View		3 chunks	+32 lines, -27 lines	0 comments	Download
M	lib/compiler/implementation/compiler.dart	View		2 chunks	+2 lines, -0 lines	1 comment	Download
M	lib/core/object.dart	View		2 chunks	+6 lines, -1 line	1 comment	Download
M	tests/compiler/dart2js/mock_compiler.dart	View		1 chunk	+2 lines, -1 line	0 comments	Download
A	tests/language/compile_time_constant10_test.dart	View		1 chunk	+107 lines, -0 lines	0 comments	Download
M	tests/language/language.status	View	1	2 chunks	+3 lines, -0 lines	0 comments	Download

Messages

Total messages: 4 (0 generated)

Expand Messages | Collapse Messages

ngeoffray

LGTM https://codereview.chromium.org/10908287/diff/1/tests/language/language.status File tests/language/language.status (right): https://codereview.chromium.org/10908287/diff/1/tests/language/language.status#newcode161 tests/language/language.status:161: compiler_time_constant10_test: Fail # issue 5215. compiler_time -> compile_time

8 years, 3 months ago (2012-09-17 19:28:47 UTC) #2

Lasse Reichstein Nielsen

https://codereview.chromium.org/10908287/diff/1/tests/language/language.status File tests/language/language.status (right): https://codereview.chromium.org/10908287/diff/1/tests/language/language.status#newcode161 tests/language/language.status:161: compiler_time_constant10_test: Fail # issue 5215. Well spotted.

8 years, 3 months ago (2012-09-17 19:35:56 UTC) #3

ahe

8 years, 3 months ago (2012-09-18 08:30:17 UTC) #4

DBC

I would really prefer if you make changes to the core library separate from
changes to dart2js. It is harder for the Editor and VM teams to follow changes
to the core library if they are "polluted" with dart2js changes.

Also, if this change broke because of something you overlooked in dart2js, and
you had to revert it, the change to the core library would also be reverted.

https://chromiumcodereview.appspot.com/10908287/diff/1007/lib/compiler/implem...
File lib/compiler/implementation/compiler.dart (right):

https://chromiumcodereview.appspot.com/10908287/diff/1007/lib/compiler/implem...
lib/compiler/implementation/compiler.dart:371: identicalFunction =
coreLibrary.find(const SourceString('identical'));
This doesn't report an error if it cannot find 'identical'. Same problem for
'assert'. So you forgot to update dart/tests/utils/dummy_compiler_test.dart, and
possibly other tests.

Perhaps we should report errors here.

https://chromiumcodereview.appspot.com/10908287/diff/1007/lib/core/object.dart
File lib/core/object.dart (right):

https://chromiumcodereview.appspot.com/10908287/diff/1007/lib/core/object.dar...
lib/core/object.dart:23: bool identical(Object a, Object b) => a === b;
This file is called "object.dart". This function is not part of Object.

Expand Messages | Collapse Messages