Skip to content
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

omit assert under --optimize; introduce@check macro? #10614

Open
StefanKarpinski opened this issue Mar 24, 2015 · 23 comments · May be fixed by #41342
Open

omit assert under --optimize; introduce@check macro? #10614

StefanKarpinski opened this issue Mar 24, 2015 · 23 comments · May be fixed by #41342
Labels
error handling Handling of exceptions by Julia or the user

Comments

@StefanKarpinski
Copy link
Member

People often use @assert in their code to check that functions are being used correctly. It's typical in other systems to eliminate assertions under higher optimization levels. Should we do that under --optimize? If assertions are ignored (compiled to no-ops) but also used to check correct usage – as opposed to checking the correctness of internal logic – then this can accidentally introduce a dangerous lack of argument checking. So perhaps we should introduce a @check macro that is not eliminated by --optimize and is intended for checking usage as opposed to internal correctness.

@toivoh
Copy link
Contributor

toivoh commented Mar 24, 2015 via email

@JeffBezanson
Copy link
Member

+1 to removing asserts, but I'd prefer if statements and throw to @check. A one line macro makes it harder to specify what type of exception to throw, and I think @check and @assert will be easy to confuse.

@StefanKarpinski
Copy link
Member Author

People like just writing @assert size(A,2) == size(B,1) and having the error message automatically generated for them – that's a huge part of the reason people use @assert for this kind of thing even though it's not really right.

@JeffBezanson
Copy link
Member

Fair enough. But what would we throw? ArgumentError? ErrorException?

@StefanKarpinski
Copy link
Member Author

ErrorException seems like the reasonable default. If you want to do something more specific, just do a check and throw like normal. This is a feature for hastily written code.

@ViralBShah
Copy link
Member

AssertException?

@jakebolewski
Copy link
Member

assert now throws an AssertionError in 0.4.

@JeffBezanson
Copy link
Member

It will probably have to be an ErrorException. You don't want a special type of exception just for using @check; it should be a shorthand for some routine kind of checking like argument checking.

@mbauman mbauman added the error handling Handling of exceptions by Julia or the user label Mar 24, 2015
@jakebolewski
Copy link
Member

I'm struggling to see the value in this proposal, does this mean that "library" code will have assertions which can not be turned off and user level code will have @check assertions that can? To me if a library is using assertions to check internal consistency of arguments / values that library is poorly designed, why can't it throw a proper Exception instead? I feel with this proposal that you will have a mix of assertions / checks in user and library code which will be a mess.

@JeffBezanson
Copy link
Member

Read it again; @assert could be removed, but @check would be a shorthand for error checking (not assertions) that can't be removed.

@jakebolewski
Copy link
Member

@check seems too convenient. If users are writing library code that contains @checks how do you recover from an error when you have no clue where it was thrown or what condition led to it being thrown. This will be like generic error() messages on steroids.

Being able to turn @assertions off seems worthwhile.

@StefanKarpinski
Copy link
Member Author

The alternative is people misusing @assert or not error checking at all, not writing better error checking.

@ivarne
Copy link
Member

ivarne commented Mar 25, 2015

We already have one concept for turning off correctness tests with the @inbounds macro, and a runtime flag. We should extend that, rather than introducing a new concept.

@ivarne
Copy link
Member

ivarne commented Mar 25, 2015

Ref #7799.

To elaborate: Bounds checking and assertions are both checks that are written in order to ensure that if the caller of a function makes a mistake, you get an exception instead of data corruption. Some code might be written with the assuption that assertions/bounds checks are on, while other code does the checking before calling the function.

A global flag to turn theese sanity checks on and off, is great for debugging, and performance testing. For real programs though, I'd want to have them on by default, and use some sort of signaling at the call site to disable them for specific regions where I can verify that the conditions are true and that the performance is critical.

I'm not qualified to talk about implementation, but the only way I can really see this work is to have a Boolean parameter to all functions that tells if it was called in a assertion free context (or a bitmask style enum to have finer granularity, but I can't imagine when you'd want to disable bounds checks, but not assertions)

@tkelman
Copy link
Contributor

tkelman commented Mar 25, 2015

I like the idea of conceptual unification with @inbounds. And I swear there's an issue already open on making assertions no-ops in some conditions, and likewise having an alternate macro that does the same thing as @assert but can't be disabled. Github search is terrible though.

I find myself writing my own assertion macro for input checking that lets me know what went wrong, what the input values were if they were unexpected, sort of part way towards Base.Test in functionality, but maybe that's just a consequence of not having a debugger.

@samoconnor
Copy link
Contributor

#15495

@Sacha0
Copy link
Member

Sacha0 commented Apr 16, 2016

#7732

@x-ji
Copy link

x-ji commented Dec 23, 2018

I wonder if @assert is now turned off when I start Julia with the -O3 flag. I see in @assert's function doc that "@assert might be turned off at various optimization levels". I'm not sure if it's actually turned off or not currently.

@fredrikekre
Copy link
Member

It's not.

@Seelengrab
Copy link
Contributor

Though, just because it's not turned off right now, doesn't mean it's safe to use for checking. It's explicitly allowed to no longer be run with any new version, so relying on @assert working is still not safe.

@ericphanson ericphanson linked a pull request Sep 6, 2023 that will close this issue
@Uroc327
Copy link

Uroc327 commented Nov 21, 2023

Any progress on this?

The advantage of disabling @asserts via julia instead of via custom assert macros is that the optimizer can still use the type information.

Something like

function fun(a::AbstractArray{<:Number})
    i = findfirst(iseven, a)
    @assert !isnothing(i)
    return a[i] + 1
end

would loose the i::Int64 type info after removing the assert using custom macros. The same thing also happens for something like

@assert i >= 1
@assert i <= length(a)
a[length(a) - i + 1] # no @inbounds needed

and the bounds checking.

@jakobnissen
Copy link
Contributor

I don't think this has anything to do with the @assert macro - the compiler can get type information from any code that throws an exception.
In fact, if the @asserts were disabled, then the compiler couldn't use the type information, because then the inferred type might be incorrect.

@Uroc327
Copy link

Uroc327 commented Nov 22, 2023

Sure, that's my point exactly. I want something that

  • is removed from the final compiled assembly completely because this is performance relevant code where I don't want to check/throw anything and
  • still can give the same information to the compiler.

For performance critical code, it is the caller who makes sure the function is used correctly. Yet, I'd like to be able to tell the compiler that it's fine to assume that the asserted condition holds. Also, being able to check asserts during "debug builds" (whatever that may turn out to be) is a nice plus. Asserting / checking and throwing leaves all the code in the assembly. Sure, most of it is cold. But the branches are there and code locality isn't optimal either.

For that use case, having a macro @assume <condition> would also be fine. For debugging, one would need to check those assumptions manually. But at least the compiler knows about the contract between caller and callee then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error handling Handling of exceptions by Julia or the user
Projects
None yet
Development

Successfully merging a pull request may close this issue.