Use normal equality for const maps.

Use normal equality for const maps in dart2js.

R=lrn@google.com

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

[Lasse Reichstein Nielsen, 2014-02-27 08:15:34]

https://codereview.chromium.org/177053004/diff/1/sdk/lib/_internal/lib/consta...
File sdk/lib/_internal/lib/constant_map.dart (right):

https://codereview.chromium.org/177053004/diff/1/sdk/lib/_internal/lib/consta...
sdk/lib/_internal/lib/constant_map.dart:107: JS('', r'this.$map = #',
fillLiteralMap(_jsData, backingMap));
The spec is silent on how constant maps work, but the constant keys objects must
not implement equals, or they must be String or int. 
So, in dart2js where strings are identical if equal, this should change nothing,
correct?

Except if the objects override hashCode in an incompatible way, but then they
deserve what they get.

https://codereview.chromium.org/177053004/diff/1/tests/language/const_map_tes...
File tests/language/const_map_test.dart (right):

https://codereview.chromium.org/177053004/diff/1/tests/language/const_map_tes...
tests/language/const_map_test.dart:11: if (new DateTime.now() == 42) return
confuse(const { B: 42 });
Type objects do implement operator==, so this should be a compile time error.
You can't use type constants in constant maps.

This is visible behavior in dart2js:

class C {}
class X<T> { T get type => T; }

const Type t1 = C;    // force it to be a compile-time constant.

main() {
 Type t2 = new X<C>().type; 
 print(identical(t1, t2));  // false
 print(t1 == t2);           // true - so "C" overrides operator==.
}

[floitsch, 2014-02-28 18:26:36]

Updated test, since https://codereview.chromium.org/179293005/ will make Type in
const maps illegal.

The change in the Map implementation (switching from identical to equality)
should not have any effect anymore (once the other CL lands). However I think
it's better if we just use the same type as the normal Map literals.

[floitsch, 2014-02-28 18:26:53]

PTAL.

[Lasse Reichstein Nielsen, 2014-03-03 10:31:24]

lgtm

https://codereview.chromium.org/177053004/diff/60001/sdk/lib/_internal/lib/co...
File sdk/lib/_internal/lib/constant_map.dart (right):

https://codereview.chromium.org/177053004/diff/60001/sdk/lib/_internal/lib/co...
sdk/lib/_internal/lib/constant_map.dart:106: Map backingMap = new Map<K, V>();
I prefer to still write "LinkedHashMap" so it is obvious what map is being used.
It's just the LinkedHashMap with default equalities.

https://codereview.chromium.org/177053004/diff/60001/tests/language/const_map...
File tests/language/const_map_test.dart (right):

https://codereview.chromium.org/177053004/diff/60001/tests/language/const_map...
tests/language/const_map_test.dart:7: confuse(x) {
Document "confuse": Used to prevent static optimizations and inlining by naking
the value too hard to predict at compile time.

https://codereview.chromium.org/177053004/diff/60001/tests/language/const_map...
tests/language/const_map_test.dart:13: // Make sure that const maps use the ==
operator and not identical.
Way that the spec doesn't require this explicitly, but it does so implicitly (by
allowing int and String keys even if they override operator==).

Luckily the VM agrees.

(And, also luckliy, you can't use doubles as keys, or we would have to figure
out what const{1:42,1.0:37} would mean :).

[floitsch, 2014-03-03 10:42:37]

https://codereview.chromium.org/177053004/diff/60001/sdk/lib/_internal/lib/co...
File sdk/lib/_internal/lib/constant_map.dart (right):

https://codereview.chromium.org/177053004/diff/60001/sdk/lib/_internal/lib/co...
sdk/lib/_internal/lib/constant_map.dart:106: Map backingMap = new Map<K, V>();
On 2014/03/03 10:31:24, Lasse Reichstein Nielsen wrote:
> I prefer to still write "LinkedHashMap" so it is obvious what map is being
used.
> It's just the LinkedHashMap with default equalities.

Done.

https://codereview.chromium.org/177053004/diff/60001/tests/language/const_map...
File tests/language/const_map_test.dart (right):

https://codereview.chromium.org/177053004/diff/60001/tests/language/const_map...
tests/language/const_map_test.dart:7: confuse(x) {
On 2014/03/03 10:31:24, Lasse Reichstein Nielsen wrote:
> Document "confuse": Used to prevent static optimizations and inlining by
naking
> the value too hard to predict at compile time.

Done.

https://codereview.chromium.org/177053004/diff/60001/tests/language/const_map...
tests/language/const_map_test.dart:13: // Make sure that const maps use the ==
operator and not identical.
On 2014/03/03 10:31:24, Lasse Reichstein Nielsen wrote:
> Way that the spec doesn't require this explicitly, but it does so implicitly
(by
> allowing int and String keys even if they override operator==).
> 
> Luckily the VM agrees.
> 
> (And, also luckliy, you can't use doubles as keys, or we would have to figure
> out what const{1:42,1.0:37} would mean :).
> 

Added string test and added more comment.

[floitsch, 2014-03-03 10:43:37]

Committed patchset #5 manually as r33205 (presubmit successful).