Add some comments to base.

Add a comment about templates for hashing pointers and correct the case of a function inside a comment.

TEST=none
BUG=none


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=25300

[Mark Mentovai, 2009-09-02 14:24:15]

This comes up periodically.  For example,

http://groups.google.com/group/chromium-reviews/browse_thread/thread/65031198...

It's not necessarily a great idea.  It's not always right or what is intended,
and having the hash available to all pointers can create difficult-to-find
problems.

Is it that onerous to decide how to hash for each class that winds up needing
it?

[jorlow, 2009-09-02 16:00:14]

Ok, so should I subclass this just for the one I care about?  And put it in
it's .h file rather than the hash_tables.h one?

On Wed, Sep 2, 2009 at 11:24 PM, <mark@chromium.org> wrote:

> This comes up periodically.  For example,
>
>
>
http://groups.google.com/group/chromium-reviews/browse_thread/thread/65031198...
>
> It's not necessarily a great idea.  It's not always right or what is
> intended, and having the hash available to all pointers can create
> difficult-to-find problems.
>
> Is it that onerous to decide how to hash for each class that winds up
> needing it?
>
>
> http://codereview.chromium.org/190003
>

[jorlow, 2009-09-03 01:11:32]

Uploaded a new version of the patch.  GIven that people keep trying to add this
functionality, I think it's best that we add a comment.

[Mark Mentovai, 2009-09-03 01:21:02]

LGTM

Please adjust the changelist description to match what you're doing.

I don't see any hash specific to any pointer type in this CL, did you add what
you needed separately?

http://codereview.chromium.org/190003/diff/4001/4003
File base/hash_tables.h (right):

http://codereview.chromium.org/190003/diff/4001/4003#newcode17
Line 17: // header file) and keep it specific to just pointers to that class.
Maybe also add some text to explain that we're doing this because identity
hashes aren't necessarily desirable for all types that might show up in
containers as pointers.

[jorlow, 2009-09-03 01:26:04]

On Thu, Sep 3, 2009 at 10:21 AM, <mark@chromium.org> wrote:

> LGTM
>
> Please adjust the changelist description to match what you're doing.
>
> I don't see any hash specific to any pointer type in this CL, did you
> add what you needed separately?
>

This was a pre-req for a change I was working on, so I'll just put the
specialization in that CL rather than this one.  Sound good?


> http://codereview.chromium.org/190003/diff/4001/4003
> File base/hash_tables.h (right):
>
> http://codereview.chromium.org/190003/diff/4001/4003#newcode17
> Line 17: // header file) and keep it specific to just pointers to that
> class.
> Maybe also add some text to explain that we're doing this because
> identity hashes aren't necessarily desirable for all types that might
> show up in containers as pointers.


Done.  (I actually just ended up using your words.  :-)

[Mark Mentovai, 2009-09-03 02:02:35]

LGTM

Touching scoped_ptr - prepare to rebuild the world.  :)

[jorlow, 2009-09-03 02:12:41]

I wonder how many tons of carbon that one change will waste.  :-)

On Thu, Sep 3, 2009 at 11:02 AM, <mark@chromium.org> wrote:

> LGTM
>
> Touching scoped_ptr - prepare to rebuild the world.  :)
>
> http://codereview.chromium.org/190003
>

[darin (slow to review), 2009-09-03 05:23:11]

LGTM2

http://codereview.chromium.org/190003/diff/5003/6002
File base/hash_tables.h (right):

http://codereview.chromium.org/190003/diff/5003/6002#newcode15
Line 15: // function for pointers.  If you to hash a pointers to a particular
class,
nit: "If you _want_ to hash pointers..."