merge with SmartAsserts

[jw3126]

As @MrVPlusOne suggested on discourse it probably makes sense to merge this package with SmartAsserts.jl. I think the best option would be to copy the clever enable/disable mechanism to ArgCheck.jl. What do you think @MrVPlusOne ?

[jw3126]

Currently OptionalArgCheck.jl gives compile time control over checks, but it is not really maintained and depends on IRTools. OTOH the mechanism of @MrVPlusOne is really lightweight and simple, so I would prefer it.

[MrVPlusOne]

I think the best option would be to copy the clever enable/disable mechanism to ArgCheck.jl. What do you think @MrVPlusOne ?

Yes, that sounds like a plan, and it should be very simple to implement. Also, I wonder if ArgCheck has a macro that follows @assert's signature (i.e., accepts an optional error message provided by the user). If not, would it be a good idea to implement such a macro using the existing infrastructure?

[jw3126]

Awesome! Yes, you can already pass an error message as a second argument.

[jw3126]

I wonder how the mechanism in SmartAsserts.jl interacts with precompilation. For instance if I disable argchecks in Main and import a package, that was already precompiled with argchecks enabled. Then checks in that package are still enabled right?

[MrVPlusOne]

Yes, I feel this is kind of inevitable if we want to switch the assertions at compile-time... But at least the user would be able to control the assertions inside their own packages.

[jw3126]

I think this is a very serious bug. Especially in the reverse direction. Imagine a user disables checks in one session and under the hood a bunch of random packages get precompiled. In another session a user wants to track down a bug, but checks are silently still disabled.

[jw3126]

Also I am not sure it is inevitable. A function redefinition triggers a recompile. Maybe check could insert a noop function imto the code and a @disable macro redefines this function into another noop

[jw3126]

Or maybe just macroexpand a check into:

if ArgCheck.enabled()
    # the usual @check code
end

and we have enabled() = true in ArgCheck.jl, but of course the user can write ArgCheck.enabled() = false.

[MrVPlusOne]

I think this is a very serious bug. Especially in the reverse direction. Imagine a user disables checks in one session and under the hood a bunch of random packages get precompiled. In another session a user wants to track down a bug, but checks are silently still disabled.

Hmmm... I'm not familiar with how precompilation works in Julia. I kind of assumed that all dependencies are compiled before the current project for this simple approach to work, but I guess this might not always be the case? Then I agree that silently turning off checks in other packages sounds very dangerous.

[MrVPlusOne]

Or maybe just macroexpand a check into:

if ArgCheck.enabled()
    # the usual @check code
end

and we have enabled() = true in ArgCheck.jl, but of course the user can write ArgCheck.enabled() = false.

I wonder after the user redefined ArgCheck.enabled() = false in their package, whether it is guaranteed that all the previous @check calls will be recompiled? I remember seeing Julia complaining "incremental compilation may be fatally broken for this module" whenever I try to override a method definition of another package.

[MrVPlusOne]

If we only aim to let the user disable @checks in their own package, then I guess an alternative that does not have this "silent switch-off" issue would be to change ArgCheck.ENABLED::Ref{true} into ArgCheck.DISABLED_MODULES::Set{Module}. Then, in @check, we expand to noop if it's called within a module belonging to this set. And we can provide the user with another macro ArgCheck.@disable_checks_for_current_module that inserts the current module into this set.

[jw3126]

I wonder after the user redefined ArgCheck.enabled() = false in their package, whether it is guaranteed that all the previous @check calls will be recompiled? I remember seeing Julia complaining "incremental compilation may be fatally broken for this module" whenever I try to override a method definition of another package.

Ah right, that is problematic.

[jw3126]

If we only aim to let the user disable @checks in their own package, then I guess an alternative that does not have this "silent switch-off" issue would be to change ArgCheck.ENABLED::Ref{true} into ArgCheck.DISABLED_MODULES::Set{Module}. Then, in @check, we expand to noop if it's called within a module belonging to this set. And we can provide the user with another macro ArgCheck.@disable_checks_for_current_module that inserts the current module into this set.

That would probably work. But I am not sure how useful it is. Personally, I have two main use cases for turning off checks.

• One is I run a simulation, I am sure it is correct and I want best possible speed. In this situation, I want to disable all checks everywhere.
• The other is I have a small immutable struct that gets instantiated in a hot loop and has an argcheck in its constructor. Say Particle(position, direction, energy) and (@argcheck energy > 0 in its constructor). The check introduces a branch, that prevents many optimizations in the hot loop. In this case, I would like to disable checks very locally in the hot loop only. Pretty much what @inbounds does.

What are your use cases? Would the ArgCheck.@disable_checks_for_current_module be useful for you?

[MrVPlusOne]

What are your use cases? Would the ArgCheck.@disable_checks_for_current_module be useful for you?

Personally, in most of my use cases, I don't really mind letting the checks stay. And for cases where I do perform very expensive checks (typically when I'm debugging something), I used to manually code up some API to control the checks (e.g., should_check && @assert <some expensive checks>), or simply comment out the expensive checks after the bug is fixed. But I guess sometimes it might be very handy for the user if they can turn off all their own tests using a simple switch.

I'm not sure about the idea of letting the user turn off the checks in other packages, though. It feels like a more principled approach might be for the other (performance-critical) packages to provide an API to turn off their checks?

The other is I have a small immutable struct that gets instantiated in a hot loop and has an argcheck in its constructor. Say Particle(position, direction, energy) and (@argcheck energy > 0 in its constructor). The check introduces a branch, that prevents many optimizations in the hot loop. In this case, I would like to disable checks very locally in the hot loop only. Pretty much what @inbounds does.

For this use case, can we implement something like @disable_checks_for_function which internally call disable_checks_for_current_module and enable_checks_for_current_module at the beginning and end of the function body?

[jw3126]

I'm not sure about the idea of letting the user turn off the checks in other packages, though. It feels like a more principled approach might be for the other (performance-critical) packages to provide an API to turn off their checks?

That's true, I like the inbounds mechanism in Base. Would be good to have such a thing, but for other kinds of checks as well. https://github.com/simeonschaub/OptionalArgChecks.jl lets you sprinkle your code with markers and provides means to erase all code sections with a given marker at compile time. This allows to have custom mechanisms, close to the boundschecking in Base. Sadly the package is based on IRTools. But I hope/expect that in future there will be stable apis for compiler plugins, that can be used instead of IRTools.

For this use case, can we implement something like @disable_checks_for_function which internally call disable_checks_for_current_module and enable_checks_for_current_module at the beginning and end of the function body?

I think @disable_checks_for_function would need to be added to the source code of a method? In that case I one can directly comment out the checks as well I guess?

I remember seeing Julia complaining "incremental compilation may be fatally broken for this module" whenever I try to override a method definition of another package.

There might be a hack to work around this. In Argcheck we define enabled(args...) = true, while the use can overload the more specific enabled() = true...

Right now I don't find any of the options we have very appealing. Personally, I would probably wait until there is a more stable replacement for IRTools and revive OptionalArgChecks. However, I am open to adding something in the meantime if you need it.

[MrVPlusOne]

I think @disable_checks_for_function would need to be added to the source code of a method? In that case I one can directly comment out the checks as well I guess?

Right, I probably misunderstood your use case. Now it sounds like you only want to disable the checks for specific uses of a method, not to turn off all of them. So the approach taken by OptionalArgChecks would be more suitable.

Right now I don't find any of the options we have very appealing. Personally, I would probably wait until there is a more stable replacement for IRTools and revive OptionalArgChecks. However, I am open to adding something in the meantime if you need it.

Sounds good. I guess in the meantime, I can implement a run-time checking mechanism for SmartAsserts so that people can still use this functionality if they do not mind a little bit of extra overhead.