Proposal for better assertions in LLVM

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
31 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Proposal for better assertions in LLVM

Talin-3
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?

--
-- Talin

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal for better assertions in LLVM

FlyLanguage
>    #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).
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal for better assertions in LLVM

Talin-3
On Tue, Jul 26, 2011 at 10:56 AM, FlyLanguage <[hidden email]> wrote:
  #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).

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

--
-- Talin

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal for better assertions in LLVM

FlyLanguage
Den 26.07.2011 20:12, skrev Talin:

> On Tue, Jul 26, 2011 at 10:56 AM, FlyLanguage <[hidden email]
> <mailto:[hidden email]>> wrote:
>
>            #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).
>
>
> What's the best way to do that? I'd like to incorporate it into my
> frontend's diagnostics.

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

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal for better assertions in LLVM

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

On Tue, Jul 26, 2011 at 12:12 PM, FlyLanguage <[hidden email]> wrote:
Den <a href="tel:26.07.2011%2020" value="+12607201120" target="_blank">26.07.2011 20:12, skrev Talin:
On Tue, Jul 26, 2011 at 10:56 AM, FlyLanguage <[hidden email]
<mailto:[hidden email]>> wrote:

          #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).


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

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




--
-- Talin

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal for better assertions in LLVM

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

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

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal for better assertions in LLVM

Alistair Lynn
Hi-

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

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

Alistair

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal for better assertions in LLVM

Reid Kleckner-3
He wants to be able to resume execution from the debugger after
assertion failure.

Reid

On Tue, Jul 26, 2011 at 8:07 PM, Alistair Lynn <[hidden email]> wrote:

> Hi-
>
>> Yep, but tripping the debugger is highly non-portable.
>
> You're suggesting that inline asm is more portable than calling abort?
>
> Alistair
>
> _______________________________________________
> LLVM Developers mailing list
> [hidden email]         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal for better assertions in LLVM

Nathan Jeffords
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)


On Tue, Jul 26, 2011 at 6:36 PM, Reid Kleckner <[hidden email]> wrote:
He wants to be able to resume execution from the debugger after
assertion failure.

Reid

On Tue, Jul 26, 2011 at 8:07 PM, Alistair Lynn <[hidden email]> wrote:
> Hi-
>
>> Yep, but tripping the debugger is highly non-portable.
>
> You're suggesting that inline asm is more portable than calling abort?
>
> Alistair
>
> _______________________________________________
> LLVM Developers mailing list
> [hidden email]         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal for better assertions in LLVM

Nathan Jeffords
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)

On Tue, Jul 26, 2011 at 7:57 PM, Nathan Jeffords <[hidden email]> wrote:
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)


On Tue, Jul 26, 2011 at 6:36 PM, Reid Kleckner <[hidden email]> wrote:
He wants to be able to resume execution from the debugger after
assertion failure.

Reid

On Tue, Jul 26, 2011 at 8:07 PM, Alistair Lynn <[hidden email]> wrote:
> Hi-
>
>> Yep, but tripping the debugger is highly non-portable.
>
> You're suggesting that inline asm is more portable than calling abort?
>
> Alistair
>
> _______________________________________________
> LLVM Developers mailing list
> [hidden email]         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal for better assertions in LLVM

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

On Tue, Jul 26, 2011 at 8:01 PM, Nathan Jeffords <[hidden email]> wrote:
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)


On Tue, Jul 26, 2011 at 7:57 PM, Nathan Jeffords <[hidden email]> wrote:
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)


On Tue, Jul 26, 2011 at 6:36 PM, Reid Kleckner <[hidden email]> wrote:
He wants to be able to resume execution from the debugger after
assertion failure.

Reid

On Tue, Jul 26, 2011 at 8:07 PM, Alistair Lynn <[hidden email]> wrote:
> Hi-
>
>> Yep, but tripping the debugger is highly non-portable.
>
> You're suggesting that inline asm is more portable than calling abort?
>
> Alistair
>
> _______________________________________________
> LLVM Developers mailing list
> [hidden email]         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev




--
-- Talin

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal for better assertions in LLVM

Nathan Jeffords
do {} while (false)

On Tue, Jul 26, 2011 at 8:53 PM, Talin <[hidden email]> wrote:
Great suggestion - what should the !NDEBUG version look like?


On Tue, Jul 26, 2011 at 8:01 PM, Nathan Jeffords <[hidden email]> wrote:
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)


On Tue, Jul 26, 2011 at 7:57 PM, Nathan Jeffords <[hidden email]> wrote:
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)


On Tue, Jul 26, 2011 at 6:36 PM, Reid Kleckner <[hidden email]> wrote:
He wants to be able to resume execution from the debugger after
assertion failure.

Reid

On Tue, Jul 26, 2011 at 8:07 PM, Alistair Lynn <[hidden email]> wrote:
> Hi-
>
>> Yep, but tripping the debugger is highly non-portable.
>
> You're suggesting that inline asm is more portable than calling abort?
>
> Alistair
>
> _______________________________________________
> LLVM Developers mailing list
> [hidden email]         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev




--
-- Talin


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal for better assertions in LLVM

Talin-3
In reply to this post by Nathan Jeffords
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.

On Tue, Jul 26, 2011 at 8:01 PM, Nathan Jeffords <[hidden email]> wrote:
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)


On Tue, Jul 26, 2011 at 7:57 PM, Nathan Jeffords <[hidden email]> wrote:
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)


On Tue, Jul 26, 2011 at 6:36 PM, Reid Kleckner <[hidden email]> wrote:
He wants to be able to resume execution from the debugger after
assertion failure.

Reid

On Tue, Jul 26, 2011 at 8:07 PM, Alistair Lynn <[hidden email]> wrote:
> Hi-
>
>> Yep, but tripping the debugger is highly non-portable.
>
> You're suggesting that inline asm is more portable than calling abort?
>
> Alistair
>
> _______________________________________________
> LLVM Developers mailing list
> [hidden email]         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev




--
-- Talin

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal for better assertions in LLVM

Nathan Jeffords
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)

On Tue, Jul 26, 2011 at 9:17 PM, Talin <[hidden email]> wrote:
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.

On Tue, Jul 26, 2011 at 8:01 PM, Nathan Jeffords <[hidden email]> wrote:
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)


On Tue, Jul 26, 2011 at 7:57 PM, Nathan Jeffords <[hidden email]> wrote:
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)


On Tue, Jul 26, 2011 at 6:36 PM, Reid Kleckner <[hidden email]> wrote:
He wants to be able to resume execution from the debugger after
assertion failure.

Reid

On Tue, Jul 26, 2011 at 8:07 PM, Alistair Lynn <[hidden email]> wrote:
> Hi-
>
>> Yep, but tripping the debugger is highly non-portable.
>
> You're suggesting that inline asm is more portable than calling abort?
>
> Alistair
>
> _______________________________________________
> LLVM Developers mailing list
> [hidden email]         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev




--
-- Talin


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal for better assertions in LLVM

Chris Lattner-2
In reply to this post by Talin-3

On Jul 26, 2011, at 9:17 PM, Talin wrote:

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.

-Chris

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal for better assertions in LLVM

Talin-3
On Tue, Jul 26, 2011 at 10:59 PM, Chris Lattner <[hidden email]> wrote:

On Jul 26, 2011, at 9:17 PM, Talin wrote:

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.

-Chris



--
-- Talin

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal for better assertions in LLVM

Chandler Carruth-2
In reply to this post by Talin-3
On Tue, Jul 26, 2011 at 10:41 AM, Talin <[hidden email]> wrote:
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:


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...

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal for better assertions in LLVM

Joachim Durchholz
In reply to this post by Chris Lattner-2
> 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.

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal for better assertions in LLVM

Andrew Trick
In reply to this post by Chandler Carruth-2

On Jul 27, 2011, at 12:51 AM, Chandler Carruth wrote:

On Tue, Jul 26, 2011 at 10:41 AM, Talin <[hidden email]> wrote:
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:


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.

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

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

Re: Proposal for better assertions in LLVM

Talin-3
In reply to this post by Chandler Carruth-2
On Wed, Jul 27, 2011 at 12:51 AM, Chandler Carruth <[hidden email]> wrote:
On Tue, Jul 26, 2011 at 10:41 AM, Talin <[hidden email]> wrote:
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:


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.
 
*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...



--
-- Talin

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
12