Re: [Warzone-dev] assert return macro

2009-02-01 Thread Giel van Schijndel
On Sat, Jan 31, 2009 at 11:45:11PM +0100, Per Inge Mathisen wrote:
 No. There are in fact three places now that the expression result is checked:
 1) Checking if we should log an error
 2) The assert()
 3) The return condition
 
 In my patch I only cache it in the first case for the last case, since
 the second case will be void in non-debug builds anyway. But we should
 consider just doing assert(false) since we log the expression and
 error better in the code right above it anyway.

I'd rather not use assert(false) as on some systems assert() is used to
produce a nice GUI message. E.g. on Windows you'll get a message like
assertion $expression failed, abort, ignore, attach debugger?.
Deciding which of those buttons to click becomes difficult if you don't
get to see any sensible expression there.

Perhaps as a fallback we could do something like this:
 #define ASSERT_XXX(expr, str_expr, ...) \
   assert(!str_expr)
Which for ASSERT(a  b, ...) (sh|w)ould be equal to:
 assert(!a  b);

That would be false as well but would still retain the benefit of single
evaluation.

 Additionally I think ASSERT_OR_RETURN is a slightly better name
 
 It is misleading, though, since it can also be ASSERT_AND_RETURN or
 just RETURN, depending on how the program is compiled and how gdb
 reacts to abort signals. So I would prefer to keep the slight
 ambiguity in the name.

Then do I read this wrong?
 Assert that returns given return value on failure in non-debug builds.

I interpret that as, when I do:
 ASSERT_RETURN(expr, retval, blabla);
it'll return retval when expr yields false when evaluated.

To me the above ASSERT_RETURN statement reads like this:
 * assert that expr holds true
 * failing to assert that expression, return retval

I'd say that's always equal to ASSERT_OR_RETURN, I can't think of any
case where it'd become ASSERT_AND_RETURN. Translating those two names to
macros (vastly simplified and using invalid C syntax) I'd write this:
 #define ASSERT_OR_RETURN(expr, retval) \
   (expr) || return(retval)
 #define ASSERT_AND_RETURN(expr, retval) \
   (expr)  return(retval)

I think (taking shortcut behaviour into account), the former is what you
want, not the latter.

-- 
Giel


signature.asc
Description: Digital signature
___
Warzone-dev mailing list
Warzone-dev@gna.org
https://mail.gna.org/listinfo/warzone-dev


Re: [Warzone-dev] assert return macro

2009-02-01 Thread Per Inge Mathisen
On Sun, Feb 1, 2009 at 11:55 PM, Giel van Schijndel m...@mortis.eu wrote:
 I'd rather not use assert(false) as on some systems assert() is used to
 produce a nice GUI message. E.g. on Windows you'll get a message like
 assertion $expression failed, abort, ignore, attach debugger?.
 Deciding which of those buttons to click becomes difficult if you don't
 get to see any sensible expression there.

 Perhaps as a fallback we could do something like this:
 #define ASSERT_XXX(expr, str_expr, ...) \
   assert(!str_expr)
 Which for ASSERT(a  b, ...) (sh|w)ould be equal to:
 assert(!a  b);

 That would be false as well but would still retain the benefit of single
 evaluation.

That sounds like a good idea.

 It is misleading, though, since it can also be ASSERT_AND_RETURN or
 just RETURN, depending on how the program is compiled and how gdb
 reacts to abort signals. So I would prefer to keep the slight
 ambiguity in the name.

 Then do I read this wrong?
 Assert that returns given return value on failure in non-debug builds.

In debug builds you get both an assert and a return if the condition
fails.  You may not always get to the return, but it is there for
those cases where you ignore the abort signal.

 I'd say that's always equal to ASSERT_OR_RETURN

Maybe. Maybe not. But ASSERT_RETURN is what I find used elsewhere.
According to google it is somewhat frequent (26k pages), while it
gives me exactly 3 pages for either of ASSERT_OR_RETURN as
ASSERT_AND_RETURN.

  - Per

___
Warzone-dev mailing list
Warzone-dev@gna.org
https://mail.gna.org/listinfo/warzone-dev