Skip to content
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

adjust to recent binding partition changes #61

Merged
merged 1 commit into from
Feb 3, 2025
Merged

Conversation

aviatesk
Copy link
Member

@aviatesk aviatesk commented Feb 3, 2025

No description provided.

@aviatesk aviatesk requested a review from Keno February 3, 2025 05:23
Comment on lines +146 to +152
@assert @invokelatest isconst(M, S)
mt = @invokelatest getglobal(M, S)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even with these @invokelatests I'm still seeing the warning:

julia> using CassetteOverlay, Test

julia> import Base: cos

julia> @MethodTable SinTable
# 0 methods for callable object

julia> mutable struct CosCounter <: AbstractBindingOverlay{@__MODULE__, :SinTable}
           ncos::Int
       end

julia> @overlay SinTable function Base.cos(x::Union{Float32,Float64})
           getpass().ncos += 1
           return @nonoverlay cos(x)
       end

julia> pass! = CosCounter(0)
CosCounter(0)

julia> @code_lowered pass!((Int,)) do x
           sin(x) * cos(x)
       end
WARNING: Detected access to binding `Main.SinTable` in a world prior to its definition world.
  Julia 1.12 has introduced more strict world age semantics for global bindings.
  !!! This code may malfunction under Revise.
  !!! This code will error in future versions of Julia.
Hint: Add an appropriate `invokelatest` around the access to this binding.
CodeInfo(
    @ REPL[7]:2 within `#15`
1 ─        builtin (getfield)(fargs, 1)
│   %2 =   builtin (getfield)(fargs, 2)
│   %3 = Main.:*%4 = Main.sin
│   %5 =   dynamic (pass)(%4, %2)
│   %6 = Main.cos
│   %7 =   dynamic (pass)(%6, %2)
│   %8 =   dynamic (pass)(%3, %5, %7)
└──      return %8
)

It is difficult to determine where the access is occurring, so adding information like a line number to this warning would likely be helpful. Would it be possible to do that?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was the suggestion to add a CLI flag to turn the warning into an error, which is possible.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I get:

julia> pass!(42) do x
           sin(x) * cos(x)
       end
ERROR: Internal error happened in `CassetteOverlayGenerator`:
 ┌────────────────────────────────────────────────
 │ WARNING: Detected access to binding `Main.SinTable` in a world prior to its definition world.
 │   Julia 1.12 has introduced more strict world age semantics for global bindings.
 │   !!! This code may malfunction under Revise.
 │   !!! This code will error in future versions of Julia.
 │ Hint: Add an appropriate `invokelatest` around the access to this binding.
 │ 
 │ Stacktrace:
 │   [1] apply_gr(::GlobalRef, ::Any, ::Vararg{Any})
 │     @ Base ./reflection.jl:1365
 │   [2] #invokelatest_gr#227
 │     @ ./reflection.jl:1372 [inlined]
 │   [3] invokelatest_gr
 │     @ ./reflection.jl:1368 [inlined]
 │   [4] methodtable(::Type{CosCounter})
 │     @ CassetteOverlay ~/julia/packages/CassetteOverlay/src/CassetteOverlay.jl:151
 │   [5] generate_overlay_src(world::UInt64, ::Method, passtype::Any, fargtypes::Any, selfname::Symbol, fargsname::Symbol)
 │     @ CassetteOverlay ~/julia/packages/CassetteOverlay/src/CassetteOverlay.jl:55
 │   [6] (::CassetteOverlay.CassetteOverlayGenerator)(world::UInt64, source::Method, passtype::Any, fargtypes::Any)
 │     @ CassetteOverlay ~/julia/packages/CassetteOverlay/src/CassetteOverlay.jl:41
 │   [7] call_get_staged
 │     @ ./../usr/share/julia/Compiler/src/utilities.jl:98 [inlined]
 │   [8] get_staged(mi::Core.MethodInstance, world::UInt64)
 │     @ Compiler ./../usr/share/julia/Compiler/src/utilities.jl:88
 │   [9] retrieve_code_info
 │     @ ./../usr/share/julia/Compiler/src/utilities.jl:121 [inlined]
 │  [10] Compiler.InferenceState(result::Compiler.InferenceResult, cache_mode::UInt8, interp::Compiler.NativeInterpreter)
 │     @ Compiler ./../usr/share/julia/Compiler/src/inferencestate.jl:595
 │  [11] InferenceState
 │     @ ./../usr/share/julia/Compiler/src/inferencestate.jl:600 [inlined]
 │  [12] typeinf_ext(interp::Compiler.NativeInterpreter, mi::Core.MethodInstance, source_mode::UInt8)
 │     @ Compiler ./../usr/share/julia/Compiler/src/typeinfer.jl:1148
 │  [13] typeinf_ext_toplevel(mi::Core.MethodInstance, world::UInt64, source_mode::UInt8)
 │     @ Compiler ./../usr/share/julia/Compiler/src/typeinfer.jl:1228
 │  [14] top-level scope
 │     @ REPL[8]:1
 │  [15] __repl_entry_eval_expanded_with_loc(mod::Module, ast::Any, toplevel_file::Ref{Ptr{UInt8}}, toplevel_line::Ref{Int32})
 │     @ REPL ~/julia/julia/usr/share/julia/stdlib/v1.12/REPL/src/REPL.jl:341
 │  [16] #invokelatest#1
 │     @ ./essentials.jl:1057 [inlined]
 │  [17] invokelatest
 │     @ ./essentials.jl:1053 [inlined]
 │  [18] toplevel_eval_with_hooks(mod::Module, ast::Any, toplevel_file::Any, toplevel_line::Any)
 │     @ REPL ~/julia/julia/usr/share/julia/stdlib/v1.12/REPL/src/REPL.jl:348
 │  [19] toplevel_eval_with_hooks(mod::Module, ast::Any, toplevel_file::Any, toplevel_line::Any) (repeats 3 times)
 │     @ REPL ~/julia/julia/usr/share/julia/stdlib/v1.12/REPL/src/REPL.jl:352
 │  [20] toplevel_eval_with_hooks
 │     @ ~/julia/julia/usr/share/julia/stdlib/v1.12/REPL/src/REPL.jl:345 [inlined]
 │  [21] eval_user_input(ast::Any, backend::REPL.REPLBackend, mod::Module)
 │     @ REPL ~/julia/julia/usr/share/julia/stdlib/v1.12/REPL/src/REPL.jl:370
 │  [22] repl_backend_loop(backend::REPL.REPLBackend, get_module::Function)
 │     @ REPL ~/julia/julia/usr/share/julia/stdlib/v1.12/REPL/src/REPL.jl:482
 │  [23] start_repl_backend(backend::REPL.REPLBackend, consumer::Any; get_module::Function)
 │     @ REPL ~/julia/julia/usr/share/julia/stdlib/v1.12/REPL/src/REPL.jl:467
 │  [24] start_repl_backend
 │     @ ~/julia/julia/usr/share/julia/stdlib/v1.12/REPL/src/REPL.jl:464 [inlined]
 │  [25] run_repl(repl::REPL.AbstractREPL, consumer::Any; backend_on_current_task::Bool, backend::Any)
 │     @ REPL ~/julia/julia/usr/share/julia/stdlib/v1.12/REPL/src/REPL.jl:690
 │  [26] run_repl(repl::REPL.AbstractREPL, consumer::Any)
 │     @ REPL ~/julia/julia/usr/share/julia/stdlib/v1.12/REPL/src/REPL.jl:676
 │  [27] run_std_repl(REPL::Module, quiet::Bool, banner::Symbol, history_file::Bool)
 │     @ Base ./client.jl:489
 │  [28] #invokelatest#1
 │     @ ./essentials.jl:1057 [inlined]
 │  [29] invokelatest
 │     @ ./essentials.jl:1053 [inlined]
 │  [30] run_main_repl(interactive::Bool, quiet::Bool, banner::Symbol, history_file::Bool)
 │     @ Base ./client.jl:510
 │  [31] repl_main
 │     @ ./client.jl:593 [inlined]
 │  [32] _start()
 │     @ Base ./client.jl:568
 └────────────────────────────────────────────────
Stacktrace:
 [1] macro expansion
   @ ~/julia/packages/CassetteOverlay/src/CassetteOverlay.jl:-1 [inlined]
 [2] (::CosCounter)(::var"#18#19", ::Int64)
   @ CassetteOverlay ~/julia/packages/CassetteOverlay/src/CassetteOverlay.jl:116
 [3] top-level scope
   @ REPL[8]:1

, from which I guess the implementation of apply_gr seems to be wrong?

apply_gr(gr::GlobalRef, @nospecialize args...) = getglobal(gr.mod, gr.name)(args...)
apply_gr_kw(@nospecialize(kwargs::NamedTuple), gr::GlobalRef, @nospecialize args...) = Core.kwcall(kwargs, getglobal(gr.mod, gr.name), args...)

? I think getglobal(gr.mod, gr.name) need to be wrapped with invokelatest there?

@aviatesk aviatesk merged commit bb7023b into master Feb 3, 2025
16 of 17 checks passed
@aviatesk aviatesk deleted the avi/adjust-binding branch February 3, 2025 06:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants