Add @check macro for non-disable-able @assert

[ericphanson]

I've found that sometimes folks use @assert for its simple syntax and clear error message in cases that are inappropriate for asserts (e.g. checking that data is consistent rather than program logic is consistent). I think it would be great to provide an alternative with an equally simple syntax and clear error messages that will never be disabled. At the same time, we can use the opportunity of the macro to outline the string creation to improve performance. E.g. with this PR,

function hv_cat(rows::Tuple{Vararg{Int}}, xs::Int...)
   nr = length(rows)
   nc = rows[1]
   for i = 2:nr
       @audit nc == rows[i]
   end
   Base.hvcat_fill!(Matrix{Int}(undef, nr, nc), xs)
end

matches the "fast" performance of #41307 for me (~23ns, instead of ~80ns for the "slow" performance). (I made the same tweaks to @assert here, so @assert would work as well, although that would be an incorrect usage of it here nevermind, took that out due to bootstrap issues. Which incidentally is why there's code duplication from @assert and prepare_error).

I also experimented with using code from FastCaptures to outline the whole logic check + error (as suggested by @KristofferC in #29688 (comment)) in 6a6b9ac, but in the hv_cat example it was the same speed as not outlining at all. Perhaps I made a mistake (I am not very experienced with macros), though from @code_warntype and @macroexpand it looked like it was doing what I wanted.

@kleinschmidt jokingly suggested the name @audit when I was calling it @throw_unless and I ended up liking it so I figured I'd use it here.

closes: #10614

[simeonschaub]

Regarding the name, maybe @require?

[jw3126]

The ArgCheck.jl package has the @check macro, which seems similar.

[jpsamaroo]

@require is already taken by Requires.jl. How about @ensure?

[ericphanson]

Regarding the name, maybe @require?

I thought that could be confusing since Base.require is very different, but that's not a public API anyway so maybe it's fine? @require does get the point across well.

The ArgCheck.jl package has the @check macro, which seems similar.

True, it does. One difference is the outlining feature, but I'm sure that could be added to ArgCheck.jl. One reason I think it would be good to have something like this in Base is that @assert is in Base and it would be nice to have an equally available alternative to help push users away from incorrect use of @asserts. I think sometimes @assert is used out of convenience but one-off code has a way of sticking around...

[jw3126]

True, it does. One difference is the outlining feature, but I'm sure that could be added to ArgCheck.jl. One reason I think it would be good to have something like this in Base is that @assert is in Base and it would be nice to have an equally available alternative to help push users away from incorrect use of @asserts.

I agree. I would love it if the ArgCheck package could be deprecated and Base would have that functionality instead.
One thing ArgCheck does is put energy into creating good error messages, a bit like @test.
Observe that while @assert only puts compile time information into the error, @test and @check provide information about runtime values:

julia> using Test, ArgCheck

julia> x = NaN
NaN

julia> @assert isfinite(x)
ERROR: AssertionError: isfinite(x)

julia> @test isfinite(x)
Test Failed at REPL[28]:1
  Expression: isfinite(x)
ERROR: There was an error during testing

julia> @check isfinite(x)
ERROR: CheckError: isfinite(x) must hold. Got
x => NaN

julia> @assert x ≈ 2
ERROR: AssertionError: x ≈ 2

julia> @test x ≈ 2
Test Failed at REPL[32]:1
  Expression: x ≈ 2
   Evaluated: NaN ≈ 2
ERROR: There was an error during testing

julia> @check x ≈ 2
ERROR: CheckError: x ≈ 2 must hold. Got
≈ => isapprox
x => NaN

[StefanKarpinski]

@check does seem like the best name for this.

[ericphanson]

I started working on tests and realized the outlined function in my code only works if the msg you pass is a string, not an exception or even arbitrary code like @assert allows (and evaluates when triggered). I think this is due to eval’ing it, but I did want to generate the function in global scope at macro expansion time. Maybe that is the wrong approach though. I’d appreciate any pointers.

[jw3126]

I started working on tests and realized the outlined function in my code only works if the msg you pass is a string, not an exception or even arbitrary code like @assert allows (and evaluates when triggered). I think this is due to eval’ing it, but I did want to generate the function in global scope at macro expansion time. Maybe that is the wrong approach though. I’d appreciate any pointers.

You can take a look at ArgCheck.jl. It does outline the error generation. It only inlines the throw, which would be trivial to outline as well.

@macroexpand @argcheck x > 1 DimensionMismatch
    begin
        #= /home/jan/.julia/packages/ArgCheck/5xEDR/src/checks.jl:239 =#
        var"#1###calle#274" = (>)
        var"#2###arg#272" = x
        var"#3###arg#273" = 1
        #= /home/jan/.julia/packages/ArgCheck/5xEDR/src/checks.jl:240 =#
        if var"#1###calle#274"(var"#2###arg#272", var"#3###arg#273")
            #= /home/jan/.julia/packages/ArgCheck/5xEDR/src/checks.jl:241 =#
            ArgCheck.nothing
        else
            #= /home/jan/.julia/packages/ArgCheck/5xEDR/src/checks.jl:243 =#
            ArgCheck.throw(ArgCheck.build_error(ArgCheck.CallErrorInfo($(QuoteNode(:(x > 1))), ArgCheck.ArgCheckFlavor(), $(QuoteNode(Any[:>, :x, 1])), [var"#1###calle#274", var"#2###arg#272", var"#3###arg#273"], (DimensionMismatch,))))
        end
    end

[KristofferC]

Perhaps these should all be put into some submodule to not pollute the namespace of e.g. Base too much?

[ericphanson]

Ok, I removed the outlining exceptions bit because I couldn't figure out how to do it quickly (despite the nice example provided by ArgCheck.jl), but I figure that can be done in a future PR since it doesn't change the semantics. I also renamed it to @check, and added NEWS, docs, and tests (by copying @assert).

[ericphanson]

Bump 🤞

[DilumAluthge]

Bump @KristofferC @StefanKarpinski

[DilumAluthge]

It would be great to get this into 1.8.

[ericphanson]

Bump

[tkf]

Why not copy code from ArgCheck.jl which provides strictly more information and also is battle-tested? We can optimize this more later using the same logic as #41342 (comment)

[ericphanson]

Just because this is simpler and having a nice alternative to @assert to use and to point to in code reviews is more important to me than performance (outlining etc) here. In terms of battle-tested this is just @assert which seems plenty battle-tested. But if the fancier implementation will help get something merged then I’ll do it :).

[tkf]

Quoting your comment in Zulip

I sometimes want things in the stdlib because there’s something else similar there and I don’t want that to be favored by virtue of not needing dependencies.

I agree with your observation. However, this PR as-is seems to make Base.@check favorable over ArgChecks.@check while the latter makes debugging easier and thus makes the ecosystem more robust.

That said, I don't know enough about the principle on which the core devs operate and so I can't guess which way can be more easily accepted.

[StefanKarpinski]

My take here is that some package exporting a name can't be considered to block Base from exporting the same name. @check seems like the most obvious name for this; people who use ArgCheck can/should import ArgCheck: @check. The name @ensure is ok too though. We should do something.

[Seelengrab]

What about @assume as an alternative to @check? According to juliahub, that is still free and IMO has a much stronger association with not being removed than @check does.

[MasonProtter]

I don't see how @assume is associated with not being removed. If anything, I'd think that @assume x > 1 is saying "I promise that x is greater than 1, you don't need to check it"

[Seelengrab]

It's just a different suggestion to avoid conflicts with the ecosystem 🤷 In Hypothesis (and my soon-to-be-released port of it), assume is a verb to designate conditions that MUST be true, otherwise an error is thrown (which is handled by the framework, but that's an irrelevant detail). It's not ever removed.

I'm fine with @ensure too, I just want to avoid having conflicts between what an established package means with @check and what Base means.

[ericphanson]

avoid having conflicts between what an established package means with @check and what Base means

there's no conflict here with meaning; semantically, this PR's @check is identical in meaning to ArgCheck's.

[ericphanson]

Maybe @jw3126 would be willing to upstream ArgCheck’s @check macro to Base? It is fuller featured though more complicated than this PR which just duplicates @assert with a new name. Then ArgCheck could just re-export it on Julia versions where Base has it.

[jw3126]

I would love to have ArgCheck in Base. However, I have experienced quite a few PRs to julia in the past, where the following happened:

• Somebody put work into a PR
• The PR then it did not get the attention of the right people and would neither be merged nor rejected, see for instance RFC: setindex for mutable collections #33495 (comment)

Because of this, I am hesitant to put work into julialang PRs. I am happy to do the work, but only if triage or some core dev states that we want it and promises that the PR either gets merged or rejected for good technical reasons.

[KristofferC]

Why can't the answer here be to just use ArgCheck.jl? It looks great, you can get updates whenever etc.

[ericphanson]

1. People abuse @assert since it’s “easier” since you don’t need a new dependency
2. If we start turning off @assert sometimes (as we have every right to do) it’s easier to fix code with a drop-in replacement that doesn’t need a new dependency to do so

[ericphanson]

BTW, ArgCheck hasn't had any changes since June 2022, so the slow release cadence of Base actually seems totally fine at this stage of maturity.

[tgross35]

Twocents: I would strongly prefer @assert be made the non-disabled version, then add a new @debug_assert that can be turned off sometimes. Reasoning:

1. @assert is currently used a lot of places where it is assumed to always be turned on, even though the warning reserves the right to change that behavior (this is a problem in the Python ecosystem too). Introducing something new for the behavior of RFC: implement proper debug mode for packages (with support for re-precompilation) #37874 is much less likely to break things, even if those cases aren't technically doing the correct thing.
2. The difference between the assert and debug_assert is pretty self explanatory, and I think it is pretty unlikely for someone to mistakenly use debug_assert! to enforce code correctness. The names check vs. assert really don't give a clear indication of which is which.
3. There is already some prior teaching available for shared knowledge - Rust uses assert and debug_assert and does a pretty good job of explaining when either should be used. Kotlin is the only language I know of to use assert and check terminology (also require), but I don't think it does as good a job explaining when either should be used.

I think doing this would also make #37874 more palpable, since some of the more recent objections are assert-related.

[o314]

• @assert is currently used a lot of places where it is assumed to always be turned on, even though the warning reserves the right to change that behavior (this is a problem in the Python ecosystem too). Introducing something new for the behavior of RFC: implement proper debug mode for packages (with support for re-precompilation) #37874 is much less likely to break things, even if those cases aren't technically doing the correct thing.

I agree with that.
Another choice is to use a level parameter to the macro and consider a default choice somewhere like for logging

macro logmsg(level, exs...) logmsg_code((@_sourceinfo)..., esc(level), exs...) end
macro debug(exs...) logmsg_code((@_sourceinfo)..., :Debug, exs...) end
macro  info(exs...) logmsg_code((@_sourceinfo)..., :Info,  exs...) end
macro  warn(exs...) logmsg_code((@_sourceinfo)..., :Warn,  exs...) end
macro error(exs...) logmsg_code((@_sourceinfo)..., :Error, exs...) end

the macro may be in base. differents behaviors may even be handled in different module via multidispatch.
That may require a dispatch on singleton-struct-based type hierarchy like iterators trait or some type lift of bitstype via Val.