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 [email protected] https://mail.gna.org/listinfo/warzone-dev
