This should be renamed to just hash(hasher) and let method overloading do the work. The precedent is in how to_s and to_json work.

@RX14 , how could I distinguish def hash(hasher) from def hash with responds_to?(:hash) ?

@funny-falcon just implement def hash(hasher) in base classes to not use responds_to?.

Remove this line, it's redundant because of the method definition below.

I think this constant deserves a comment.

It is prime number.

These first two conditions are weird to me, can you explain what's going on?

Comment to Object#hash:

This method must have the property that a == b implies a.hash == b.hash

$ bin/crystal eval 'require "big"; b=BigInt.new(-1000); p b.to_i64, b.to_u64'
Using compiled compiler at `.build/crystal'
-1000
1000

So I can't use just to_u64. Nor I can just use to_i64 cause of numbers between INT64MAX and UINT64_MAX. Although, probably I can use self & 0xffffffffffffffff_u64, but it will be tweaky.

You might be able to work around this by adjusting the require order in the prelude.

Sorry I'm not too convenient in Crystal.
Issues are cause of:

• enum Signal used in src/event/signal_handler.cr
• enum LLVM::Attribute defined in src/llvm/events/cr (don't know, where it is used)
• class Thread is also used very early in src/thread.cr : @@threads = Set(Thread).new

If you point me how to reorder prelude to fill StdHasher @@seed before this hashes constructed, it will be great.

It could be fixed also with defining special-cased Hash, if def hashkey(key); key.hash; end will be defined, and then overloaded in subclass specially for this use cases.

there is src/prelude.cr, but I doesn't check for.

This also has no comment or explanation

it just "take four chars in a row". I could use counter here. Just was lasy to introduce another one variable.

Write a comment, please :)

What?

Just sanity check remained from debugging. I'll remove it.

Why not just make this part of the @@seed initializer and avoid having an init method? Do you see any other init methods called from the prelude?

It works with just filling in StdHasher body. init could be removed.

Honestly what's the point of this it just seems to me you've replicated a class exactly by using 2 structs...

Not!!!! See StdHasher.build method: it can allocate Impl on a stack. There is no way to allocate class on a stack. Compiler is NOT smart enough!!!

If you want, I may remove StdHasher#initalize() at all, so there will be no malloc in this file.
I don't use it. I put it just for completeness.

See #1517 : @asterite thought compiler could be smart enough. But compiler is not smart enough yet.

You don't need pointers (nor allocate HEAP memory). As long as you access @h (not through an accessor) you can mutate @h properties directly:

struct Hasher
  struct Impl
    property v : Int32
    
    def initialize(@v)
    end
  end
  
  def initialize(seed)
    @h = Impl.new(seed)
  end
  
  def <<(v)
    @h.v += v
  end
  
  def digest
    @h.v
  end
end

hasher = Hasher.new(0)
hasher << 1
hasher << 4
p hasher.digest  # => 5

See https://play.crystal-lang.org/#/r/299v

Initializers could then be reduced to:

def self.new
  new(@@seed[0], @@seed[1])
end

protected def initialize(a, b)
  @h = Impl.new(a, b)
end

Method chaining is nice sugar but not really necessary. Your suggestions for alternate names would all be without chaining as well. I don't see that << would imply chainability.

Anyway class/struct creators will have knowledge to return Hasher in hash, for example.

Moreover it's just hasher << whaaaaat pattern.

And compiler will help with type stricture.

btw, @RX14 already said about def_hash, looks like 80+% authors just will use this macro.

@akzhan, cause it is easy to break with chaining:

# good
def hash(hasher)
  hasher << @a << @b
end
# wrong, but compiles without error
def hash(hasher)
  hasher << @a << @b
  hasher << @c
end
# corrected
def hash(hasher)
  hasher = hasher << @a << @b
  hasher << @c
end

So, resolution:

• default hasher is a struct without pointers (for safety),
• it causes that def hasher have to return hasher,
• << is used, but without chaining (to reduce usage errors),
  Is it correct?

Ok

Please use crystal tool format

ok

btw, use bin/crystal tool format, it's changed in master branch.

hasher should better be spelled out.

Ok (and for othet same notes).

hasher should better be spelled out.

hasher should better be spelled out.

hasher should better be spelled out.

hasher should better be spelled out.

hasher should better be spelled out.

hasher should better be spelled out.

Could this perhaps be generalized? Like if a type has method object_id, use this for calculating the hash, whether it is a reference or not.

This is bad idea, but I know, that current implementation is also weird.

Will be replaced with #4581 I hope.

It is neccessary cause of definition of hash function: "if a==b then hash(a)==hash(b)".

Without it Hash(Int32|Float32, V) is broken.

Does #4581 fix hashing of Float32|Float64 ? If so, then it could be better to apply.
Do you want I include it to my patch, or they will be applied in some order?

@funny-falcon Float hashes in pyhash manner are identical to integer values if no fraction part exists. Feel free to include this, of course.

p [1, 2, 3, 4, 1.0, 2.0_f64, 3.0_f32, 4.0].map &.hash
[1, 2, 3, 4, 1, 2, 3, 4]

p [1, 2, 3, 4, 1.0, 2.0_f64, 3.0_f32, 4.1].map &.hash
[1, 2, 3, 4, 1, 2, 3, 1932733648]

Oh no, there is one exception - -1 value. Will fix now. There is one line fix.

I mean more 4.25_f64.hash == 4.25_f32 ? If not, then my variant is simpler. If yes, then it is better to use more correct variant.

(note: I believe 4.1_f64 != 4.1_f32 in any case, but I could be mistaken).

Great! Ok, I will include that function in next version.

To be strict:

puts "%.20f" % 4.1
puts "%.20f" % 4.1_f32

4.09999999999999964473
4.09999990463256835938

we need no shortcuts. digest sounds better than dgst.

why it is removed?

Beause Struct already provides correct hash(hasher).

In which case could Int not be a primitive?

BigInt.

SecureRandom is stdlib, not corelib, it shouldn't be injected into prelude (indireclty).

But without "secure_random" no safety for hash table.

What do you suggest?

Use Crystal::System::Random directly.

@ysbaddaden, big thanks!!!! I'll do.

btw, Crystal@master has new value.unsafe_as(UInt32) method. AFAIR it's faster for these cases.

Is it? Here is integer to integer conversion. Isn't it compiler primitive already?

AFAIK, No.

Crystal compiler is lower than 1.0, so lot of optimizations doesn't exist yet.

You have i64.i32 conversion here.

I'm wrong :) Just found these primitives :)

Looks like this code isn't compatible with #4581. There is 31/62bit divider.

I have commented Hashing::Type but you may introduce it as alias StdHasher::Type = Int32.

So please update it, you need these lines for 64bit integers.

  private HASH_BITS     =     31 # sizeof(StdHasher::Type) >= 8 ? 61 : 31

    exp = exp >= 0 ? exp % HASH_BITS : HASH_BITS - 1 - ((-1 - exp) % HASH_BITS)
    x = ((x << exp) & HASH_MODULUS) | x >> (HASH_BITS - exp)