New post: 'A caveat when using assert()'

[ghost]

The post explains a performance problem that we may be creating inadvertently when using assert(), together with a proposed solution.

[undefdev]

Your special-purpose assert is a good idea, but it is not compatible with the standard assert and will break code. (Define it as a separate function but do not replace assert with this.)

If its first argument is truish, assert( … ) returns all arguments unchanged. Your function drops the second argument in that case.

Usually you use assert( ) to check correct usage (and the error means that your code is not proper as it's using the function in the wrong way). You never use assert( ) to check things that might fail at runtime -- you return nil, message. The user of the function may then decide to check manually or (hacky, should only be done at the top level) wrap assert( … ) around the call to hard-fail (e.g. if opening a file fails and there's absolutely nothing to clean up and nothing can be fixed).

special case: You decide to use assert/error in your library and pcall all things that might throw an error, so that you return nil, message at the top level. Then you can use assert for run time checks, and it's a good idea to define extra special-purpose asserts, like assert_type, assert_pred( p, ... ) [if p( ... ) is true, returns ... else p returns nil, error and the error message is formatted by/in p and then thrown.]

[ghost]

Thanks for your comments. You're right, I was failing to return the second parameter and that was a bug in my function. I'm submitting a fixed version now, with the additional feature of using error()'s level parameter to report the position where the assertion failed rather than reporting it within the assert() function itself. I've re-run the benchmark because the function has changed, though the result is pretty much always the same (within some random variation), because the code for passed assertions doesn't change and that's what's timed.

I haven't changed the function name because I'm afraid that that might discourage its usage, so I'd ask you to reconsider. I will change it if that's the general opinion.

I don't understand where you're going with the rest of your comment. I'm not sure if you're suggesting a change or not, and if so, which. If it's just a side comment, I'll add that one clear place where I see a need for protecting failed assertions is unit testing. If one test fails because of an assertion error, you may still want to keep the tests going in order to report a total of passed/failed tests.

[undefdev]

Sorry for the delay, I'm very ill at the moment.

There still is a bug in your function, it also drops the first argument. Assert usually returns all arguments unchanged (including a). Sorry for not noticing earlier.

I don't believe people will be discouraged from using your function if it's not called assert, they will either implement it or not, and if they use it they should use it knowingly.

Your post makes it very clear that the original assert has some shortcomings, and by educating people about that you encourage them to make use of custom asserts when needed.

I strongly suggest to change the function name – the simplest choice is probably xassert, in analogy to xpcall as a variation on pcall (xpcall allows lets you define the error handling function, likewise xassert would allow you to define the error formatting function.)

It should also be faster than a multi-purpose assert. It could look like this (not compatible with assert):

function xassert( f, ... )
  if ... then  return ...  end
  error( f( ... ) or "assertion failed!", 2 )
end

Or if you really prefer having a multi-purpose assert:

function xassert( b, f, ... )
  if b then  return b, f, ...  end
  if type( f ) == "function" then  error( f( ... ), 2 )  end
  error( f or "assertion failed!", 2 )
end

My side comment wasn't very clear, sorry. What I meant was that in Lua code you usually avoid the use of assert/error in runtime code, because unless you want your program to terminate, you will have to wrap it into pcall which makes things really slow (like you also mention in the post). The common thing is to return nil and an error message if a function fails, which is also how functions in the standard library behave (e.g. load, io.open, package.searchpath) Which in turn can then be wrapped with asserts for testing.

The other part of the side comment was about how I've usually seen custom asserts for unit testing. In C, the predicate used inline as the condition is just reused as the message, nice and simple. Here, the inline predicate disappears and all you get is a bool, and you somehow have to re-state everything for the message:

assert(type(x) == "number", type_err, "x", x)

You're stating twice that you're type-checking, and refer to the variable three times. So it would be a good idea to make a custom assert for type checking that you could use like this:

assert_type( x, "number" )

Anyway, that's just my two cents. Your blog post shares some important knowledge and even though I would do things differently I would like to merge it, but I'd really like you to change the name of your function, for the reasons I've mentioned.

[ghost]

No problem, hope you get well soon! You're right once more, that was embarrassing. So I've fixed it, and this time I've taken your advice on not rewriting the system's one and using xassert as a name, adjusting the text accordingly.

I much prefer to make it compatible, to make people have a base to rely on. Unfortunately, your suggested replacement isn't compatible enough: returning nil as second parameter is not the same as returning one parameter. Consider, for example, io.write(assert("x")) vs. io.write(xassert("x")): with your definition, the latter fails with: luajit: xassert.lua:7: bad argument #2 to 'write' (string expected, got nil).

After some thought on your suggestion about the custom assert function, I've decided to reformulate my function and leave a short note pointing to your solution as an alternative one. Your definition wouldn't allow to display the original name of the variable as the example does, so you need to pass the name of the variable anyway, and then the parameter list starts to become confusing as to what the parameters are and in which order. That was the case in my example too, so I've reformulated it in a way that has at least some logic.

Thanks again for the comments.

[undefdev]

Hey, thanks for the changes!

Good thing you found that bug, let's merge this then!

[Etiene]

Thank you for your post, @pgimeno! Thank you for the time you took to write it, and the time you took to review and modify it :dancers: It's a very interesting technical post

Also, thank you very much @undefdev, for the work you've put into reviewing the post, specially being ill! Get better :+1:

This is pretty awesome! I'm gonna pull and publish this tomorrow because monday posts usually get more views than sunday's :dancer: Great work, everyone! 💖

[ghost]

I submitted a comment (duplicated) about a week ago and it said it was in the moderation queue. Is there anyone in charge of approving comments?

[Etiene]

Oh! That one is me and I didn't get a notification! I'm very sorry, it's approved now!