Suggestion: include object data in assertion messages

Hi all,

There are many assertions sprinkled throughout the llvm codebase,
which is a GoodThing. Most of the assertions even have informative
messages, which is a BetterThing.

However, assertion messages are static strings, and don't include any
information about the particular object/value which caused the
assertion. In a 'data oriented' system like llvm, this makes it really
difficult to pinpoint which node is actually triggering the assertion.

For example, in ScheduleDAGSDNodesEmit.cpp
(lib/CodeGen/SelectionDAG/), in the ScheduleDAGSDNodes::AddOperand
method, there is the following assert:

assert(Op.getValueType() !=MVT::Other && Op.getValueType() !=
MVT::Flag && "Chain and flag operands should occur at end of operand
list!");

In order to understand which operand was causing the failure, I need
to dig quite a lot.

What I propose is that an 'assertf' function be added, which wraps
assert and sprintf. This would allow the 'missing' information to be
added to assertion messages. This (imo) would be the BestThing :wink:

Thoughts?

The typical idiom we use for this are things like:

Value::~Value() {
#ifndef NDEBUG // Only in -g mode...
   // Check to make sure that there are no uses of this value that are still
   // around when the value is destroyed. If there are, then we have a dangling
   // reference and something is wrong. This code is here to print out what is
   // still being referenced. The value in question should be printed as
   // a <badref>
   //
   if (!use_empty()) {
     cerr << "While deleting: " << *VTy << " %" << getNameStr() << "\n";
     for (use_iterator I = use_begin(), E = use_end(); I != E; ++I)
       cerr << "Use still stuck around after Def is destroyed:"
            << **I << "\n";
   }
#endif
   assert(use_empty() && "Uses remain when a value is destroyed!");

This lets you do anything you want before crashing.

-Chris

Thats good... but the NDEBUG region gets printed even if the assertion passes.

Ideally, a printf-style format string would be better. That way you
get info relevant to your assertion, with your assertion.

Except that the condition on the 'if' is the inverse of the assertion, so that's not the case. However, if you're concerned about the redundant branch you can instead write:

     if (!...) {
#ifndef NDEBUG
       cerr << ...;
#endif
       assert(0 && "panic!");
     }

— Gordon