[Proposal] NBL ASSERT Macros

[Erfan-Ahmadi]

Description

Currently in implementation of nbl functions we use variety of ways to validate inputs and calculations using asserts, ifs and debug breaks.

Problems with the current state of things invlove:

• Most of the time we debug in RelWithDebug and we might not get important asserts and delay fixing an important bug
• The code gets longer, dirtier and a less unreadable with a lot of assert(false)s
• There is no concept of a "AssertLevel" based on build configuration since we use the assert from c
• Sometimes we want to get the ASSERT but be able to continue and not terminate the application (Based on the configuration or build settings)
• If someone is not in VisualStudio and not debugging they won't be able to see where the application failed or hit an assert

Of course we can do all above using DEBUG_BREAK, ASSERT, ILogger. but it would require longer and redundant code

Here is some patterns from around the engine:

if (!condition)
{
logger.log("some error happened");
assert(false);
}

if (!condition)
{
assert(false);
return false;
}

My solution described below fixes the mentioned problems. and in addition brings some beauty to how we get the errors and asserts.

Solution proposal

We define and NBL_ASSERT_LEVEL which can take values 0, 1, 2 NBL_ASSERT_LEVEL == 2 -> Strictest -> is for DEBUG NBL_ASSERT_LEVEL == 1 -> somewhat strict -> is for not DEBUG NBL_ASSERT_LEVEL == 0 -> Doesn't do anything on Asserts -> can be overriden by cmake (by defining it first)

Here we define 3 new macros:

NBL_ASSERT 
NBL_ASSERT_STRONG
NBL_ASSERT_REPAIR

** I'll define what NblAssert() function is below

NBL_ASSERT: does nothing in level 0 and 1, but does NblAssert() for level 2 NBL_ASSERT_STRONG: does nothing in level 0, but does NblAssert() for level 1 and 2 NBL_ASSERT_REPAIR: returns (condition) at level 0 and 1, but does NblAssert() for 2 this is a special macro for us to be able to use an assert and check for an if at the same time.

if (NBL_ASSERT_REPAIR(VK_RET==VK_SUCCESS))
 return CVulkanSomething;

Assertion Process

NblAssert() is our internal static function used for assertions and in the macros above

bool NBL_API
NblAssert (
    bool cond, char const * cond_str,
    char const * func, char const * file, int line,
    char const * msg_fmt, ...)

This function constructs a beutifull message from the input strings and cond_str provided with some macro fu**ery in the case that condition fails.

We now have the option to:

• log to stderr stream using fprintf (seperate from our logging system because it's not global)
• use MessageBoxA to show a nice message box in windows and ask the user if they want to terminate or continue

If the condition fails we definitely do these:

• __debugbreak()
• we use std::terminate(); unless the message box returned ID_CONTINUE(?)

What it would look like:

NBL_ASSERT(inputOffset != InvalidOffset);
NBL_ASSERT(some_value == 0, "some_value should be 0 but It's %d", some_value);
NBL_ASSERT_STRONG(inputOfAHighlyImportantUtility.valid(), "Offff someone messed up");
if(NBL_ASSERT_REPAIR(ret_value == SUCCESS))
{
   // do some more logic
}

Open for discussion

• How we're going to log, maybe take Logger as an input instead of stderr but we probably need a default or else code gets dirtier (I have some Logger ideas I'll write later in the future that might have an effect here)

[devshgraphicsprogramming]

Assert is meant to be used in "absolutely should not happen" scenarios

Sometimes we want to get the ASSERT but be able to continue and not terminate the application (Based on the configuration or build settings)

This is when we'd want to use NBL_DEBUG_BREAK_IF, assert is not meant to be used to handle Vulkan API returned VK_SUCCESS.

Assert is for when the application MUST close, because:

• the situation makes no sense at all (usually when we use assert(False) without an "error message" string or a comment)
• there's no fallback or error handling code implemented

Insted of Levels, lets have build configs

For example NBL_ASSERT_CONFIGS=Release;RelWithDebInfo;Debug then we can toggle if assert code actually asserts depending on the builds.

@AnastaZIuk could add NBL_ASSERTS_ENABLED conditionally when the build config matches NBL_ASSERT_CONFIGS, to the generated BuildConfig.h containing all the defines.

We can do likewise for NBL_DEBUG_BREAK_IF and NBL_DEBUG_BREAK_IF_NOT

Suggested macro signatures

#define NBL_ASSERT_CUSTOM_LOGGER(COND,LOGGER,FMT_STRING,...) nbl::core::_assert(COND,LOGGER,FMT_STRING,__FUNCTION__,_FILE_,_LINE_,__VA_ARGS__)

#define NBL_DEBUG_BREAK_IF(COND,LOGGER,FMT_STRING,...) nbl::core::debug_break_if(COND,LOGGER,FMT_STRING,__FUNCTION__,_FILE_,_LINE_,__VA_ARGS__)

#define NBL_DEBUG_BREAK_IF_NOT(COND,LOGGER,FMT_STRING,...) nbl::core::debug_break_if_not(COND,LOGGER,FMT_STRING,__FUNCTION__,_FILE_,_LINE_,__VA_ARGS__)

#defne NBL_DEFAULT_DEBUG_LOGGER nbl::core::IDebugLogger::getDefault()

Sadly macros dont support overloading.

Suggested function signatures

namespace nbl::core
{

class IDebugLogger
{
   public:
      virtual void operator()(const char* fmtString, ...) = 0;
      
      // the only singleton I will EVER allow in the engine
      // implementation of the default operator() MUST be THREAD SAFE
      NBL_API static IDebugLogger* getDefault();
};

void _assert(const bool cond, IDebugLogger* logger, const char* fmtString, const char* funcName, const char* fileName, const uint32_t line, ...);
// default logger overload
inline void _assert(const bool cond, const char* fmtString, const char* funcName, const char* fileName, const uint32_t line, ...)
{
    _assert(cond, fmtString, funcName, fileName, line, va_list);
}

// returns `cond`
bool debug_break_if(const bool cond, IDebugLogger* logger, const char* fmtString, const char* funcName, const char* fileName, const uint32_t line, ...);
// default logger overload
bool debug_break_if(const bool cond, const char* fmtString, const char* funcName, const char* fileName, const uint32_t line, ...)
{
    return debug_break_if(cond,fmtString,funcName,fileName,line,va_list);
}

// also returns `cond`
bool debug_break_if_not(const bol cond, IDebugLogger* logger, const char* fmtString, const char* funcName, const char* fileName, const uint32_t line, ...);
// default logger overload
bool debug_break_if_not(const bool cond, const char* fmtString, const char* funcName, const char* fileName, const uint32_t line, ...)
{
    return debug_break_if_not(cond,fmtString,funcName,fileName,line,va_list);
}

}

The Debug-Break-If should somehow detect if a debugger is attached

Throwing exceptions or trigger breakpoints crashes an application which is without a debugger.

When not in a debugger, we should still log stuff, but without tripping breakpoints.

[devshgraphicsprogramming]

WE could alternatively move ILogger from system to core and define everything in terms of that.

[devshgraphicsprogramming]

Ok, lets raise the bar

!!! GLSL ASSERTS !!!

#if _NBL_GLSL_ASSERT_ENABLE_


#extension GL_EXT_debug_printf : enable


#define NBL_GLSL_LOG_ERROR_IF(COND) if (cond) \
{ \
    debugPrintfEXT("Nabla ERROR in File:%s Line:%d",__FILE__,__LINE__); \
    discard; \
}
#define NBL_GLSL_LOG_ERROR_IF_NOT(COND) NBL_GLSL_LOG_ERROR_IF(!COND)

// could decide discard vs return depending on stage type
#define NBL_GLSL_ASSERT(COND) if (!cond) \
{ \
    debugPrintfEXT("Nabla ASSERT in File:%s Line:%d",__FILE__,__LINE__); \
    discard; \
}

#endif

[devshgraphicsprogramming]

We could also steal some ideas from: https://www.boost.org/doc/libs/master/libs/assert/doc/html/assert.html