1290587 - Clean up XPCNativeSetKey hashing a little bit

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Comment 1

[Andrew McCreight [:mccr8]]

This is just to make further more substantial changes easier.

Comment 2

[Andrew McCreight [:mccr8]]

Attachment: Bug 1290587, part 1 - Fix style for XPCNativeSetKey.

Review commit: https://reviewboard.mozilla.org/r/68166/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/68166/

Comment 3

[Andrew McCreight [:mccr8]]

Attachment: Bug 1290587, part 2 - Make XPCNativeSetKey hashing a method.

Review commit: https://reviewboard.mozilla.org/r/68168/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/68168/

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: Bug 1290587, part 3 - Inline accessors in XPCNativeSetKey::Hash().

Review commit: https://reviewboard.mozilla.org/r/68170/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/68170/

Comment 5

[Andrew McCreight [:mccr8]]

Attachment: Bug 1290587, part 4 - Lower case Current in XPCNativeSetKey::Hash().

Review commit: https://reviewboard.mozilla.org/r/68172/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/68172/

Comment 6

[Andrew McCreight [:mccr8]]

Attachment: Bug 1290587, part 5 - Add helper function to hash pointers in HashNativeKey.

Also, use NS_PTR_TO_UINT32 instead of NS_PTR_TO_INT32 because it is not
undefined.

Get rid of the optimization of 0 ^ x which required a comment.

Review commit: https://reviewboard.mozilla.org/r/68174/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/68174/

Comment 7

[Andrew McCreight [:mccr8]]

Attachment: Bug 1290587, part 6 - Remove unused method NativeSetMap::Add.

Also convert some NS_PRECONDITION in NativeSetMap.

Review commit: https://reviewboard.mozilla.org/r/68176/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/68176/

Comment 8

[Andrew McCreight [:mccr8]]

There's a bunch of patches here but there's nothing too complex going on.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d84c376c45ad

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8776397 [details]
Bug 1290587, part 5 - Add helper function to hash pointers in HashNativeKey.

https://reviewboard.mozilla.org/r/68174/#review65438

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8776393 [details]
Bug 1290587, part 1 - Fix style for XPCNativeSetKey.

https://reviewboard.mozilla.org/r/68166/#review65440

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8776394 [details]
Bug 1290587, part 2 - Make XPCNativeSetKey hashing a method.

https://reviewboard.mozilla.org/r/68168/#review65444

Comment 12

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8776395 [details]
Bug 1290587, part 3 - Inline accessors in XPCNativeSetKey::Hash().

https://reviewboard.mozilla.org/r/68170/#review65446

Comment 13

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8776396 [details]
Bug 1290587, part 4 - Lower case Current in XPCNativeSetKey::Hash().

https://reviewboard.mozilla.org/r/68172/#review65448

Comment 14

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 8776398 [details]
Bug 1290587, part 6 - Remove unused method NativeSetMap::Add.

https://reviewboard.mozilla.org/r/68176/#review65436

Comment 15

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to Andrew McCreight [:mccr8] from comment #6)
> Also, use NS_PTR_TO_UINT32 instead of NS_PTR_TO_INT32 because it is not
> undefined.'

As a commit message this is pretty unclear. I'm actually not entirely sure what you mean by it. The change itself is correct, though.

Comment 16

[Andrew McCreight [:mccr8]]

(In reply to Blake Kaplan (:mrbkap) from comment #15)
> As a commit message this is pretty unclear. I'm actually not entirely sure
> what you mean by it. The change itself is correct, though.

Oh sorry. I just picked that up from bug 1047176. I think MSVC does or did complain about casting from a signed integer to an unsigned integer (PLDHashNumber is unsigned). I'm not sure if that is defined behavior or not.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/59d623592228
part 1 - Fix style for XPCNativeSetKey. r=mrbkap
https://hg.mozilla.org/integration/autoland/rev/bcf7690fc657
part 2 - Make XPCNativeSetKey hashing a method. r=mrbkap
https://hg.mozilla.org/integration/autoland/rev/81072f3a058a
part 3 - Inline accessors in XPCNativeSetKey::Hash(). r=mrbkap
https://hg.mozilla.org/integration/autoland/rev/2376df9ef2b7
part 4 - Lower case Current in XPCNativeSetKey::Hash(). r=mrbkap
https://hg.mozilla.org/integration/autoland/rev/4dff23362d43
part 5 - Add helper function to hash pointers in HashNativeKey. r=mrbkap
https://hg.mozilla.org/integration/autoland/rev/4c2020735c69
part 6 - Remove unused method NativeSetMap::Add. r=mrbkap

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/59d623592228
https://hg.mozilla.org/mozilla-central/rev/bcf7690fc657
https://hg.mozilla.org/mozilla-central/rev/81072f3a058a
https://hg.mozilla.org/mozilla-central/rev/2376df9ef2b7
https://hg.mozilla.org/mozilla-central/rev/4dff23362d43
https://hg.mozilla.org/mozilla-central/rev/4c2020735c69