Is using `Base.delete_method` on user-defined functions safe?

does someone know if using Base.delete_method user-defined functions is dangerous in any way? I searched for its use on registered packages and someone does already make use of it: https://juliahub.com/ui/Search?q=delete_method&type=symbols. As a minimal example:

julia> f() = 1
f (generic function with 1 method)

julia> Base.delete_method(methods(f)[1])

julia> f() = 2
f (generic function with 1 method)

julia> f()
2

is this bad for some reason (apart that it is unexported)? Or are there other cases which it can cause unexpected behaviours?

For clarification, is there any reason you can’t just redefine the method?

julia> f() = 1
f (generic function with 1 method)

julia> f() = 2
f (generic function with 1 method)

julia> f()
2

@CameronBieganek when this happens in a package it makes precompilation to fail with WARNING: Method definition X in module Y overwritten at Z, and this is my real use case. It doesn’t fail if you first remove the method.

Why are you overwriting the method at all? Why not just delete the first definition from your code?

1 Like

it’s because I actually generate the function with a macro given the information of the function until that point. If it happens that new information shows up for that updates that specific signature of the function I need to redefine it to incorporate the new info. More concretely, I produce a series of if-else branches inside a function at different point in the code with the help of a macro.

It’s hard to say without knowing more, but perhaps you can keep an Expr around with the function definition, and then evaluate the Expr once you know it’s ready?

At any rate, I am of the opinion that you should avoid using private functions from Base (or third-party packages) unless absolutely necessary.

3 Likes

Actually I can let you know a lot more :smiley: , this is the package in question: GitHub - JuliaDynamics/DynamicSumTypes.jl: Combine multiple types in a single one. Actually I sidestepped a bit all this issue by doing exactly what you said with a Expr…I tried to load all the expressions inside (a subpackage) __init__() which is loaded automatically after the package is precompiled, there is a problem though because it means that one can’t actually use a precompile.jl file,actually the __init__ function could call the precompile statements. Stupid me. So this is okay. The only real issue is that they functions defined that way can’t be extended outside of the package, but I can live with that because I can’t really conceive that being useful in this case. So I think I’m fine actually :slight_smile:

That said, I’m still curious, also because there are already some packages doing something similar

1 Like

I have a similar issue in DuckDispatch where I need a fallback method to catch all the calls, wrap them based on the current set of applicable DuckTypes and then redispatch the wrapped arguments.

But each time a new DuckType is used to dispatch that same function, I need to replace the original fallback with one that has the new list of DuckTypes to check.

So I get the warning too.

Edit: sorry for autocorrect profanity :man_facepalming:

Using an internal function to evade an error doesn’t imply you don’t run into the dangers the error was trying to prevent. Precompilation caches apparently don’t support method overwriting well at all:

Turn Method Overwritten Error into a PrecompileError – turning off caching by vchuravy · Pull Request #52214 · JuliaLang/julia · GitHub

I don’t know the mechanics of why overwriting methods during precompilation is such an issue, and the underlying code in gf.c appears to be completely separate, so I can’t say if deleting then replacing a method would do the same thing.

2 Likes

thanks guys for all the help, I’m hoping someone a lot knowleadgeable on internals will see this post because I’m looking for a definite answer to this question since my package has some limitations I could lift with the approach I mentioned, but I really first need to know if this is safe or not

Let me elaborate a bit on the few aspects:

  1. it’s fully internal, so it’s possible for a minor revision to make a breaking change that forces you to edit your code to keep up.
  2. separate precompilation caches fundamentally cannot handle a duplicated method because loading order is chaotic and thus should not affect behavior. For example, precompiling a 3rd package loading 2 prior caches would need a fixed loading order to pick 1 of 2 duplicated method definitions, but you could easily load those caches in the opposite order before importing the 3rd package, causing a silent discrepancy. This is especially bad when load order can be fully arbitrary e.g. extensions CExtA and CExtB with the statement import A, B, C. This clear danger is what the error is trying to stop. If you work around this with delete_method, you will encounter this danger. My earlier question was about overwriting within the same cache prior to any specializations.
  3. __init__ executes after loading, so nothing in it including precompile results will make it into the cache. I would still avoid overwriting methods there because it’ll invalidate precompiled methods, but at least this is suboptimal rather than impossible.
2 Likes

thank you a lot @Benny, this clarifies everything…I’m a bit sad about point 3. though, because I would really want to allow precompilation. Is there a way to lift 3. by bulk-defining all the methods defined with the macro somewhere else?

Maybe I just need to go with the simplest solution: the user adds a single statement at the end of the module which defines them all.

Incremental AOT compilation, which is what precompilation caches are doing for packages and extension in a JIT-compiled language, has fundamental limitations, and while it’s technically possible to improve things there is a big list of things precompilation can’t handle properly.

Again, I don’t know why method overwriting within the same module before precompiling any function calls is disallowed, but do you really have to evaluate the function before it’s final state in the precompilation cache? Since you’re doing metaprogramming, you could manipulate an Expr and eval it when it’s done. Granted, I’ve never done this before so I would experiment with this and make another thread asking people to confirm that this won’t go wrong, but my hunch is this should work because there are plenty of @eval loops over Cartesian products of base operators and types that seem to be precompiled.

that is basically what I’m doing, it is just that I don’t really know when the final body of the function will be reached, even if I know it should be at most at the end of the module, so what I can do is to put the evaluation in the __init__ function of the module. But clearly there is the simple solution to just let the user say when the function is finalized, it is just that it would have been cooler to have this automatically (with a “define all those function at the end of the module” instruction). But it is probably something impossible, and actually maybe obfuscating the real work of the system, so I’m now convinced the simple solution is the best one :slight_smile:

1 Like