Implement Hash for raw pointers to unsized types #45483

mbrubeck · 2017-10-23T21:23:57Z

This is useful for some niche cases, like a hash table of slices or trait objects where the key is the raw pointer. Example use case: https://docs.rs/by_address

rust-highfive · 2017-10-23T21:24:07Z

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

cuviper · 2017-10-23T22:42:24Z

src/libcore/hash/mod.rs

+            // We know T is unsized, because Sized types are handled
+            // by the specialized impl below.
+            let (a, b) = unsafe {
+                *(*self as *const T as *const (usize, usize))


Doesn't this end up interpreting T itself as the usize pair? i.e. self is &*const T, then *self as *const T is a redundant cast, then the final cast changes it to a pointer the fat pointer pieces.

I think you want *(self as *const *const T as *const (usize, usize))

Oops, you are right, of course. Fixed this and added a test.

kennytm · 2017-10-24T13:55:42Z

src/libcore/hash/mod.rs

@@ -664,13 +664,39 @@ mod impls {
        }
    }

+    #[unstable(feature = "unsized_pointer_hash", issue = "0")] // TODO


I thought impl are insta-stable?

Also, please change the TODO to FIXME to pass tidy check.

[00:03:26] tidy error: /checkout/src/libcore/hash/mod.rs:667: TODO is deprecated; use FIXME [00:03:26] tidy error: /checkout/src/libcore/hash/mod.rs:687: TODO is deprecated; use FIXME

alexcrichton · 2017-10-25T17:31:56Z

Is this something that's possible to implement without specialization? We've been hesitant to stabilize any API surface area that requires specialization to exist up to this point unfortunately

mbrubeck · 2017-10-25T17:41:11Z

Yes, it could be done in a single ?Sized impl that checks if size_of::<*const T>() == size_of::<usize>().

alexcrichton · 2017-10-25T17:42:55Z

In that case, could this PR be updated to that implementation?

mbrubeck · 2017-10-25T18:08:02Z

Updated to not use specialization.

alexcrichton · 2017-10-25T18:42:10Z

@bors: r+

Awesome, thanks!

cc @rust-lang/libs

I'm going ahead an approving this as this seems like the natural extension of PartialEq to me which compares both pointers. If y'all have any objections though please let me know!

bors · 2017-10-25T18:42:11Z

📌 Commit c2c1910 has been approved by alexcrichton

Implement Hash for raw pointers to unsized types This is useful for some niche cases, like a hash table of slices or trait objects where the key is the raw pointer. Example use case: https://docs.rs/by_address

Rollup of 7 pull requests - Successful merges: #45059, #45212, #45398, #45483, #45496, #45508, #45526 - Failed merges:

dtolnay

Seems good to me. This implementation is not necessarily correct if we ever add extra-fat pointers, but we can sort that out at the time.

This fixes the problem where only the "data" part of a fat pointer could be hashed. Depends on rust-lang/rust#45483. Fixes #2.

Allow ptr::hash to accept fat pointers Fat pointers implement Hash since rust-lang#45483. This is a follow-up to rust-lang#56250.

rust-highfive assigned alexcrichton Oct 23, 2017

cuviper reviewed Oct 23, 2017

View reviewed changes

kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Oct 24, 2017

mbrubeck force-pushed the hash branch 2 times, most recently from 7b24872 to f4b4631 Compare October 24, 2017 03:46

kennytm reviewed Oct 24, 2017

View reviewed changes

mbrubeck force-pushed the hash branch from f4b4631 to b3fe88c Compare October 24, 2017 16:26

kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 24, 2017

kennytm added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 25, 2017

Implement Hash for raw pointers to unsized types

c2c1910

mbrubeck force-pushed the hash branch from b3fe88c to c2c1910 Compare October 25, 2017 18:07

mbrubeck added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 25, 2017

kennytm added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 25, 2017

kennytm mentioned this pull request Oct 25, 2017

Rollup of 7 pull requests #45532

Merged

bors added a commit that referenced this pull request Oct 25, 2017

Auto merge of #45532 - kennytm:rollup, r=kennytm

e847f30

Rollup of 7 pull requests - Successful merges: #45059, #45212, #45398, #45483, #45496, #45508, #45526 - Failed merges:

dtolnay approved these changes Oct 26, 2017

View reviewed changes

bors merged commit c2c1910 into rust-lang:master Oct 26, 2017

mbrubeck mentioned this pull request Oct 26, 2017

Idea for hashing pointers to unsized types mbrubeck/by_address#2

Closed

mbrubeck added a commit to mbrubeck/by_address that referenced this pull request Jan 19, 2018

Hash fat pointers

1ca9e78

This fixes the problem where only the "data" part of a fat pointer could be hashed. Depends on rust-lang/rust#45483. Fixes #2.

mbrubeck mentioned this pull request Mar 26, 2018

Compare only the data part of fat pointers #48814

Closed

mbrubeck mentioned this pull request Dec 12, 2018

Allow ptr::hash to accept fat pointers #56751

Merged

kennytm added a commit to kennytm/rust that referenced this pull request Dec 14, 2018

Rollup merge of rust-lang#56751 - mbrubeck:hash, r=dtolnay

3b0d7da

Allow ptr::hash to accept fat pointers Fat pointers implement Hash since rust-lang#45483. This is a follow-up to rust-lang#56250.

kennytm added a commit to kennytm/rust that referenced this pull request Dec 14, 2018

Rollup merge of rust-lang#56751 - mbrubeck:hash, r=dtolnay

433f083

Allow ptr::hash to accept fat pointers Fat pointers implement Hash since rust-lang#45483. This is a follow-up to rust-lang#56250.

pietroalbini added a commit to pietroalbini/rust that referenced this pull request Dec 14, 2018

Rollup merge of rust-lang#56751 - mbrubeck:hash, r=dtolnay

e9580b1

Allow ptr::hash to accept fat pointers Fat pointers implement Hash since rust-lang#45483. This is a follow-up to rust-lang#56250.

pietroalbini added a commit to pietroalbini/rust that referenced this pull request Dec 15, 2018

Rollup merge of rust-lang#56751 - mbrubeck:hash, r=dtolnay

7c83737

Allow ptr::hash to accept fat pointers Fat pointers implement Hash since rust-lang#45483. This is a follow-up to rust-lang#56250.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Hash for raw pointers to unsized types #45483

Implement Hash for raw pointers to unsized types #45483

mbrubeck commented Oct 23, 2017

rust-highfive commented Oct 23, 2017

cuviper Oct 23, 2017 •

edited

Loading

mbrubeck Oct 24, 2017

kennytm Oct 24, 2017

mbrubeck Oct 24, 2017

alexcrichton commented Oct 25, 2017

mbrubeck commented Oct 25, 2017 •

edited

Loading

alexcrichton commented Oct 25, 2017

mbrubeck commented Oct 25, 2017

alexcrichton commented Oct 25, 2017

bors commented Oct 25, 2017

dtolnay left a comment

Implement Hash for raw pointers to unsized types #45483

Implement Hash for raw pointers to unsized types #45483

Conversation

mbrubeck commented Oct 23, 2017

rust-highfive commented Oct 23, 2017

cuviper Oct 23, 2017 • edited Loading

Choose a reason for hiding this comment

mbrubeck Oct 24, 2017

Choose a reason for hiding this comment

kennytm Oct 24, 2017

Choose a reason for hiding this comment

mbrubeck Oct 24, 2017

Choose a reason for hiding this comment

alexcrichton commented Oct 25, 2017

mbrubeck commented Oct 25, 2017 • edited Loading

alexcrichton commented Oct 25, 2017

mbrubeck commented Oct 25, 2017

alexcrichton commented Oct 25, 2017

bors commented Oct 25, 2017

dtolnay left a comment

Choose a reason for hiding this comment

cuviper Oct 23, 2017 •

edited

Loading

mbrubeck commented Oct 25, 2017 •

edited

Loading