Type-assert opts.delim_flags to reduce invalidations

[JamesWrigley]

In particular the last line of invalidations from PythonCall.jl (CC @cjdoris):

 inserting |(x::Number, y::PythonCall.Py) @ PythonCall.Core ~/git/PythonCall.jl/src/Core/Py.jl:432 invalidated:
   mt_backedges: 1: signature Tuple{typeof(|), Int64, Any} triggered MethodInstance for Base.JuliaSyntax._register_kinds!(::Dict{Int64, Union{Module, Symbol}}, ::Dict{UInt16, String}, ::Dict{String, UInt16}, ::Module, ::Int64, ::Vector{String}) (0 children)
                 2: signature Tuple{typeof(|), Int64, Any} triggered MethodInstance for Base.JuliaSyntax._register_kinds!(::Dict{Int64, Union{Module, Symbol}}, ::Dict{UInt16, String}, ::Dict{String, UInt16}, ::Module, ::Int64, ::Vector{String}) (0 children)
                 3: signature Tuple{typeof(|), Int64, Any} triggered MethodInstance for JuliaSyntax._register_kinds!(::Dict{Int64, Union{Module, Symbol}}, ::Dict{UInt16, String}, ::Dict{String, UInt16}, ::Module, ::Int64, ::Vector{String}) (0 children)
                 4: signature Tuple{typeof(|), Bool, Any} triggered MethodInstance for <=(::Any, ::Char) (0 children)
                 5: signature Tuple{typeof(|), Bool, Any} triggered MethodInstance for <=(::Char, ::Any) (9 children)
                 6: signature Tuple{typeof(|), UInt16, Any} triggered MethodInstance for Base.JuliaSyntax.parse_function_signature(::Base.JuliaSyntax.ParseState, ::Bool) (1155 children)

The issue is that ops.delim_flags was getting inferred to Any:

2220                 ambiguous_parens::Bool = opts::NamedTuple.maybe_grouping_parens::Any &&                                                                                                                                                   
2221                                    (peek_behind(ps::JuliaSyntax.ParseState)::@NamedTuple{kind::JuliaSyntax.Kind, flags::UInt16, orig_kind::JuliaSyntax.Kind, is_leaf::Bool}.kind::JuliaSyntax.Kind in KSet"macrocall $")::Bool            
2222                 emit(ps::JuliaSyntax.ParseState, mark::JuliaSyntax.ParseStreamPosition, K"tuple", (PARENS_FLAG::Core.Const(0x0100)|opts::NamedTuple.delim_flags::Any)::Any)                                                               
2223                 if ambiguous_parens::Bool                                                                                                                                                                                                 

But from reading parse_brackets() I think it's safe to restrict it to a RawFlags

[KristofferC]

Fixing #600 should fix this too.

[JamesWrigley]

Looks like the invalidations are also triggered by InitialValues.jl (in the dependency tree of OhMyThreads.jl):

 inserting |(x, ::Union{InitialValues.NonspecificInitialValue, InitialValues.SpecificInitialValue{typeof(|)}}) @ InitialValues ~/.julia/packages/InitialValues/OWP8V/src/InitialValues.jl:161 invalidated:
   mt_backedges: 1: signature Tuple{typeof(|), Int64, Any} triggered MethodInstance for Base.JuliaSyntax._register_kinds!(::Dict{Int64, Union{Module, Symbol}}, ::Dict{UInt16, String}, ::Dict{String, UInt16}, ::Module, ::Int64, ::Vector{String}) (0 children)
                 2: signature Tuple{typeof(|), Int64, Any} triggered MethodInstance for Base.JuliaSyntax._register_kinds!(::Dict{Int64, Union{Module, Symbol}}, ::Dict{UInt16, String}, ::Dict{String, UInt16}, ::Module, ::Int64, ::Vector{String}) (0 children)
                 3: signature Tuple{typeof(|), Bool, Any} triggered MethodInstance for <=(::Char, ::Any) (0 children)
                 4: signature Tuple{typeof(|), Bool, Any} triggered MethodInstance for <=(::Any, ::Char) (0 children)
                 5: signature Tuple{typeof(|), Missing, Any} triggered MethodInstance for <=(::Any, ::Char) (1 children)
                 6: signature Tuple{typeof(|), Missing, Any} triggered MethodInstance for <=(::Char, ::Any) (10 children)
                 7: signature Tuple{typeof(|), UInt16, Any} triggered MethodInstance for Base.JuliaSyntax.parse_function_signature(::Base.JuliaSyntax.ParseState, ::Bool) (1161 children)

[KristofferC]

Would it make sense to type assert the whole opts? Is it only delim_flags that is non-inferred?

[JamesWrigley]

I had another look at this and I don't think we can realistically type-assert opts since it's a NamedTuple with varying fields depending on the function that's passed to it. The only reliable thing is to specialize on the function argument as suggested in #600, so I did that in 28fa41d. Checked it with Cthulhu and can confirm that everything's concretely typed now.

I realize it's a bit late, but is there any chance of getting this in for 1.12.2? 🫣 If so then I can make any necessary backport PRs.

[KristofferC]

Checked it with Cthulhu and can confirm that everything's concretely typed now.

Can you just confirm that Cthulhu shows the status quo as bad? I have some memory that I tried to look at this with Cthuklhu and it showed that even the current master was ok (which is not true but it still showed that...)

[JamesWrigley]

Hmm this is weird, with Revise loaded the second invocation to descend always shows concretely typed variables, even on main. And without it loaded the variables never show up as concretely typed, even on this branch 😕 I think I'm gonna try building Julia with this branch to a proper check.

[JamesWrigley]

Yeah this isn't doing it for some reason. For some reason Cthulhu also doesn't show an option to descend into parse_brackets from parse_function_signature.

[JamesWrigley]

I tried this commit on the backports-1.12 branch: JamesWrigley@a148565

Tested with InitialValues and DataStructures, and it fixes all the JuliaSyntax invalidations for them. This is what Cthulhu reports (without Revise loaded):

2160             opts::NamedTuple = parse_brackets(ps::JuliaSyntax.ParseState, K")") do had_commas, had_splat, num_semis, num_subexprs                                                                                                         
2161                 _parsed_call = was_eventually_call(ps)                                                                                                                                                                                    
2162                 _needs_parse_call = peek(ps, 2) ∈ KSet"( ."                                                                                                                                                                               
2163                 _is_anon_func = (!_needs_parse_call && !_parsed_call) || had_commas                                                                                                                                                       
2164                 return (needs_parameters = _is_anon_func,                                                                                                                                                                                 
2165                         is_anon_func     = _is_anon_func,                                                                                                                                                                                 
2166                         parsed_call      = _parsed_call,                                                                                                                                                                                  
2167                         needs_parse_call = _needs_parse_call,                                                                                                                                                                             
2168                         maybe_grouping_parens = !had_commas && !had_splat && num_semis == 0 && num_subexprs == 1)                                                                                                                         
2169             end::NamedTuple::Type{JuliaSyntax.var"#parse_function_signature##0#parse_function_signature##1"{JuliaSyntax.ParseState}}                                                                                                      
2170             is_anon_func::Bool = (opts::NamedTuple.is_anon_func::Any::Bool)::Bool                                                                                                                                                         
2171             parsed_call::Bool = (opts::NamedTuple.parsed_call::Any::Bool)::Bool                                                                                                                                                           
2172             needs_parse_call::Bool = (opts::NamedTuple.needs_parse_call::Any::Bool)::Bool                                                                                                                                                 
2173             maybe_grouping_parens::Bool = (opts::NamedTuple.maybe_grouping_parens::Any::Bool)::Bool                                                                                                                                       
2174             delim_flags::UInt16 = (opts::NamedTuple.delim_flags::Any::RawFlags::Type{UInt16})::UInt16                                                                                                                                     
2175             if is_anon_func::Bool                                                                                                                                                                                                         
2176                 # function (x) body end ==>  (function (tuple-p x) (block body))                                                                                                                                                          
2177                 # function (x::f()) end ==>  (function (tuple-p (::-i x (call f))) (block))                                                                                                                                               
2178                 # function (x,y) end    ==>  (function (tuple-p x y) (block))                                                                                                                                                             
2179                 # function (x=1) end    ==>  (function (tuple-p (= x 1)) (block))                                                                                                                                                         
2180                 # function (;x=1) end   ==>  (function (tuple-p (parameters (= x 1))) (block))                                                                                                                                            
2181                 # function (f(x),) end  ==>  (function (tuple-p (call f x)) (block))                                                                                                                                                      
2182                 ambiguous_parens::Bool = maybe_grouping_parens::Bool &&                                                                                                                                                                   
2183                                    (peek_behind(ps::JuliaSyntax.ParseState)::@NamedTuple{kind::JuliaSyntax.Kind, flags::UInt16, orig_kind::JuliaSyntax.Kind, is_leaf::Bool}.kind::JuliaSyntax.Kind in KSet"macrocall $")::Bool            
2184                 emit(ps::JuliaSyntax.ParseState, mark::JuliaSyntax.ParseStreamPosition, K"tuple", (PARENS_FLAG::Core.Const(0x0020)|delim_flags::UInt16)::UInt16)

I fully admit it's a bit ugly... Not sure if it makes sense to merge this PR into main now that JuliaSyntax has been merged into the Julia repo, but if folks think this is acceptable I can make another PR to the upstream version. Is there still time to get it in for Julia 1.12.2?

[JamesWrigley]

Bump, would be great to get this into 1.12.2 if possible.

[c42f]

Heya, we just moved JuliaSyntax into Base in JuliaLang/julia#59870 - if you want to re-propose this PR over there, I've created a branch https://github.com/JuliaLang/JuliaSyntax.jl/tree/pr-for-Base/601 to make this easier.

To make use of the branch, you can use the following steps:

• Clone JuliaLang/julia
• Add JuliaSyntax as a git remote
• Check out pr-for-Base/601
• Rebase on top of master

For example:

# git clone [email protected]:JuliaLang/julia julia_dir
# cd julia_dir

git remote add JuliaSyntax [email protected]:JuliaLang/JuliaSyntax.jl
git fetch JuliaSyntax
git checkout pr-for-Base/601
git rebase origin/master

Sorry this wasn't dealt with prior to the big move!

[JamesWrigley]

I think if JamesWrigley@a148565 is going to be merged then it should be into this repo and then I can make the other PR to Base. But it looks like we'll need a backports-1.12 branch because main and the commit in 1.12 have diverged quite a bit.