Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pointer hashing breaks equality #26038

Closed
chethega opened this issue Feb 13, 2018 · 2 comments
Closed

Pointer hashing breaks equality #26038

chethega opened this issue Feb 13, 2018 · 2 comments
Assignees
Labels
breaking This change will break code bug Indicates an unexpected problem or unintended behavior hashing

Comments

@chethega
Copy link
Contributor

p0=reinterpret(Ptr{Int8},0)
isequal(p0,C_NULL)
#true 
isequal(hash(p0),hash(C_NULL))
#false 

both on 0.62 and current 0.7.

I feel silly about opening so many small hashing-related issues (and apologize for spamming the tracker); if there is a more appropriate meta-issue for collecting these, could anyone tell me and/or move this to there? [this one is actually not so pathological: using pointers as keys in a dict makes sense; but then they should all be turned into void-pointers for type-stability anyway, at least as long as two pointers of different types to the same address are isequal]

@mbauman
Copy link
Member

mbauman commented Feb 13, 2018

julia> @which p0 == C_NULL
==(x::Ptr, y::Ptr) in Base at pointer.jl:150

julia> @which hash(p0, UInt(0))
hash(x, h::UInt64) in Base at hashing.jl:23

This is effective the same as #12198, but it's really good to audit our codebase for those issues. This would make for a really easy and good PR, if you're game.

@Keno Keno added the bug Indicates an unexpected problem or unintended behavior label Feb 13, 2018
@JeffBezanson
Copy link
Member

Hate to say it, but this might be a case where we need == and isequal to diverge. We want p == q to compare only addresses, but dict keys should not silently substitute pointers of different types.

@JeffBezanson JeffBezanson added triage This should be discussed on a triage call breaking This change will break code labels Apr 16, 2018
@JeffBezanson JeffBezanson self-assigned this Apr 19, 2018
@JeffBezanson JeffBezanson removed the triage This should be discussed on a triage call label Apr 19, 2018
mbauman added a commit that referenced this issue Apr 23, 2018
* origin/master: (23 commits)
  fix deprecations of \cdot and \times (#26884)
  Support reshaping custom 0-dimensional arrays (#26870)
  fix some cases of dot syntax lowering (#26878)
  Pkg3: deterministically close the LibGit2 repo in tests (#26883)
  code loading docs: add missing graph edge (#26874)
  add news for #26858 and #26859 [ci skip] (#26869)
  Deprecate using && and || within at-dot expressions (#26792)
  widen `Int8` and `Int16` to `Int` instead of `Int32` (#26859)
  fix #26038, make `isequal` consistent with `hash` for `Ptr` (#26858)
  Deprecate variadic size(A, dim1, dim2, dims...) method (#26862)
  add using Random to example in manual (#26864)
  warn once instead of depwarn since we want to test it
  Revert "reserve syntax that could be used for computed field types (#18466) (#26816)" (#26857)
  Fix compilation on LLVM 6.0
  change promotion behaviour of `cumsum` and `cumsum!` to match `sum`
  [LLVM 6] add patch to diamond if-conversion
  add a precompile command that can be used to precompile all dependencies (#254)
  use registry if no version entry exist in project for developed pacakges
  make Pkg3 work as a drop in for the old CI scripts
  update registries when adding (#253)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This change will break code bug Indicates an unexpected problem or unintended behavior hashing
Projects
None yet
Development

No branches or pull requests

5 participants