-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Presumed dispatch error on tuple types #9536
Comments
It also appears to me that |
I got it to fail again the same way, as long as I ran the whole sequence; just doing I wonder if this could be #8631? |
does my proposed patch help? diff --git a/src/jltypes.c b/src/jltypes.c
index 7d4d53c..50a838e 100644
--- a/src/jltypes.c
+++ b/src/jltypes.c
@@ -751,10 +751,12 @@ static int tuple_to_Type(jl_tuple_t *a, jl_tuple_t **ptemp)
for(i=0; i < alen; i++) {
jl_value_t *el = jl_tupleref(a, i);
if (jl_is_type_type(el)) {
- jl_tupleset(*ptemp, i, jl_tparam0(el));
+ jl_value_t *T = jl_type_intersection(jl_tparam0(el), (jl_value_t*)jl_any_type); // removes Undef
+ jl_tupleset(*ptemp, i, T);
}
else if (i==alen-1 && jl_is_vararg_type(el) && jl_is_type_type(jl_tparam0(el))) {
- jl_tupleset(*ptemp, i, jl_wrap_vararg(jl_tparam0(jl_tparam0(el))));
+ jl_value_t *T = jl_type_intersection(jl_tparam0(jl_tparam0(el)), (jl_value_t*)jl_any_type); // removes Undef
+ jl_tupleset(*ptemp, i, jl_wrap_vararg(T));
}
else {
*ptemp = NULL; |
I wish it did, but unfortunately it didn't seem to. |
@simonster debugged this further (#9565), and a little further work comes up with this quite-minimal test case: $ julia --inline=no
julia> hash((BigFloat, 256, 7))
0x24d1cc479e693f05
julia> hash(v"2.0.0-rc.1")
0x2dc29d0d282a3c23
julia> hash(v"2.0.0-rc.1+bld")
ERROR: BoundsError
in hash at tuple.jl:90
in hash at tuple.jl:88
in hash at ./version.jl:159
in hash at hashing.jl:3
in eval at no file |
Interestingly, I am unable to replicate the bug with myhash(x::Any) = myhash(x, zero(UInt))
# myhash(w::WeakRef, h::UInt) = myhash(w.value, h)
myhash(x::ANY, h::UInt) = myhash(object_id(x), h)
myhash(x::UInt64, h::UInt) = Base.hx(x, float64(x), h)
myhash(x::Int64, h::UInt) = Base.hx(reinterpret(UInt64,abs(x)), float64(x), h)
const tuplehash_seed = UInt === UInt64 ? 0x77cfa1eef01bca90 : 0xf01bca90
myhash(::(), h::UInt) = h + tuplehash_seed
myhash(x::(Any,), h::UInt) = myhash(x[1], myhash((), h))
myhash(x::(Any,Any), h::UInt) = myhash(x[1], myhash(x[2], myhash((), h)))
myhash(x::Tuple, h::UInt) = myhash(x[1], myhash(x[2], myhash(Base.tail(x), h)))
function myhash(v::VersionNumber, h::UInt)
h += 0x8ff4ffdb75f9fede % UInt
h = myhash(v.major, h)
h = myhash(v.minor, h)
h = myhash(v.patch, h)
h = myhash(v.prerelease, ~h)
h = myhash(v.build, ~h)
end and replacing those |
OK, with this diff: diff --git a/src/codegen.cpp b/src/codegen.cpp
index a9f2e38..e518df6 100644
--- a/src/codegen.cpp
+++ b/src/codegen.cpp
@@ -3511,9 +3511,18 @@ static Function *gen_jlcall_wrapper(jl_lambda_info_t *lam, jl_expr_t *ast, Funct
return w;
}
+extern "C" void jl_(void *jl_value);
+
// cstyle = compile with c-callable signature, not jlcall
static Function *emit_function(jl_lambda_info_t *lam, bool cstyle)
{
+ if (strncmp(lam->name->name, "hash", 4) == 0 ||
+ strncmp(lam->name->name, "__hash", 6) == 0 ||
+ strncmp(lam->name->name, "myhash", 4) == 0 ||
+ strncmp(lam->name->name, "__myhash", 6) == 0) {
+ jl_(lam->name);
+ jl_(lam->specTypes);
+ }
// step 1. unpack AST and allocate codegen context for this function
jl_expr_t *ast = (jl_expr_t*)lam->ast;
jl_tuple_t *sparams = NULL; and starting like this: tim@diva:~/src/julia$ rm usr/lib/julia/sys.so
tim@diva:~/src/julia$ julia --inline=no I get the following output:
So then I created this test file: include("hashbug2.jl") #this is the script in the previous comment
z = zero(UInt)
r = RemoteRef()
println("RemoteRef")
myhash(r, z)
println("2-tuple")
myhash(("hello",5), z)
println("char")
myhash('c')
# Missing the REPLHistoryProvider stuff
println("UInt64")
myhash(z, z)
println("\n\nHere's the test that should give an error:\n")
myhash((BigFloat, 256, 7))
myhash(v"2.0.0-rc.1")
myhash(v"2.0.0-rc.1+bld") and got this:
(All this was using deleted To me, the most conspicuous difference is what I marked with "label" above; for some reason |
Here's something else I'm not sure I understand: from either
which is different. How do I force |
As suggested by @timholy in #9722, the following appears to be a more direct way to cause this bug:
|
Inlining looks like a red-herring here. It is triggering a deeper issue. When inlining is on, we get constant propagation and end up with:
When inlining is off we get:
Obviously this is a valid call and should still work. When I look at a backtrace from
We should be calling the more specific version on line 42:
The issue here appears to boil down to a mismatch in the signature the compiler chooses in
Running
The second signature Here is a speculative patch which fixes this particular issue but breaks several tests:
(this seems very similar to #8631) |
This looks like progress! Does your patch also fix #8631, even though it breaks other things? CCing @vtjnash (compare to your fix?) and @JeffBezanson. These bugs are pretty scary, and it would be lovely to squash them. |
It's likely not a regression, just hard to trigger on 0.3. (#8631 is "convertalypse," which annoyed many and was also the most infamous of Dan Luu's gripes about bugs in julia; Color.jl was triggering that bug for many users on julia 0.3, and only stopped being visible when a few bandaids got applied to Color.jl.) If there's any fix prior to #8974 (which I imagine is not imminent), it would be great to have. |
I tried some of Jake's examples in #8631 and the patch does not seem to help -- but it is really just a (very specific) band-aid. Questions I don't know the answer to:
Also, I just re-read this thread and found your comment here to be interesting: I had actually tried to do the same thing with a minimal set of |
I believe all of the test cases here are fixed (presumably by 4e9bf7a). |
I have no idea how reproducible this is, but I'm filing it while I have all the output.
I was testing #9493 and starting julia as
JULIA_CPU_CORES=1 julia --code-coverage=all --inline=no
, after deletingsys.so
, and theninclude("runtests.jl")
in thetest/
folder. Tests completed successfully up toresolve.jl
, which gave the following error:Here are the key lines. I infer that it must have called line 90 when the tuple did not have sufficient length. (Looking forward to #9534!)
The text was updated successfully, but these errors were encountered: