-
-
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
fix invalidations for Dicts from Static.jl #46490
Conversation
Rather than going 1 by 1 and type annotating every |
That would be nice, of course. However, that would basically mean annotating if (!isequal(key, key0))::Bool automatically but we need if !(isequal(key, key0)::Bool) in this case to fix the invalidations (stemming from a new definition of |
Also, I'd be very weary of "perfect is the enemy of good" here. We can come up with some super elegant solution that fixes all problems and is released in Julia v1.11, but really though, this is a nice small fix that will improve a lot of things so even if there's some big grand plans I'd want this kind of thing in 1.8.1 ASAP as it has major real-world user effects. |
Don't we already assume that anything that goes to an |
I do not know. Even if we do, that's not the problem this PR is intended to fix - the input of |
Test failure https://buildkite.com/julialang/julia-master/builds/15063#0182d9a2-cb0e-4c97-95ec-f16ab36b312c looks like memory corruption; I ran julia> badm = Ref{Any}()
Base.RefValue{Any}(#undef)
julia> for _ in 1:10^6
A = 10*I + sprandn(10, 10, 0.4)
badm[] = copy(A)
F1 = lu(A)
any(isnan, F1.nzval) && error("NaN")
end and never got an instance of NaN. |
Thanks a lot! Can we get a |
Thanks! |
(cherry picked from commit 5c5af1f)
It is doable by moving the
though has the advantage that parenthesis are never needed for it. |
This should hopefully fix quite some invalidations coming from Static.jl.
Here is the code:
Since
!isequal(...)
is used here in anif
stament - andif
statements only acceptBool
s -it appears to be sane to assert that
isequal
returns aBool
here.See also #46481