From ab70c8c9421f09df9f705628d405de3109dcefd3 Mon Sep 17 00:00:00 2001 From: Rohan Prinja Date: Wed, 21 Oct 2015 01:40:19 +0900 Subject: [PATCH 1/5] implement PartialEq for MutHeap> and MutNullableHeap> --- components/script/dom/bindings/js.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/components/script/dom/bindings/js.rs b/components/script/dom/bindings/js.rs index 1ef1d93a86d..7c44555924d 100644 --- a/components/script/dom/bindings/js.rs +++ b/components/script/dom/bindings/js.rs @@ -260,6 +260,12 @@ impl HeapSizeOf for MutHeap { } } +impl PartialEq for MutHeap> { + fn eq(&self, other: &MutHeap>) -> bool { + self.get().eq(&other.get()) + } +} + /// A holder that provides interior mutability for GC-managed values such as /// `JS`, with nullability represented by an enclosing Option wrapper. /// Essentially a `Cell>>`, but safer. @@ -337,6 +343,12 @@ impl HeapSizeOf for MutNullableHeap { } } +impl PartialEq for MutNullableHeap> { + fn eq(&self, other: &MutNullableHeap>) -> bool { + self.get().eq(&other.get()) + } +} + impl LayoutJS { /// Returns an unsafe pointer to the interior of this JS object. This is /// the only method that be safely accessed from layout. (The fact that From 5e8dc366debdaac349b7aba204f29c53dfb1f827 Mon Sep 17 00:00:00 2001 From: Rohan Prinja Date: Thu, 22 Oct 2015 01:05:48 +0900 Subject: [PATCH 2/5] in PartialEq impls, use Self instead of explicitly naming the struct type --- components/script/dom/bindings/js.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/script/dom/bindings/js.rs b/components/script/dom/bindings/js.rs index 7c44555924d..5e8ce0d731d 100644 --- a/components/script/dom/bindings/js.rs +++ b/components/script/dom/bindings/js.rs @@ -261,7 +261,7 @@ impl HeapSizeOf for MutHeap { } impl PartialEq for MutHeap> { - fn eq(&self, other: &MutHeap>) -> bool { + fn eq(&self, other: &Self) -> bool { self.get().eq(&other.get()) } } @@ -344,7 +344,7 @@ impl HeapSizeOf for MutNullableHeap { } impl PartialEq for MutNullableHeap> { - fn eq(&self, other: &MutNullableHeap>) -> bool { + fn eq(&self, other: &Self>) -> bool { self.get().eq(&other.get()) } } From d9f8f686155da67322bcaf8a654596af11b900eb Mon Sep 17 00:00:00 2001 From: Rohan Prinja Date: Thu, 22 Oct 2015 03:05:13 +0900 Subject: [PATCH 3/5] add equal() methods for comparisons with other types --- components/script/dom/bindings/js.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/components/script/dom/bindings/js.rs b/components/script/dom/bindings/js.rs index 5e8ce0d731d..3e4cfeb0fce 100644 --- a/components/script/dom/bindings/js.rs +++ b/components/script/dom/bindings/js.rs @@ -251,6 +251,11 @@ impl MutHeap> { ptr::read(self.val.get()).root() } } + + /// Compare this object to an `&T` value. + fn eq(&self, other: &T) -> bool { + self.get() == Root::from_ref(other) + } } impl HeapSizeOf for MutHeap { @@ -325,6 +330,11 @@ impl MutNullableHeap> { *self.ptr.get() = val.map(|p| JS::from_ref(p)); } } + + /// Compare this object to an `Option<&T>` value. + fn equal(&self, other: Option<&T>) -> bool { + self.get() == other.map(|p| Root::from_ref(p)) + } } impl Default for MutNullableHeap { @@ -344,7 +354,7 @@ impl HeapSizeOf for MutNullableHeap { } impl PartialEq for MutNullableHeap> { - fn eq(&self, other: &Self>) -> bool { + fn eq(&self, other: &Self) -> bool { self.get().eq(&other.get()) } } From fe59b62db7e342abdc7b706a633ec4cfb9448b1b Mon Sep 17 00:00:00 2001 From: Rohan Prinja Date: Sat, 24 Oct 2015 01:42:22 +0900 Subject: [PATCH 4/5] make changes according to code review (#8097) --- components/script/dom/bindings/js.rs | 41 ++++++++++++++++++---------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/components/script/dom/bindings/js.rs b/components/script/dom/bindings/js.rs index 3e4cfeb0fce..66d3074edc7 100644 --- a/components/script/dom/bindings/js.rs +++ b/components/script/dom/bindings/js.rs @@ -251,11 +251,6 @@ impl MutHeap> { ptr::read(self.val.get()).root() } } - - /// Compare this object to an `&T` value. - fn eq(&self, other: &T) -> bool { - self.get() == Root::from_ref(other) - } } impl HeapSizeOf for MutHeap { @@ -267,7 +262,17 @@ impl HeapSizeOf for MutHeap { impl PartialEq for MutHeap> { fn eq(&self, other: &Self) -> bool { - self.get().eq(&other.get()) + unsafe { + *self.val.get() == *other.val.get() + } + } +} + +impl PartialEq for MutHeap> { + fn eq(&self, other: &T) -> bool { + unsafe { + *self.val.get() == JS::from_ref(other) + } } } @@ -331,9 +336,21 @@ impl MutNullableHeap> { } } - /// Compare this object to an `Option<&T>` value. - fn equal(&self, other: Option<&T>) -> bool { - self.get() == other.map(|p| Root::from_ref(p)) +} + +impl PartialEq for MutNullableHeap> { + fn eq(&self, other: &Self) -> bool { + unsafe { + *self.ptr.get() == *other.ptr.get() + } + } +} + +impl<'a, T: Reflectable> PartialEq> for MutNullableHeap> { + fn eq(&self, other: &Option<&T>) -> bool { + unsafe { + *self.ptr.get() == other.map(|p| JS::from_ref(p)) + } } } @@ -353,12 +370,6 @@ impl HeapSizeOf for MutNullableHeap { } } -impl PartialEq for MutNullableHeap> { - fn eq(&self, other: &Self) -> bool { - self.get().eq(&other.get()) - } -} - impl LayoutJS { /// Returns an unsafe pointer to the interior of this JS object. This is /// the only method that be safely accessed from layout. (The fact that From c1abb4fdb2fd20556cb5399a2ceb2e2c1d662913 Mon Sep 17 00:00:00 2001 From: Rohan Prinja Date: Mon, 26 Oct 2015 03:52:05 +0900 Subject: [PATCH 5/5] clean code as per code review --- components/script/dom/bindings/js.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/script/dom/bindings/js.rs b/components/script/dom/bindings/js.rs index 66d3074edc7..8940ca7bd0a 100644 --- a/components/script/dom/bindings/js.rs +++ b/components/script/dom/bindings/js.rs @@ -268,10 +268,10 @@ impl PartialEq for MutHeap> { } } -impl PartialEq for MutHeap> { +impl PartialEq for MutHeap> { fn eq(&self, other: &T) -> bool { unsafe { - *self.val.get() == JS::from_ref(other) + **self.val.get() == *other } } } @@ -349,7 +349,7 @@ impl PartialEq for MutNullableHeap> { impl<'a, T: Reflectable> PartialEq> for MutNullableHeap> { fn eq(&self, other: &Option<&T>) -> bool { unsafe { - *self.ptr.get() == other.map(|p| JS::from_ref(p)) + *self.ptr.get() == other.map(JS::from_ref) } } }