RFC: introduce check macro that can be disabled with a flag

[KristofferC]

TLDR: for triage: Add docs to @assert that the block might not be executed, so don't e.g. use it for needed side effects?

It is common for packages to roll their own way of enabling a debug mode. This is typically done using a global const DEBUG = true / false which one has to edit in the source file to set. Another option is to use an ENV variable and start julia with package precompilation disabled. These are non ideal because manually editing files is awkward and doesn't really scale when you want to run multiple packages in debug mode and then swap back to release mode, and disabling package precompilation increases load time every time you want to debug.

In #10614 a suggestion was made for a @check macro completely disables a code block when not running in debug mode / alt. only enables it when running in "optimize" mode. This PR introduces a --debug flag that enables the @check macro (i.e. code in a @check block is then not removed). Furthermore, running with --debugchanges the precompilation path (adds adebug` directory) so that it works with packages that enables precompilation.

The macro is currently not usable in base because any result of it will be cached into the sysimg. For it to work in Base, we probably need a second debug sysimg that is chosen based on the --debug flag.

Note that @check is not really intended for debug logging (the logging macros already work for that) but for situations where verifying certain properties might be too expensive to do normally. Such an example might be to check that the input to searchsorted actually is sorted.

This PR is mostly to spur some discussion if this is something we want, if this type of implementation makes sense, etc. It is not aimed at 1.0 but one thing we need to decide for 1.0 is if to instead repurpose the current @assert macro to be what @check is in this PR. There is currently no way to remove @assert but if we want to use @assert for this in 1.1 we need to document that @asserts can be removed. Therefore, putting the triage label on it.

[JeffBezanson]

+1 to documenting that @assert can be removed. I also think @check and @assert should be merged, with a specific option like --enable-assert={yes|no}.

[StefanKarpinski]

Agree with @JeffBezanson: this is exactly what @assert is already meant for – we just haven't had modes in which @assert does or doesn't execute.

[c42f]

I think something like this is useful, and agree that merging with @assert would make sense.

To make this more dynamic (and making the most of #265) I suggest the following mechanism:

module DebugAsserts

export debug_asserts, @dassert

function debug_asserts(m::Module, dodebug::Bool)
    m.eval(:(_debug_enabled() = $dodebug))
    nothing
end

# Like @assert, but only active in debug mode
macro dassert(exs...)
    if !isdefined(__module__, :_debug_enabled)
        eval(__module__, :(_debug_enabled() = false))
    end
    quote
        if $__module__._debug_enabled()
            @assert $(map(esc,exs)...)
        end
    end
end

end

This way, if you need to turn debug assertions on in a release build you can do that, and vice versa (the release and debug builds can change the default value returned by _debug_enabled).

Edit - related discussion here: https://discourse.julialang.org/t/defensive-programming-assert/8383/11. To be clear, I'm not suggesting we use the name dassert, simply that the above mechanism might be useful.

[thofma]

Why merge them? They have different purposes as @KristofferC has described. Some you always want to do and some you want to do only in debug mode.

[c42f]

There's a rather strong convention from other languages that the thing called assert is for debugging and should be compiled out in release mode.

There's at least four or so prior issues/prs discussing a release-mode version of something like assert. #7732 suggests @enforce which I quite like.

[JeffBezanson]

Some you always want to do

For those just write a normal check with no macro.

[KristofferC]

I think most people are after the pretty printing that comes with @assert.

[thofma]

Indeed, it is very simple to use. No need to come up with an error message and no need to write if/&&.

[JeffBezanson]

Ok, if people want a @check macro for that purpose that might be ok, but then there shouldn't be an option for removing it. A check that can be removed is by definition an assertion.

[JeffBezanson]

This is a new feature; doesn't need to be triaged now.

[KristofferC]

What we need to decide is if we should introduce a new macro with the same effect as the current @assert, which we promise to not optimize away, deprecate @assert and then reintroduce it later with a similar implementation as in this PR.

[fredrikekre]

Lets deprecate @assert to @enforce in 0.7, and then introduce @assert for 1.0 that can be optimized away. For 0.7 we can use the new Future stdlib and introduce a Future.@assert.

[StefanKarpinski]

How about calling it @check instead? @enforce seems like kind of a weird name.

Or perhaps, since this will generally be used for checking argument validity, calling it @argcheck and throwing and ArgumentError when it fails. In that case, if we get the pretty printing good enough, it can be a nice, concise way to express argument checking with good error messages instead of just a lazy and inferior way to do it.

[JeffBezanson]

Going through a cycle of renaming and un-renaming assert is too annoying. That punishes people who are using it correctly. We should just document it better.

[KristofferC]

If you are using it correctly, the point is to just do using Future and get @assert. I'm fine just documenting as well.

[StefanKarpinski]

It seems like we may want to just:

• Document that @assert should only be used where it can correctly be removed
• Add debug and release modes where assertions are included and omitted, respectively
• Add unconditional @checkargs that produces nicely formatted errors for argument checks

[KristofferC]

Ok, so only thing for 1.0 is to fixup the docs for @assert.

[StefanKarpinski]

It might be good for @assert to be ignored in the default mode for 1.0 although that's kind of a weird situation to have @assert be a comment with no way of turning asserts on.

[antoine-levitt]

It would be strange if the default mode was the optimized one and typing @assert 1==2 at the default REPL did nothing. Removing the asserts to me belongs to the same class of options as removing bound checks : the default should be the debug mode, and once you're satisfied your code runs without throwing and you want extra speed, you switch on flags so that errors can be treated by the compiler as undefined behavior.

(also -1 on having an @checkarg without a @check, as asserts/checks are useful in other contexts too)

[swissr]

How about calling it @check instead? @enforce seems like kind of a weird name.

No longer relevant for this issue, just to mention another possible name: @ensure might fit nicely (stolen from Eiffel postcondition).

[chethega]

I really like the general idea of having three modes:
default: perform @assert
release: Skip asserts
slow-debug: really, really check. Maybe, for OOB, throw different exceptions depending on whether we were in an @inbounds context or not (one can be catched, the other is a hard fail). If it gets a 100x slowdown, then so be it.

Then, something like "@debug_assert level expression" could be used to document and possibly check assumptions like issorted.

Possibly, one could also do very slow things like integer overflow checking (AFAIK signed integer overflows on addition are currently UB?). Possibly emit checks for other UB.

[KristofferC]

#37874