Fix invalidations from loading OrderedCollections

[timholy]

Somehow, I think I convinced myself early on that the OrderedCollections invalidations were unfixable. That is most assuredly false, and this PR gets rid of them all. That seems pretty useful, because if memory serves more than half the Julia ecosystem loads OrderedCollections.

The major source of invalidations come from print_status; I don't know why I didn't fix this properly before. (Note that the double type-specification in each comprehension seems necessary, if we want to get rid of all the vulnerabilities, since there's a Union involved---the Tuple{T,S,S}[comp...] ensures we get the right element type and [(x,y,z)::Tuple{T,S,S}... eliminates a call to convert.) Additionally, TOML._print calls itself, with apparently-noninferrable types, so we force runtime dispatch to sever the resulting self-backedges. An alternative fix would be to specify the arg types more precisely than AbstractDict or AbstractArray{<:AbstractDict}, i.e., if

Pkg.jl/ext/TOML/src/print.jl

Line 78 in 20f9b9e

if is_table(value)

means that it's really a Dict{String,Any} and

Pkg.jl/ext/TOML/src/print.jl

Line 93 in 20f9b9e

elseif is_array_of_tables(value)

means it's a Vector{Dict{String,Any}} then it might be better to just enforce that.

[timholy]

Interesting. With invoke I got

julia> t = Template(plugins=[Git(;ssh=true), GitHubActions(), TravisCI(), Codecov(), GitHubPages()]);

julia> t("SomeNewPkg")
[ Info: Running prehooks
[ Info: Running hooks
ERROR: invoke: argument type error
Stacktrace:
 [1] _print(::IOStream, ::Dict{String,Any}, ::Array{String,1}; indent::Int64, first_block::Bool, sorted::Bool, by::Function) at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.6/Pkg/ext/TOML/src/print.jl:91
 [2] _print at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.6/Pkg/ext/TOML/src/print.jl:60 [inlined] (repeats 2 times)
 [3] #print#10 at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.6/Pkg/ext/TOML/src/print.jl:113 [inlined]
 [4] print at /home/tim/src/julia-master/usr/share/julia/stdlib/v1.6/Pkg/ext/TOML/src/print.jl:113 [inlined]
 [5] #44 at /home/tim/.julia/packages/PkgTemplates/pOq4d/src/plugins/project_file.jl:24 [inlined]
 [6] open(::PkgTemplates.var"#44#45"{Dict{String,Any}}, ::String, ::Vararg{String,N} where N; kwargs::Base.Iterators.Pairs{Union{},Union{},Tuple{},NamedTuple{(),Tuple{}}}) at ./io.jl:330
 [7] open(::Function, ::String, ::String) at ./io.jl:328
 [8] hook(::ProjectFile, ::Template, ::String) at /home/tim/.julia/packages/PkgTemplates/pOq4d/src/plugins/project_file.jl:24
 [9] (::PkgTemplates.var"#17#20"{typeof(PkgTemplates.hook),Template,String})(::ProjectFile) at /home/tim/.julia/packages/PkgTemplates/pOq4d/src/template.jl:132
 [10] foreach(::PkgTemplates.var"#17#20"{typeof(PkgTemplates.hook),Template,String}, ::Array{PkgTemplates.Plugin,1}) at ./abstractarray.jl:2018
 [11] (::PkgTemplates.var"#16#19"{Template,String})(::typeof(PkgTemplates.hook)) at /home/tim/.julia/packages/PkgTemplates/pOq4d/src/template.jl:131
 [12] foreach(::PkgTemplates.var"#16#19"{Template,String}, ::Tuple{typeof(PkgTemplates.prehook),typeof(PkgTemplates.hook),typeof(PkgTemplates.posthook)}) at ./abstractarray.jl:2018
 [13] (::Template)(::String) at /home/tim/.julia/packages/PkgTemplates/pOq4d/src/template.jl:129
 [14] top-level scope at REPL[3]:1

but switching it to Base.invokelatest worked fine. I'm not quite sure what's up with this, but perhaps invoke really shouldn't be used with runtime-defined types?

The fact that this passed tests previously suggests there's some missing test coverage.

[c42f]

Hi Tim, I'm very curious now that the experimental JuliaLang/julia#35844 is merged - can it be used as a general tool (wrapped around pkg repl mode entry points, for example) to solve this without needing these kind of very specific-looking workarounds scattered through the code?

The obvious caveat with the frozen-world idea is that by default it would make Pkg act as a non-extensible library with respect to user types and methods, separate from the user's session. Oh, and it's available only on 1.6.

[timholy]

I agree that's worth thinking about, and I'm glad we have that functionality, but the frozen-world issue does seem like a disadvantage.

My view is that for "library-quality" code that has to deal with type-instability (e.g., the compiler, Base.show_unquoted, our core IO infrastructure, etc.), it's not unreasonable that it should be written to a standard that doesn't have lots of unnecessary inference problems. In other words, I've been taking the perspective is that the invalidation hunt is just revealing places where we should have improved the quality of engineering anyway.

Keeping it that way will be a challenge, of course, which is why I'm eager to get people taking JuliaLang/julia#36393 a bit more seriously.

EDIT: or perhaps you were referring to the invokelatest part of this. Yes, that's basically a hack for those cases where inference problems seem unavoidable. (I'm not actually certain that's true here, the dict types might be perfectly predictable.) But we only have a tiny handful of those (here, the couple I added to the logging framework, and some that Jeff added in some of his invalidation-related PRs).

The other thing that the invalidation-hunt seems to reveal is type-piracy (JuliaLang/julia#35922). Since we don't have another systematic tool for detecting piracy, that seems useful too.

[c42f]

the frozen-world issue does seem like a disadvantage

It's a matter of perspective. I think the litmus test is thus: "would you be happy if the pkg> repl mode ran in a separate process from user code?"

If yes, then Pkg is not expecting users to extend it in a meaningful way, and frozen-world makes a lot of sense for this use case. Actually it's an advantage because users can't unintentionally break things.

or perhaps you were referring to the invokelatest part of this

Somewhat, but I also feel the need for "unnecessary-looking" type annotations is a sign of trouble, in that it contributes to two things:

• Fragility of avoiding these coming back in future. You already mentioned this.
• "Rigidity" - people being afraid of changing tricky-looking code because it doesn't directly express the intentions of the original programmer. Luckily git blame comes to the rescue here, to some extent.

Not to say I object to these changes specifically — I trust these help in the short/medium term, and they're minimal enough for sure.

[timholy]

I definitely see the merits of your perspective.

I think the litmus test is thus: "would you be happy if the pkg> repl mode ran in a separate process from user code?"

Easy answer: if I can Revise it, I'm happy---if not, not. I'm actually not entirely certain of whether your recent work affects Revise-ability? Core.Compiler is walled off, yet I can still Revise stuff. As long as you can Revise it, then not being able to inadvertently break it is really cool.

the need for "unnecessary-looking" type annotations

Here it's because of JuliaLang/julia#36454. In other places they might look unnecessary, the reasoning explained in JuliaLang/julia#36470 (comment) is a common source. As we deliberately scale back the thoroughness of our inference this will affect both performance and vulnerability to invalidation.

[c42f]

if I can Revise it, I'm happy---if not, not

You can. (XRef for other people following this conversation: I put a complete description of how this works over at JuliaLang/julia#35844 (comment).)