Proposal: Removing requirement for call-site annotations

[omus]

The most common complaint about Mocking.jl has been that it requires modification of the source code in order to apply patches. As of Julia 1.12 additions such as Base.delete_method provide us with new options to apply function patches in new ways. This proposal documents an approach inspired by @tz-lom's work on https://github.com/tz-lom/Mock.jl for an approach to mocking without using call-site annotations.

Apply patches via eval

We can use eval to add/override a function method. By using eval we can inject a user defined patch into a function's method table. Adding a method in this way will cause calling functions to be invalidated and call our patch. Restoring the original function method can be done by simply using Base.delete_method to remove the patch function.

Unfortunately, applying patches this way is not thread safe which can lead to some unexpected behavior when a patch is called outside of an apply block. To address this problem we can utilize a runtime check within the patched method which checks for a ScopedValue and conditionally calls the patched method when within the scope of an apply block otherwise they will call the original method.

Calling the original method

With the removal of the call-site macro @mock we need a new approach for calling the original method from within the patch itself. To do this we can create the macro @original which utilize Base.invoke_in_world which would allow us to call the original method while a patch as been applied.

One tricky part here is that since patches can be reused in different apply blocks we probably want to utilize the world age number from the start of the apply block rather than the world age from when the patch was created. The advantage of this is that interative workflows using both Mocking.jl and Revise.jl should work better. If we took the world age from point at which the patch was created then user function changes updated via Revise would not be picked up when calling @original from within a patch in an apply block.

Backwards compatibility with Base.delete_method

For removing a patch we can utilize Base.delete_method to remove the added method. As this is only available in Julia 1.12 we can need a solution which also works at least with the Julia LTS (1.10). Once again we can utilize Base.invoke_in_world to restore calling the original method:

f(x) = Base.invoke_in_world($prev_world_age, f, x)

We would generate these methods for each patch we applied. These methods would result in us calling the function using the world age from just before the apply block such that nested apply blocks should work correctly. As we leave each apply block we'll be defining a new "restore" method which will be hardcoded with the earlier world age.

When considering this approach I became a bit concerned about performance. Would it be possible multiple apply/restore iterations result in "restore" methods calling "restore" methods such that it would have a performance impact? I don't think think we'll have a problem here since as we leave each apply block the "restore" method would be calling an earlier world age such that these methods would typically call the original method directly. In scenarios where nested apply blocks have more and more specific patches then we could end up having a "restore" method calling another "restore" method with a more generic signature. However, this scenario seems rare enough that it won't be an issue.

The primary downside of this backwards compatible approach is that we end up polluting the method table with additional methods if a patch uses a different argument types than what was defined in the original function. This isn't a big problem as Mocking.jl is expected to be use primarily for testing purposes. There could be some packages which are testing results from methods which could see test failures. In those scenarios the package maintainers would just need to be careful to only create patches which match the signatures of the original methods.

Backwards compatibility with Base.invoke_in_world

We plan on using Base.invoke_in_world in the @original macro as well as for some backwards compatibility when Base.delete_method isn't available. This function has been available since Julia 1.6 which allows us to at least support the Julia LTS (1.10).

Backwards compatibility with ScopedValue

We are already using ScopedValues in Mocking.jl but it's still worth mentioning this requirement here as this feature requires Julia 1.11 and we now rely on it in different way.

Previously, when we didn't have ScopedValue support we ended up falling back on a global value that wasn't thread-safe. This global was used by the @mock at the call site to determine whether to call the patch or the original function. With the new approach we have the same thread-safety limitations.

Repercussions

In looking into applying this proposal I suspect the following will also occur as part of these changes:

• Removal of @mock: If there isn't any performance downside to this approach then we can remove this
• Removal of Mocking.activate: Was used to trigger invalidation at @mock call-sites
• Removal of Mocking.nullify: Only was needed for @mock call-sites
• Removal of Mocking.dispatch: Used for performing dispatch as if two functions had the same method table. Shouldn't be necessary as we can no just use Julia's method dispatch with world age.
• Nesting patch environments will no longer require them to be merged: We should be able to extend a function and rely on world age to temporarily apply patches
• @patch will need to construct the function which will be added (eval'd) later: The user will define there patch like they did before but we'll also create an function definition expression which perform the runtime check which determines if the patch or the original function is called. This function cannot use args... since this would impact method dispatch so we'll need to extract the actual args names when we call the original/patch functions. Probably want to add additional tests for this as I can foresee scenarios we pass in too many args into the original function.

[tz-lom]

Thanks for documenting the proposal in such nice way.

To do this we can create the macro @original which utilize Base.invoke_in_world which would allow us to call the original method while a patch as been applied.

That is indeed a right approach, the one that I've used is not working correctly in certain scenarios.

The primary downside of this backwards compatible approach is that we end up polluting the method table with additional methods if a patch uses a different argument types than what was defined in the original function. This isn't a big problem as Mocking.jl is expected to be use primarily for testing purposes. There could be some packages which are testing results from methods which could see test failures. In those scenarios the package maintainers would just need to be careful to only create patches which match the signatures of the original methods.

You can see in my prototype that I've made an attempt to ensure that signatures do match.
This attempt is not complete as it do not check for kwargs at this moment, but I am looking deeper how to resolve it, that should not be too problematic. The tricky part would be presence of default arguments, at this moment default argument emit 2 method instantiations with contracted method referring to expanded one. In general I don't see a big problem as it is very possible to keep that situation under control from user perspective, but it may deserve some documentation and maybe I can find a way to detect such methods and provide certain checks.
I think such functionality (verification of signatures) shall be enabled by default as it is quite likely that due to typos and misunderstanding users will get themselves into obscure errors. On other hand I would like to see option to switch that restriction off as users may want to temporary extend method table.

@patch will need to construct the function which will be added (eval'd) later: The user will define there patch like they did before but we'll also create an function definition expression which perform the runtime check which determines if the patch or the original function is called. This function cannot use args... since this would impact method dispatch so we'll need to extract the actual args names when we call the original/patch functions. Probably want to add additional tests for this as I can foresee scenarios we pass in too many args into the original function

As I stated above, there are ways to read full method signature. When you print method on terminal it extracts this information from MethodInstance, so I don't see why we would need to restrict support on args... or kwargs.... See match_signature in my prototype.

[omus]

To do this we can create the macro @original which utilize Base.invoke_in_world which would allow us to call the original method while a patch as been applied.

That is indeed a right approach, the one that I've used is not working correctly in certain scenarios.

In thinking about this more there are also some challenges with Base.invoke_in_world to consider. If we restore a method by using Base.invoke_in_world we are not only executing that function in an older world age but also restricting any function called by that function to be in the older world age. If this isn't addressable we may have to keep @mock for backwards compatibility as I do want to support the Julia LTS in some way. Maybe just documenting the limitations is good enough as this probably isn't an issue with regular usage.

I think such functionality (verification of signatures) shall be enabled by default as it is quite likely that due to typos and misunderstanding users will get themselves into obscure errors. On other hand I would like to see option to switch that restriction off as users may want to temporary extend method table.

I can get behind this as users typically intend to extend existing functions instead of more specific ones. Making it optional but the default does seem to be the right direction to go.

If we are making it optional however we could implement this feature as a follow up to this work.

@patch will need to construct the function which will be added (eval'd) later: The user will define there patch like they did before but we'll also create an function definition expression which perform the runtime check which determines if the patch or the original function is called. This function cannot use args... since this would impact method dispatch so we'll need to extract the actual args names when we call the original/patch functions. Probably want to add additional tests for this as I can foresee scenarios we pass in too many args into the original function

As I stated above, there are ways to read full method signature. When you print method on terminal it extracts this information from MethodInstance, so I don't see why we would need to restrict support on args... or kwargs.... See match_signature in my prototype.

I think I didn't explain myself enough here. I was trying to point out that when adding our patch to the method table for an existing function we will probably compile the user's patch as an anonymous function where it was defined (in order to retain closure support). When we later applying the patch we'd have to eval something to hook in our patch into the function. I expected we would have an internal function definition which would call the anonymous function. It's that internal function definition which needs to use the specific signature and not rely on using something like f(args...; kwargs...) as this would have lower precedence in multiple dispatch.

[tz-lom]

If we restore a method by using Base.invoke_in_world we are not only executing that function in an older world age but also restricting any function called by that function to be in the older world age.

I meant that invoke_in_world is better than what I did in my prototype. This is not good for reverting method table, but we may employ Revise.jl for that or at least the same technique.

Side topic, I wonder what shall be the intuitive behavior for the following code?

foo(::Any) = -1
p1 = @patch foo(::Int) = 1
p2 = @patch foo(::Float64) = @original(Int(9))
apply(p1) do
  apply(p2) do
    foo(5.0)  # == ?
  end
end

[omus]

If we restore a method by using Base.invoke_in_world we are not only executing that function in an older world age but also restricting any function called by that function to be in the older world age.

I meant that invoke_in_world is better than what I did in my prototype. This is not good for reverting method table, but we may employ Revise.jl for that or at least the same technique.

I agree with your assessment. I think invoke_in_world will work fine for calling the original function but not for reverting the method table.

Side topic, I wonder what shall be the intuitive behavior for the following code?

I was thinking that the behavior in the scenario you brought up would be to call the version of the function at the time before the current apply block. That does mean though that @original may be a misnomer so possibly using a name like @previous would be clearer. Your example would become:

foo(::Any) = -1
p1 = @patch foo(::Int) = 1
p2 = @patch foo(::Float64) = @previous foo(Int(9))
apply(p1) do
    apply(p2) do
        foo(5.0) == 1
    end
end

This also allows us to implement @original still which could call the function as it was outside of all apply blocks. Supporting this would require us to pass through the world age in the ScopedValue for each new scope introduced.

[tz-lom]

I am thinking about extension of @patch macro to allow strict parameter (with default to true) and multiple function definitions.

That's how I see it:

@patch strict=false sin(::Any) = 1  # even sin(nothing) == 1 now
@patch sin(::Any) = 1 # shall fail at `apply`

@patch begin
sin(::Int16) = 1
sin(::Real) = -1 
end  # only 2 methods will be mocked but guaranteed that it will happen at the same time

@patch  begin
sin(::Int16) = 1
cos(::Int16) = 0 # shall fail as it is different function? shall we add parameter to allow this?
end

It also brings us back to considerations of macros, maybe next to @previous and @original we need something like @same_patch to call the neighboring function from the same patch.

[tz-lom]

I've being prototyping "delete_method" for 1.10

I see two approaches:
1 - Replicate functionality from 1.12 of manipulating world ages, as it actually should not break anything
2 - Save original CodeInfo and inject overload which will re-use original one

I've also played a bit with Revise+CodeTracking approach but it can't help with methods which appeared before start of Revise so it is not that good.

I am not sure which one is better, both will require a way to re-create function signature as Expr from Method.
As 2nd one is boring, I've made implementation of the 1st one, but it definitely requires more testing.
https://gist.github.com/tz-lom/0f16df5b1ccd3a5f1343139180f421fd

@omus what do you think about the approach? Shall we create a branch/dummy repo, where we would work on testing of such solutions?

[omus]

I see two approaches:
1 - Replicate functionality from 1.12 of manipulating world ages, as it actually should not break anything
2 - Save original CodeInfo and inject overload which will re-use original one

I think the first approach would be the best (assuming we can get it to work).

I've made implementation of the 1st one, but it definitely requires more testing.
https://gist.github.com/tz-lom/0f16df5b1ccd3a5f1343139180f421fd

Nice prototype. I did some experimentation with your work and iterated on your design. I ended up experimenting with jl_method_table_insert to try to avoid the Base.eval you used to trigger invalidations. This ended up working and I ended up discovering that modifying the Method instance world age allowed me to avoid the fragile modification of _jl_typemap_entry_t: https://gist.github.com/omus/c5f956cd72743664b5acf466c7471ed9

It appears that this approach will work on Julia versions before 1.10 too (I really only care about 1.10 support though).

[omus]

I ended up turning the delete_method prototype I posted into a PR with tests: #142. Looks like we can support Julia 1.6+.

[tz-lom]

This is really nice, I like how you reduced dependencies on internal Julia API to bare minimum.
I was worried about making a deepcopy (that it will introduce issues on Julia finalization), but that is a false alarm - objects are properly rooted in that case.