-
-
Couldn't load subscription status.
- Fork 5.7k
fix invalidations for Dicts from Static.jl #46490
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
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))::Boolautomatically 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")
endand 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 anifstament - andifstatements only acceptBools -it appears to be sane to assert that
isequalreturns aBoolhere.See also #46481