allow specializing Base.hash for enum types without overwriting method
#49777
Conversation
|
I feel like we should delete this, since hashing is already defined for them via objectid, and it is mostly illogical to use an Enum as a dictionary key, as it makes a better/faster array index. |
In my usecase the enum is used as part of bigger datastructures where the hash is computed recursively. I need to specialize the hash function since the default one (both pre 1.9 and 1.9) is not deterministic across sessions: module M @enum E A end
hash(M.A)will yield different hashes for new sessions. |
|
That is fine. I am suggesting this PR be updated to just delete this code, as it is redundant with the existing definition of |
|
I'm fine with deleting it but maybe @KristofferC who introduced it in #30500 can comment. |
IIRC, the reason it was added was because the
So I rather keep it. |
|
That seems a mistaken reasoning. We can teach codegen about |
|
That's also fine (if it is actually done). Removing it because there is a possibility in doing it another way sounds like a mistaken reasoning to me. |
test/enums.jl
Outdated
| @enum HashEnum3 Enum3_a=1 | ||
| @test only(methods(hash, (HashEnum3, UInt))).sig != Tuple{typeof(hash), HashEnum3, UInt64} |
There was a problem hiding this comment.
@KristofferC can you check that this test matches what you had in mind?
|
Merged in latest master in the hopes the test errors resolve. Can you squash into the commit description you want? Otherwise we can squash+merge from here |
Previously `@enum` defined `Base.hash(::MyEnumType, ::UInt)` on the
user-defined enum type `MyEnumType`.
When the user wants to specialize the hash function for his own enum
type he will define exactly that method signature again which overwrites
it and leads to the warning
WARNING: Method definition hash(TestPackage.MyEnumType, UInt64) in module TestPackage at Enums.jl:210 overwritten at [...]
** incremental compilation may be fatally broken for this module **
This commit changes `@enum` so that an internal method is used instead
which is called through a fallback `Base.hash(::Enum, ::UInt)`.
ok, squashed into one commit |
| _enum_hash(x::Enum, h::UInt) = hash(x, h) | ||
| Base.hash(x::Enum, h::UInt) = _enum_hash(x, h) |
There was a problem hiding this comment.
The fallback for _enum_hash has to be something else, otherwise these two methods just call each other resulting in stack overflows (fredrikekre/EnumX.jl#5).
There was a problem hiding this comment.
Indeed, the assumption was that using @enum would create specialized hash functions to dispatch to.
This should fix it:
| _enum_hash(x::Enum, h::UInt) = hash(x, h) | |
| Base.hash(x::Enum, h::UInt) = _enum_hash(x, h) | |
| _enum_hash(x::Enum, h::UInt) = invoke(hash, Tuple{Any, UInt}, x, h) | |
| Base.hash(x::Enum, h::UInt) = _enum_hash(x, h) |
Could you test and open a pull request?
There was a problem hiding this comment.
I guess I can when I find the time, but I am not the one who caused this so it is more tempting for me to press the revert button.
…hod (#49777) Previously `@enum` defined `Base.hash(::MyEnumType, ::UInt)` on the user-defined enum type `MyEnumType`. When the user wants to specialize the hash function for his own enum type he will define exactly that method signature again which overwrites it and leads to the warning WARNING: Method definition hash(TestPackage.MyEnumType, UInt64) in module TestPackage at Enums.jl:210 overwritten at [...] ** incremental compilation may be fatally broken for this module ** This commit changes `@enum` so that an internal method is used instead which is called through a fallback `Base.hash(::Enum, ::UInt)`. (cherry picked from commit 22551a2)
…hod (#49777) Previously `@enum` defined `Base.hash(::MyEnumType, ::UInt)` on the user-defined enum type `MyEnumType`. When the user wants to specialize the hash function for his own enum type he will define exactly that method signature again which overwrites it and leads to the warning WARNING: Method definition hash(TestPackage.MyEnumType, UInt64) in module TestPackage at Enums.jl:210 overwritten at [...] ** incremental compilation may be fatally broken for this module ** This commit changes `@enum` so that an internal method is used instead which is called through a fallback `Base.hash(::Enum, ::UInt)`. (cherry picked from commit 22551a2)
Duplicates improvement made to Base's Enums: JuliaLang/julia#49777 and JuliaLang/julia#49964
Duplicates improvement made to Base's Enums: JuliaLang/julia#49777 and JuliaLang/julia#49964
Previously
@enumdefinedBase.hash(::MyEnumType, ::UInt)on the user-defined enum typeMyEnumType.When the user wants to specialize the hash function for his own enum type he will define exactly that method signature again which overwrites it and leads to the warning
This commit changes
@enumso that an internal method is used instead which is called through a fallbackBase.hash(::Enum, ::UInt).