Proposal for better assertions in LLVM

Project Infrastructure LLVM Dev List Archives
1

The assertions in LLVM would be a lot more useful if they could print out not only the source code of the expression that failed, but also print the values of the various arguments. To that end, I have an idea for an improved assert macro which would use raw_ostream. It would look something like this:

ASSERT_STRM(Ty == STy, "Expected " << Ty << " but got " << STy->getElementType(0));

This would transform to:

if (!(Ty == STy))
AssertionFailureStream(FILE, LINE) <<
"Expected " << Ty << " but got " << STy->getElementType(0);

The AssertionFailureStream is a subtype of raw_ostream whose destructor calls abort() after printing the contents of the stream to stderr. (I use a similar technique in my frontend.)

As you can see, this ought to work with any argument type that can work with raw_ostream. And it shouldn’t cost anything if the assert doesn’t fail.

The actual definition of the macro would be something like this:

#define ASSERT_STRM(cond, args)
if (!(cond)) AssertionFailureStream(FILE, LINE) << args

Note that there’s no trailing semicolon, as this is supplied at the point where the macro is invoked.

What do you think?

2

   #define ASSERT_STRM(cond, args) \
     if (!(cond)) AssertionFailureStream(__FILE__, __LINE__) << args

Note that there's no trailing semicolon, as this is supplied at the
point where the macro is invoked.

What do you think?

Neat, but inducing a debug trap is even more useful on asserts (optionally).

3

What’s the best way to do that? I’d like to incorporate it into my frontend’s diagnostics.

4

Depends on platform. asm {int 3} on x86, asm {trap} on ppc, DebugBreak on windows, etc.

5

By the way, abort() trips the debugger as well - at least, it does in gdb.

6

By the way, abort() trips the debugger as well - at least, it does in gdb.

Yep, but tripping the debugger is highly non-portable.

7

Hi-

Yep, but tripping the debugger is highly non-portable.

You're suggesting that inline asm is more portable than calling abort?

Alistair

8

He wants to be able to resume execution from the debugger after
assertion failure.

Reid

9

wrapping the macro’s body in:

do { … } while (false)

would make the the macro a proper statement so that:

if (cond)
ASSERT(some_other_cond);
else
do_something_cool ();

compiles as expected.

IMO, it would work as such

#define ASSERT_STM(cond,expr)

10

sorry, my previous message got sent too early

I think the macro should look something like this:

#define ASSERT_STRM(cond,expr)
do {
if (cond) {
std::cerr << expr << std::end;
assertion_trap ();
}
} while (false)

11

Great suggestion - what should the !NDEBUG version look like?

12

do {} while (false)

13

Here’s an example of how this would be used: In the constructor for ConstantVector, there’s an assert:

assert(C->getType() == T->getElementType() &&
“Initializer for vector element doesn’t match vector element type!”);

I would change this to:

ASSERT_STRM(C->getType() == T->getElementType(),
"Initializer for vector element " << I - V.begin() << " with type " <<
C->getType() << " doesn’t match vector element type " <<
T->getElementType());

With more detailed assertions like this, a much larger class of programmer errors can be diagnosed without having to go into the debugger.

Because the stream is a raw_ostream, LLVM types and values can easily be printed to the stream without having to convert them to string form.

Based on the suggestions so far, I’m guessing that the macro would look like this:

#include “llvm/Support/raw_ostream.h”

namespace llvm {

// This just writes out “Assertion Failed” and the file/line number in
// a form that IDEs can parse.
void assertion_prologue(const char * file, unsigned line);

// This does the trap or abort or whatever.
void assertion_trap();

#if NDEBUG
#define LLVM_ASSERT_STRM(expr, args) do {} while false
// Alternatively
// #define LLVM_ASSERT_STRM(expr, args) (void)0
#else

/// Assertion macro that acts as an error output stream. Typical usage:
///
/// LLVM_ASSERT_STRM(a != b, “Expected " << a << " == " << " b”)
#define LLVM_ASSERT_STRM(expr, args)
do { if (!(expr)) {
assertion_prologue(FILE, LINE);
errs() << args << “\n”;
assertion_trap();
} while (false)

#endif

/// Assertion macro that behaves like standard C assert().
#define LLVM_ASSERT(expr) LLVM_LLASSEERT_STRM(expr, #expr)

/// Assertion macro that expects two values to be equal, as defined
/// by the equality operator.
#define LLVM_LLASSERT_EQ(expected, actual)
LLVM_LLASSEERT_STRM(expected == actual, "Expected: " << expected <<
" but actually was " << actual)

I’m open to suggestions on naming - LLVM_ASSERT_STRM may be a little too verbose, but you have to be careful about name collisions since macros aren’t namespaced. In particular I want to avoid any overlap with Google Test, which has a bunch of ASSERT_XXX macros.

14

For a non-verbose version, maybe “assertf” or “asserts”, though that may be too short. Something like that could work if it where limited to non externally visible code (i.e. only cpp files or internal headers)

15

I’m unconvinced that this is worth it at all. This is only going to allow you to “avoid going into the debugger” in the most trivial cases.

-Chris

16

Here’s an example of how this would be used: In the constructor for ConstantVector, there’s an assert:

assert(C->getType() == T->getElementType() &&
“Initializer for vector element doesn’t match vector element type!”);

I would change this to:

ASSERT_STRM(C->getType() == T->getElementType(),
"Initializer for vector element " << I - V.begin() << " with type " <<
C->getType() << " doesn’t match vector element type " <<
T->getElementType());

With more detailed assertions like this, a much larger class of programmer errors can be diagnosed without having to go into the debugger.

Because the stream is a raw_ostream, LLVM types and values can easily be printed to the stream without having to convert them to string form.

I’m unconvinced that this is worth it at all. This is only going to allow you to “avoid going into the debugger” in the most trivial cases.

From an API user’s perspective, those trivial cases happen quite frequently, at least in my experience. Often times I only need to know what type is involved to figure out what went wrong.

17

Chromium (and several other Google open source projects) have a somewhat superior variant of this:

http://google.com/codesearch#hfE6470xZHk/base/logging.h&exact_package=http://src.chromium.org/git/chromium.git&q=CHECK&type=cs&l=398

It ends up looking like:

CHECK(Ty == STy) << “Expected " << Ty << " …” << …;

The important difference is that most of the parameters go outside of the macro rather than inside, making error messages way more intelligible. If we want to go this direction in LLVM, I’d much rather see this formulation. It’s still easy to have it compile away to nothing using ?: operator.

If there is a desire to go this route, I’ve worked closely with several variants of the pattern and would be happy to contribute a minimal implementation.

However, I agree with Chris’s concerns about how useful this is in LLVM. While I’ve used it on projects where it provides tremendous value, with LLVM I often end up in the debugger anyways. I think the automatic debugger trapping is marginally more useful, but only marginally. Having my debugger discard the SIGABRT and continue isn’t too hard. On the flip side, it might make crash reports from users much more useful (in the absence of, or prior to arriving at, a reduction). I think the cost is quite low, so maybe its worth it, but maybe it isn’t.

I’ll leave it to Chris and others to make the call, just wanted to make sure a slightly different implementation was on the table in case it does go in…

18

I'm unconvinced that this is worth it at all. This is only going to allow
you to "avoid going into the debugger" in the most trivial cases.

People doing something new tend to run into the trivial cases quite often.
That's exactly the user population that will profit most from more
informative error messages.

E.g. people who're getting their feet wet with LLVM in general.
Even more so, people starting to hack on LLVM.

Just my 2c.

19

If this is done we should make sure the expressions operated on by the stream are only evaluated when the condition is false and !NDEBUG.

Since the macros that I’m used to are not available in LLVM, I’m often tempted to write this:

#ifndef NDEBUG
if (!cond) {
dbgs() << something_expensive_to_lookup;
assert(false);
}
#endif

I’ve gotten used to spending time in the debugger for simple problems just because using too many DEBUG() statements blows up the trace output, but nasty #ifdefs like this obfuscate the non-error code.

-Andy

20

The assertions in LLVM would be a lot more useful if they could print out not only the source code of the expression that failed, but also print the values of the various arguments. To that end, I have an idea for an improved assert macro which would use raw_ostream. It would look something like this:

ASSERT_STRM(Ty == STy, "Expected " << Ty << " but got " << STy->getElementType(0));

Chromium (and several other Google open source projects) have a somewhat superior variant of this:

http://google.com/codesearch#hfE6470xZHk/base/logging.h&exact_package=http://src.chromium.org/git/chromium.git&q=CHECK&type=cs&l=398

It ends up looking like:

CHECK(Ty == STy) << “Expected " << Ty << " …” << …;

The important difference is that most of the parameters go outside of the macro rather than inside, making error messages way more intelligible. If we want to go this direction in LLVM, I’d much rather see this formulation. It’s still easy to have it compile away to nothing using ?: operator.

Can you explain how it avoids evaluating the arguments in the false case? I looked at the code but it’s pretty complex.