-
Notifications
You must be signed in to change notification settings - Fork 12
ExtensionErrors Alternative: WeakDepHelpers #111
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
base: master
Are you sure you want to change the base?
Conversation
|
@Krastanov When I load it in a small dummy project that devs this branch I only get the MethodError without any hints. The calls to test hinting Precompilation passes and I get auto completion for the function but only the bare error and no WeakDepError |
|
I will try to look into this later today |
| using WeakDepHelpers | ||
| import Graphs | ||
|
|
||
| const WEAKDEP_METHOD_ERROR_HINT_CACHE = WeakDepCache() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You also need the __init__ hook manually added.
https://github.com/QuantumSavory/QuantumClifford.jl/blob/master/src/init.jl#L19-L21
function __init__()
if isdefined(Base.Experimental, :register_error_hint)
Base.Experimental.register_error_hint(MethodError) do io, exc, argtypes, kwargs
method_error_hint_callback(WEAKDEP_METHOD_ERROR_HINT_CACHE, io, exc, argtypes, kwargs)
end
end
end
Let me know if this seems distasteful and we can figure out a cleaner API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be a lot of boilerplate for every code wanting to use WeakDepHelpers.
I have implemented and tested it and it works now.
I have also moved everything within init into a register_weakdep_cache function that receives the cache singleton. I think it might be helpful to add this to WeakDepHelpers to increase usability
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I concur @jpthiele .
I am happy to take care of the implementation. Could you post an issue on the WeakDepHelpers tracker with an example boilerplate that you would find "reasonably pleasant / unobtrusive"? Or do you think that just a pre-defined register_weakdep_cache would be sufficient?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this predefined function might be sufficient. Makes it easier to use for those not too familiar with the hinting system while still being easy enough to be copied and modified by expert users if needed.
|
@j-fu in comparison to ExtensionErrors this works with error hints which so far are expermental (for a while..). Between the usual |
|
Just for a bit of extra context, keeping the original MethodError (instead of hiding it behind a different error type) was an explicit design goal of WeakDepHelpers. Unlike other error types, MethodError has a "privileged" position, coming from deep inside of the runtime, hooked into IDE tools, related to the basic paradigm of how Julia works, thus various design guides advise on not hiding it (and it even has special handling from the runtime with additional context being printed about it in recent versions of julia). Another minor advantage of this approach is that it avoid invalidations. There is no custom-error-throwing method defined, so there is nothing to invalidate when the extension is loaded. A (big) drawback of this approach is that it requires an |
On the same day as I submitted ExtensionHelpers to General the package WeakDepHelpers was also submitted using an alternative approach based on error hints.