1475461 - Make PLDHashTable::Search() const

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

PLDHashTable::Search() is now not marked as const.  Therefore, if its caller is marked as const, it needs to use const_cast.  So, let's mark the method (and related methods) as const.

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

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

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1475461 - part 1: Mark PLDHashTable::Search() and called by it as const

PLDHashTable::Search() does not modify any members.  So, this method and
methods called by it should be marked as const.

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

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1475461 - part 2: Make callers of PLDHashTable::Search() const methods if possible

Some callers of PLDHashTable::Search() use const_cast, some others are not
const methods due to non-const PLDHashTable::Search().

This patch removes const_cast from the former and mark some methods of the
latter const.

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

Comment 4

[(no longer active)]

Comment on attachment 8991871 [details]
Bug 1475461 - part 1: Mark PLDHashTable::Search() and called by it as const

https://reviewboard.mozilla.org/r/256794/#review263740

Wow, this ended up being a lot more invasive than I was hoping for...  Thanks very much for doing this!  :-)

Comment 5

[(no longer active)]

Comment on attachment 8991872 [details]
Bug 1475461 - part 2: Make callers of PLDHashTable::Search() const methods if possible

https://reviewboard.mozilla.org/r/256796/#review263742

Very nice!

::: dom/commandhandler/nsCommandParams.cpp:298
(Diff revision 1)
>  }
>  
>  nsCommandParams::HashEntry*
>  nsCommandParams::GetNamedEntry(const char* aName) const
>  {
> -  return static_cast<HashEntry*>(
> +  return static_cast<HashEntry*>(mValuesHash.Search((void*)aName));

Hmm, it'd be nice if the argument type were const void*...

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 8991872 [details]
Bug 1475461 - part 2: Make callers of PLDHashTable::Search() const methods if possible

https://reviewboard.mozilla.org/r/256796/#review263742

> Hmm, it'd be nice if the argument type were const void*...

Do you think that I should change it here? (You commented this as an open issue...)

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(ping for replying to the review comment)

Comment 8

[(no longer active)]

(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #6)
> Comment on attachment 8991872 [details]
> Bug 1475461 - part 2: Make callers of PLDHashTable::Search() const methods
> if possible
> 
> https://reviewboard.mozilla.org/r/256796/#review263742
> 
> > Hmm, it'd be nice if the argument type were const void*...
> 
> Do you think that I should change it here? (You commented this as an open
> issue...)

Ah no, I was thinking out loud, I don't want to make you do more work here than you already did!  Thanks.  :-)

(Sorry, I keep forgetting that these comments open issues in MozReview, need to get better at noticing that...)

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

No, problem, thank you for your review!

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9f8a3c2f6528
part 1: Mark PLDHashTable::Search() and called by it as const r=Ehsan
https://hg.mozilla.org/integration/autoland/rev/de078de9ee98
part 2: Make callers of PLDHashTable::Search() const methods if possible r=Ehsan

Comment 11

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/9f8a3c2f6528
https://hg.mozilla.org/mozilla-central/rev/de078de9ee98