From 64fd5a50792a9ad137b7805e7b48aa60a7355a97 Mon Sep 17 00:00:00 2001 From: "rmacnak@google.com" Date: Tue, 3 Sep 2013 20:11:11 +0000 Subject: [PATCH] Implement InstanceMirror.== in dart2js and InstanceMirror.hashCode in the VM and dart2js. BUG=http://dartbug.com/12909 BUG=http://dartbug.com/12919 R=ahe@google.com, asiva@google.com Review URL: https://codereview.chromium.org//23460013 git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@27075 260f80e4-7a28-3924-810f-c04153c831b5 --- runtime/lib/mirrors.cc | 12 ++ runtime/lib/mirrors_impl.dart | 17 ++- runtime/vm/bootstrap_natives.h | 1 + runtime/vm/symbols.h | 1 + sdk/lib/_internal/lib/js_mirrors.dart | 20 ++++ tests/lib/lib.status | 2 +- tests/lib/mirrors/equality_dart2js_test.dart | 112 +++++++++++++++++++ tests/lib/mirrors/equality_test.dart | 34 +++++- 8 files changed, 194 insertions(+), 5 deletions(-) create mode 100644 tests/lib/mirrors/equality_dart2js_test.dart diff --git a/runtime/lib/mirrors.cc b/runtime/lib/mirrors.cc index 17f4e94bcce..55b99b7d045 100644 --- a/runtime/lib/mirrors.cc +++ b/runtime/lib/mirrors.cc @@ -751,6 +751,18 @@ DEFINE_NATIVE_ENTRY(TypeVariableMirror_upper_bound, 1) { } +DEFINE_NATIVE_ENTRY(InstanceMirror_identityHash, 1) { + GET_NATIVE_ARGUMENT(Instance, reflectee, arguments->NativeArgAt(0)); + ObjectStore* object_store = isolate->object_store(); + const Class& cls = Class::Handle(isolate, object_store->object_class()); + const Function& function = + Function::Handle(isolate, cls.LookupDynamicFunction(Symbols::hashCode())); + const Array& args = Array::Handle(isolate, Array::New(1)); + args.SetAt(0, reflectee); + return DartEntry::InvokeFunction(function, args); +} + + // Invoke the function, or noSuchMethod if it is null. Propagate any unhandled // exceptions. Wrap and propagate any compilation errors. static RawObject* ReflectivelyInvokeDynamicFunction(const Instance& receiver, diff --git a/runtime/lib/mirrors_impl.dart b/runtime/lib/mirrors_impl.dart index ddf05f0b479..304b0a863fd 100644 --- a/runtime/lib/mirrors_impl.dart +++ b/runtime/lib/mirrors_impl.dart @@ -283,8 +283,21 @@ class _LocalInstanceMirrorImpl extends _LocalObjectMirrorImpl identical(_reflectee, other._reflectee); } - // TODO(12909): Use the reflectee's identity hash. - int get hashCode => _reflectee.hashCode; + int get hashCode { + // If the reflectee is a double or bignum, use the base hashCode to preserve + // the illusion that boxed numbers with the same value are identical. If the + // reflectee is a Smi, use the base hashCode because Object.hashCode does + // not work for non-heap objects. Otherwise, use Object.hashCode to maintain + // correctness even if a user-defined hashCode returns different values for + // successive invocations. + var h = _reflectee is num ? _reflectee.hashCode : _identityHash(_reflectee); + // Avoid hash collisions with the reflectee. This constant is in Smi range + // and happens to be the inner padding from RFC 2104. + return h ^ 0x36363636; + } + + static _identityHash(reflectee) + native "InstanceMirror_identityHash"; _invoke(reflectee, functionName, positionalArguments) native 'InstanceMirror_invoke'; diff --git a/runtime/vm/bootstrap_natives.h b/runtime/vm/bootstrap_natives.h index 1df9edf102a..d88357057e2 100644 --- a/runtime/vm/bootstrap_natives.h +++ b/runtime/vm/bootstrap_natives.h @@ -253,6 +253,7 @@ namespace dart { V(Mirrors_makeLocalTypeMirror, 1) \ V(Mirrors_makeLocalMirrorSystem, 0) \ V(MirrorReference_equals, 2) \ + V(InstanceMirror_identityHash, 1) \ V(InstanceMirror_invoke, 4) \ V(InstanceMirror_invokeGetter, 3) \ V(InstanceMirror_invokeSetter, 4) \ diff --git a/runtime/vm/symbols.h b/runtime/vm/symbols.h index c892cbeb274..4f4a670fdc6 100644 --- a/runtime/vm/symbols.h +++ b/runtime/vm/symbols.h @@ -284,6 +284,7 @@ class ObjectPointerVisitor; V(_LocalMirrorSystemImpl, "_LocalMirrorSystemImpl") \ V(_LocalTypedefMirrorImpl, "_LocalTypedefMirrorImpl") \ V(_LocalTypeVariableMirrorImpl, "_LocalTypeVariableMirrorImpl") \ + V(hashCode, "get:hashCode") \ V(_leftShiftWithMask32, "_leftShiftWithMask32") \ diff --git a/sdk/lib/_internal/lib/js_mirrors.dart b/sdk/lib/_internal/lib/js_mirrors.dart index d5db7e5f505..8911a689052 100644 --- a/sdk/lib/_internal/lib/js_mirrors.dart +++ b/sdk/lib/_internal/lib/js_mirrors.dart @@ -677,6 +677,26 @@ class JsInstanceMirror extends JsObjectMirror implements InstanceMirror { return JSInvocationMirror.invokeFromMirror(invocation, reflectee); } + operator ==(other) { + return other is JsInstanceMirror && + identical(reflectee, other.reflectee); + } + + int get hashCode { + // If the reflectee is a built-in type, use the base-level hashCode to + // preserve the illusion that, e.g. doubles, with the same value are + // identical. Otherwise, use the primitive identity hash to maintain + // correctness even if a user-defined hashCode returns different values for + // successive invocations. + var h = ((JS('bool', 'typeof # != "object"', reflectee)) || + (reflectee == null)) + ? reflectee.hashCode + : Primitives.objectHashCode(reflectee); + // Avoid hash collisions with the reflectee. This constant is in Smi range + // and happens to be the inner padding from RFC 2104. + return h ^ 0x36363636; + } + String toString() => 'InstanceMirror on ${Error.safeToString(reflectee)}'; // TODO(ahe): Remove this method from the API. diff --git a/tests/lib/lib.status b/tests/lib/lib.status index c93b83e3d6e..cfac8e909fb 100644 --- a/tests/lib/lib.status +++ b/tests/lib/lib.status @@ -13,7 +13,7 @@ math/random_test: Fail mirrors/invoke_test: Fail # Issue 11954 mirrors/class_mirror_type_variables_test: Fail # Issue 12087 mirrors/invoke_private_test: Fail # Issue 12164 -mirrors/equality_test: Fail # Issue 12333, 12919 +mirrors/equality_test: Fail # Issue 12333 mirrors/function_type_mirror_test: Fail # Issue 12166 mirrors/generics_test: Fail # Issue 12333 mirrors/hierarchy_invariants_test: Fail # Issue 11863 diff --git a/tests/lib/mirrors/equality_dart2js_test.dart b/tests/lib/mirrors/equality_dart2js_test.dart new file mode 100644 index 00000000000..15c036d4143 --- /dev/null +++ b/tests/lib/mirrors/equality_dart2js_test.dart @@ -0,0 +1,112 @@ +// Copyright (c) 2013, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +library test.class_equality_test; + +import 'dart:mirrors'; + +import 'package:expect/expect.dart'; + +class A {} +class B extends A {} + +class BadEqualityHash { + int count = 0; + bool operator ==(other) => true; + int get hashCode => count++; +} + +checkEquality(List equivalenceClasses) { + for (var equivalenceClass in equivalenceClasses) { + equivalenceClass.forEach((name, member) { + equivalenceClass.forEach((otherName, otherMember) { + // Reflexivity, symmetry and transitivity. + Expect.equals(member, + otherMember, + "$name == $otherName"); + Expect.equals(member.hashCode, + otherMember.hashCode, + "$name.hashCode == $otherName.hashCode"); + }); + for (var otherEquivalenceClass in equivalenceClasses) { + if (otherEquivalenceClass == equivalenceClass) continue; + otherEquivalenceClass.forEach((otherName, otherMember) { + Expect.notEquals(member, + otherMember, + "$name != $otherName"); // Exclusion. + // Hash codes may or may not be equal. + }); + } + }); + } +} + +void subroutine() { +} + +main() { + LibraryMirror thisLibrary = + currentMirrorSystem() + .findLibrary(const Symbol('test.class_equality_test')) + .single; + + var o1 = new Object(); + var o2 = new Object(); + + var badEqualityHash1 = new BadEqualityHash(); + var badEqualityHash2 = new BadEqualityHash(); + + checkEquality([ + {'reflect(o1)' : reflect(o1), + 'reflect(o1), again' : reflect(o1)}, + + {'reflect(o2)' : reflect(o2), + 'reflect(o2), again' : reflect(o2)}, + + {'reflect(badEqualityHash1)' : reflect(badEqualityHash1), + 'reflect(badEqualityHash1), again' : reflect(badEqualityHash1)}, + + {'reflect(badEqualityHash2)' : reflect(badEqualityHash2), + 'reflect(badEqualityHash2), again' : reflect(badEqualityHash2)}, + + {'reflect(true)' : reflect(true), + 'reflect(true), again' : reflect(true)}, + + {'reflect(false)' : reflect(false), + 'reflect(false), again' : reflect(false)}, + + {'reflect(null)' : reflect(null), + 'reflect(null), again' : reflect(null)}, + + {'reflect(3.5+4.5)' : reflect(3.5+4.5), + 'reflect(6.5+1.5)' : reflect(6.5+1.5)}, + + {'reflect(3+4)' : reflect(3+4), + 'reflect(6+1)' : reflect(6+1)}, + + {'reflect("foo")' : reflect("foo"), + 'reflect("foo"), again' : reflect("foo")}, + + {'currentMirrorSystem().voidType' : currentMirrorSystem().voidType}, + + {'currentMirrorSystem().dynamicType' : currentMirrorSystem().dynamicType, + 'thisLibrary.functions[#main].returnType' : + thisLibrary.functions[const Symbol('main')].returnType}, + + {'reflectClass(A)' : reflectClass(A), + 'thisLibrary.classes[#A]' : thisLibrary.classes[const Symbol('A')], + 'reflect(new A()).type.originalDeclaration' : + reflect(new A()).type.originalDeclaration}, + + {'reflectClass(B)' : reflectClass(B), + 'thisLibrary.classes[#B]' : thisLibrary.classes[const Symbol('B')], + 'reflect(new B()).type' : reflect(new B()).type}, + + {'thisLibrary' : thisLibrary, + 'reflectClass(A).owner' : reflectClass(A).owner, + 'reflectClass(B).owner' : reflectClass(B).owner, + 'reflect(new A()).type.owner' : reflect(new A()).type.owner, + 'reflect(new A()).type.owner' : reflect(new A()).type.owner}, + ]); +} diff --git a/tests/lib/mirrors/equality_test.dart b/tests/lib/mirrors/equality_test.dart index 2399032fc4a..1a08b599fa2 100644 --- a/tests/lib/mirrors/equality_test.dart +++ b/tests/lib/mirrors/equality_test.dart @@ -11,6 +11,12 @@ import 'package:expect/expect.dart'; class A {} class B extends A {} +class BadEqualityHash { + int count = 0; + bool operator ==(other) => true; + int get hashCode => count++; +} + checkEquality(List equivalenceClasses) { for (var equivalenceClass in equivalenceClasses) { equivalenceClass.forEach((name, member) { @@ -45,8 +51,11 @@ main() { .findLibrary(const Symbol('test.class_equality_test')) .single; - Object o1 = new Object(); - Object o2 = new Object(); + var o1 = new Object(); + var o2 = new Object(); + + var badEqualityHash1 = new BadEqualityHash(); + var badEqualityHash2 = new BadEqualityHash(); checkEquality([ {'reflect(o1)' : reflect(o1), @@ -55,9 +64,30 @@ main() { {'reflect(o2)' : reflect(o2), 'reflect(o2), again' : reflect(o2)}, + {'reflect(badEqualityHash1)' : reflect(badEqualityHash1), + 'reflect(badEqualityHash1), again' : reflect(badEqualityHash1)}, + + {'reflect(badEqualityHash2)' : reflect(badEqualityHash2), + 'reflect(badEqualityHash2), again' : reflect(badEqualityHash2)}, + + {'reflect(true)' : reflect(true), + 'reflect(true), again' : reflect(true)}, + + {'reflect(false)' : reflect(false), + 'reflect(false), again' : reflect(false)}, + + {'reflect(null)' : reflect(null), + 'reflect(null), again' : reflect(null)}, + + {'reflect(3.5+4.5)' : reflect(3.5+4.5), + 'reflect(6.5+1.5)' : reflect(6.5+1.5)}, + {'reflect(3+4)' : reflect(3+4), 'reflect(6+1)' : reflect(6+1)}, + {'reflect("foo")' : reflect("foo"), + 'reflect("foo"), again' : reflect("foo")}, + {'currentMirrorSystem().voidType' : currentMirrorSystem().voidType, 'thisLibrary.functions[#subroutine].returnType' : thisLibrary.functions[const Symbol('subroutine')].returnType},