Skip to content

Commit

Permalink
types: fix hash values of Vararg (#50932)
Browse files Browse the repository at this point in the history
Fixes #50455
  • Loading branch information
vtjnash authored Aug 18, 2023
1 parent 762801c commit c239e99
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 7 deletions.
15 changes: 8 additions & 7 deletions src/jltypes.c
Original file line number Diff line number Diff line change
Expand Up @@ -1597,19 +1597,20 @@ static unsigned typekey_hash(jl_typename_t *tn, jl_value_t **key, size_t n, int
int failed = nofail;
for (j = 0; j < n; j++) {
jl_value_t *p = key[j];
size_t repeats = 1;
if (jl_is_vararg(p)) {
jl_vararg_t *vm = (jl_vararg_t*)p;
if (!nofail && vm->N)
return 0;
// 0x064eeaab is just a randomly chosen constant
hash = bitmix(vm->N ? type_hash(vm->N, &failed) : 0x064eeaab, hash);
if (failed && !nofail)
return 0;
if (vm->N && jl_is_long(vm->N))
repeats = jl_unbox_long(vm->N);
else
hash = bitmix(0x064eeaab, hash); // 0x064eeaab is just a randomly chosen constant
p = vm->T ? vm->T : (jl_value_t*)jl_any_type;
}
hash = bitmix(type_hash(p, &failed), hash);
unsigned hashp = type_hash(p, &failed);
if (failed && !nofail)
return 0;
while (repeats--)
hash = bitmix(hashp, hash);
}
hash = bitmix(~tn->hash, hash);
return hash ? hash : 1;
Expand Down
5 changes: 5 additions & 0 deletions test/tuple.jl
Original file line number Diff line number Diff line change
Expand Up @@ -796,3 +796,8 @@ namedtup = (;a=1, b=2, c=3)
@test_throws ErrorException("Tuple field type cannot be Union{}") Tuple{Vararg{Union{},1}}
@test Tuple{} <: Tuple{Vararg{Union{},N}} where N
@test !(Tuple{} >: Tuple{Vararg{Union{},N}} where N)

@test Val{Tuple{T,T,T} where T} === Val{Tuple{Vararg{T,3}} where T}
@test Val{Tuple{Vararg{T,4}} where T} === Val{Tuple{T,T,T,T} where T}
@test Val{Tuple{Int64, Vararg{Int32,N}} where N} === Val{Tuple{Int64, Vararg{Int32}}}
@test Val{Tuple{Int32, Vararg{Int64}}} === Val{Tuple{Int32, Vararg{Int64,N}} where N}

3 comments on commit c239e99

@vtjnash
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

@vtjnash
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A cumsum improvement seems unexpected, but good

Please sign in to comment.