I agree. :)

Created attachment 465 [details]
RFC patch for assert macro.


Here's a first shot at this. I decided to go hardcore here.  :)

Totally untested, not even compiled, this is just proof of concept.

The plan:
Using the C runtime assert() has issues and limitations, so we never use it, even if available. I figure if we're going to do this, we might as well think about the best possible assert() we can get.


This version has the following features:
- It works with single-statement "if" statements...
    if (blah)
      SDL_assert(x);
    else
      do_something();

   ...that "else" will end up in the right place, with or without assertions.

- It works with otherwise unused variables...

      int q = blah();
      SDL_assert(q != 1);

   ...won't throw a compiler warning about unused variables.

- It totally removes itself from generated code when assertions are disabled, but the compiler still parses the expression, so these don't bitrot if assertions get disabled for extended periods of time.

- It allows the developer to abort/break/retry/ignore/ignore-always for every individual assertion.

- It keeps a running total of how many times a given assertion has triggered, with unique counter for each assertion.

Obviously, there are several FIXMEs still, and the code style needs a cleanup. But I'm posting this for review of the basic idea first.

--ryan.

Woah.  I like it!

I'm not sure about the trigger count.  If you get a line printed each time the assertion is hit, it seems like it's only useful for accumulated stats at the end of the program.

Also, it would be awesome for the app to be able to provide an assertion catching function and/or set the default assertion behavior.

Can we check to see if the compiler supports __FUNCTION__ as well?

Can we have a couple levels of assertion as well?  e.g. NONE/RELEASE/PARANOID

Bumping priority on a set of bugs.

--ryan.

Let me think on the trigger count stuff.

> Also, it would be awesome for the app to be able to provide an assertion
> catching function and/or set the default assertion behavior.

I hesitate to let the app touch this stuff at all. Part of me wants to add hooks to let the app show its own UI, but this is meant for debugging SDL, and things get complicated when we start trying to interact with the app.

> Can we check to see if the compiler supports __FUNCTION__ as well?

That's a good idea.
 
> Can we have a couple levels of assertion as well?  e.g. NONE/RELEASE/PARANOID

Yeah, we can do that.

--ryan.

Created attachment 466 [details]
Second attempt.


Okay, round two.

Still TODO:
- Platform code needs work, but x86/x86-64 Windows/Mac/Linux should be good to go for the basic stuff.
- Need to fill in UI for triggering assertions. Right now it just assumes the user wants an abort, which is basically what a normal assert does.

- Now mutexed, so it's safe for two threads to assert at the same time, even on the same line of code.
- Now reports function name, if possible.
- Now hooked into the build system and compiling cleanly.
- Code has been cleaned up.
- Everything that isn't crucial to run in the macro has been moved out of it. Just the things that need to be there (unique static data allocation, breakpoint triggering so it's on the right line in the debugger, and loops so you can retry the condition).
- Can now enable "levels" of assertion. This is currently disabled, release, normal, and paranoid. Use --enable-assertions=[no|release|yes|paranoid] in the configure script to set it. Things that don't use configure have release settings in their SDL_config_*.h for now, just for basic test coverage.
The macros are SDL_assert(), SDL_assert_release() and SDL_assert_paranoid().
- We can now generate reports of all assertions (we build a linked list of the static structs each assert macro creates, as they trigger, so adding to the report is an O(1) operation with no dynamic allocations involved). Does nothing if no assertions ever trigger. Still, this can be turned off with a #define.

So if you were to have this code and ignore all assertions...

    SDL_assert_release(1 == 2);
    SDL_assert_release(5 < 4);
    SDL_assert_release(0 && "This is a test");

...you'd see this when you call SDL_Quit()...

SDL assertion report.
All SDL assertions between last init/quit:

'0 && "This is a test"'
    * SDL_Init (src/SDL.c:195)
    * triggered 1 times.
    * always ignore: no.
'5 < 4'
    * SDL_Init (src/SDL.c:194)
    * triggered 1 times.
    * always ignore: no.
'1 == 2'
    * SDL_Init (src/SDL.c:193)
    * triggered 1 times.
    * always ignore: no.



...is this sounding closer to your idea, Sam?

--ryan.

Created attachment 473 [details]
Third shot.


More work.

- Assertion handler can take an environment variable (so unit tests, etc, don't hang up, waiting on a click in an assertion GUI).
- Cocoa UI for Mac OS X is complete.
- stdio "UI" for Unix is complete.
- mutex fixes.
- other tweaks.

Still todo:
- Windows UI.

--ryan.

Created attachment 475 [details]
Feature-complete patch.


Here's the final patch. I cleaned up a few things and wrote the Windows code.

Please note that the Windows code is really nasty. There is definite room for improvement. But spending too much time building GUI controls with win32 API calls is madness.   :)

Also, Windows (and Mac OS X) throw a fit when an assertion is triggered during a fullscreen rendering context. The assertion dialog will work, but it tends to leave the desktop or application broken. This could be handled better with some later refinements.

All this needs now is Sam to look it over and clean it up to his liking, so I'm tossing the bug to him. Toss it back to me if you just want me to commit it.

After that, we can start sprinkling the codebase with assertions.

--ryan.

Tossing to Sam.

--ryan.

Okay, this is in!